resolve stricter warnings

In order to convert this repository to Meson, we need to make it
compile under `warning_level=3`. Fix a number of warning classes
across the repository or disable them.

S

resolve stricter warnings

In order to convert this repository to Meson, we need to make it
compile under `warning_level=3`. Fix a number of warning classes
across the repository or disable them.

Some fixes are:

* Add missing header files.
* Fully initialize structs as necessary.
* Add `__attribute__((unused))` on parameters as necessary.
* Fix comparisons between signed and unsigned.
* Fix printf specifiers as necessary.
* Avoid case-fallthrough.
* Remove if conditions which are always true.

Some warnings would require extensive code changes, due to their
pervasive use, and so are disabled at a per-file level:
* `-Wpointer-arith`
* `-Wunused-result`

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: If8992b9108f12b39f796ed090ba29868c9f3c627

show more ...

Disable aspeed mbox backend

This disables the mbox backend in the daemon. The associated code
remains, as it is tangled in to the tests which provide coverage for
parts of the code base beyond the m

Disable aspeed mbox backend

This disables the mbox backend in the daemon. The associated code
remains, as it is tangled in to the tests which provide coverage for
parts of the code base beyond the mbox backend.

A future cleanup would refactor the tests to not rely on the mbox
transport, andfinally remove the mbox transport itself.

Change-Id: Id4fe74617a2dd920c1f8bc8ac4cd69cfbc0239b6
Signed-off-by: Joel Stanley <joel@jms.id.au>

show more ...

mboxd: Don't require mbox

With the DBus interface to the daemon in place it's possible to
implement any hardware interface in terms of the DBus interface. This is
the strategy used to support the IP

mboxd: Don't require mbox

With the DBus interface to the daemon in place it's possible to
implement any hardware interface in terms of the DBus interface. This is
the strategy used to support the IPMI HIOMAP transport. Further, the use
of MBOX is deprecated due to security concerns. We want to drop the
driver from the kernel, which failed to get traction upstream, so make
support optional.

As a consequence, switch the default transport to DBus.

Change-Id: I9f16ca053ce48943dce59b83ca991ec5494580d8
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

Add --trace support (in blktrace format)

In an effort understand what PNOR requests come from the host, it'd be
good to be able to trace what requests come in and visualise them.
blktrace is some Li

Add --trace support (in blktrace format)

In an effort understand what PNOR requests come from the host, it'd be
good to be able to trace what requests come in and visualise them.
blktrace is some Linux infrastructure for tracing block device activity
all the way through the linux block layer, for which there is a variety
of existing tooling. These tools process the (typically) kernel produced
blktrace output. We can produce this same output programatically from
mboxd though.

This patch gives us the (option) to start mboxd in a mode where it will
write a blktrace file out, which can be fed into tools like blkparse(1)
or tools like iowatcher[1] to generate charts (and video).

A quirk of the blktrace format is that it's very geared towards a full
IO subsystem, so we can't directly map window operations (what we know
in mboxd) to specific IO ops (i.e. we don't get "firmware read one page
out of this window before closing it"). So, for each Window opening (or
reusing a cached one), we write THREE blktrace events: a Queue,
Dispatch, and Complete.

We can usk tools like blkparse to do everything from get a detailed list
of what windows were opened and for how long:

0,0 0 1 0.000000000 0 Q R 0 + 8 [(null)]
0,0 0 2 0.000000000 0 D R 0 + 8 [(null)]
0,0 0 3 0.000182022 0 C R 0 + 8 [0]
0,0 0 4 0.042416351 0 Q R 4144 + 2040 [(null)]
0,0 0 5 0.042416351 0 D R 4144 + 2040 [(null)]
0,0 0 6 0.060802662 0 C R 4144 + 2040 [0]
0,0 0 7 0.084775813 0 Q R 64 + 288 [(null)]
0,0 0 8 0.084775813 0 D R 64 + 288 [(null)]
0,0 0 9 0.087835720 0 C R 64 + 288 [0]
0,0 0 10 1.429234244 0 Q R 8488 + 2048 [(null)]

to getting a simple summary at the end of how many windows were opened
read and read/write:

CPU0 (0,0):
Reads Queued: 90, 74,040KiB Writes Queued: 6, 2,664KiB
Read Dispatches: 90, 74,040KiB Write Dispatches: 6, 2,664KiB
Reads Requeued: 0 Writes Requeued: 0
Reads Completed: 90, 74,040KiB Writes Completed: 6, 2,664KiB
Read Merges: 0, 0KiB Write Merges: 0, 0KiB
Read depth: 1 Write depth: 1
IO unplugs: 0 Timer unplugs: 0

If you change the window size to something tiny, like 4096 bytes, you
can get detailed paging information for hostboot at the expense of IPL
time.

Pretty graphs and animations:
https://www.flamingspork.com/blog/?p=4419

[1] iowatcher: http://masoncoding.com/iowatcher/

Change-Id: I5dd02b6bc616c441abf54d87a5d67c972cbaf228
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
[AJ: Resolve merge conflicts, some tidy ups]
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

Add --trace support (in blktrace format)

In an effort understand what PNOR requests come from the host, it'd be
good to be able to trace what requests come in and visualise them.
blktrace is some Li

Add --trace support (in blktrace format)

In an effort understand what PNOR requests come from the host, it'd be
good to be able to trace what requests come in and visualise them.
blktrace is some Linux infrastructure for tracing block device activity
all the way through the linux block layer, for which there is a variety
of existing tooling. These tools process the (typically) kernel produced
blktrace output. We can produce this same output programatically from
mboxd though.

This patch gives us the (option) to start mboxd in a mode where it will
write a blktrace file out, which can be fed into tools like blkparse(1)
or tools like iowatcher[1] to generate charts (and video).

A quirk of the blktrace format is that it's very geared towards a full
IO subsystem, so we can't directly map window operations (what we know
in mboxd) to specific IO ops (i.e. we don't get "firmware read one page
out of this window before closing it"). So, for each Window opening (or
reusing a cached one), we write THREE blktrace events: a Queue,
Dispatch, and Complete.

We can usk tools like blkparse to do everything from get a detailed list
of what windows were opened and for how long:

0,0 0 1 0.000000000 0 Q R 0 + 8 [(null)]
0,0 0 2 0.000000000 0 D R 0 + 8 [(null)]
0,0 0 3 0.000182022 0 C R 0 + 8 [0]
0,0 0 4 0.042416351 0 Q R 4144 + 2040 [(null)]
0,0 0 5 0.042416351 0 D R 4144 + 2040 [(null)]
0,0 0 6 0.060802662 0 C R 4144 + 2040 [0]
0,0 0 7 0.084775813 0 Q R 64 + 288 [(null)]
0,0 0 8 0.084775813 0 D R 64 + 288 [(null)]
0,0 0 9 0.087835720 0 C R 64 + 288 [0]
0,0 0 10 1.429234244 0 Q R 8488 + 2048 [(null)]

to getting a simple summary at the end of how many windows were opened
read and read/write:

CPU0 (0,0):
Reads Queued: 90, 74,040KiB Writes Queued: 6, 2,664KiB
Read Dispatches: 90, 74,040KiB Write Dispatches: 6, 2,664KiB
Reads Requeued: 0 Writes Requeued: 0
Reads Completed: 90, 74,040KiB Writes Completed: 6, 2,664KiB
Read Merges: 0, 0KiB Write Merges: 0, 0KiB
Read depth: 1 Write depth: 1
IO unplugs: 0 Timer unplugs: 0

If you change the window size to something tiny, like 4096 bytes, you
can get detailed paging information for hostboot at the expense of IPL
time.

Pretty graphs and animations:
https://www.flamingspork.com/blog/?p=4419

[1] iowatcher: http://masoncoding.com/iowatcher/

Change-Id: I5dd02b6bc616c441abf54d87a5d67c972cbaf228
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
[AJ: Resolve merge conflicts, some tidy ups]
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Tidy up failure paths on daemon startup

Don't try to cleanup objects we haven't initialised.

Change-Id: I218ab785af36bc3a1d3085c9dcd575e812402433
Signed-off-by: Andrew Jeffery <andrew@aj.id.

mboxd: Tidy up failure paths on daemon startup

Don't try to cleanup objects we haven't initialised.

Change-Id: I218ab785af36bc3a1d3085c9dcd575e812402433
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

vpnor: Rename mboxd_pnor_partition_table sources to backend

Change-Id: I6f0fff4ab54e011c1765fc04186e899754787641
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

mboxd: Add backend DBus interface and commandline options

Also implement a backend commandline option to mboxctl: `mboxctl
--backend ...`, to allow easy run-time switching of the backend from the
co

mboxd: Add backend DBus interface and commandline options

Also implement a backend commandline option to mboxctl: `mboxctl
--backend ...`, to allow easy run-time switching of the backend from the
commandline.

Switching between VPNOR and file backends via mboxctl was tested on
Witherspoon, and MTD and file backends on Romulus.

Change-Id: Iaf0e27ecf1d5cdd9e3a31729fb179096bbc37408
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Add support for file-backed flash devices.

This commit adds a new file based backing source. The file based
backing source takes a raw pnor image that is usually flashed to a mtd
device.

Th

mboxd: Add support for file-backed flash devices.

This commit adds a new file based backing source. The file based
backing source takes a raw pnor image that is usually flashed to a mtd
device.

This backing source enabled rapid testing of pnor images.

Tested on Witherspoon with the VPNOR and file backends, and Romulus for
the MTD and file backends.

Change-Id: I253ecfa6b58d071c7982f153ad50da8e4ad39fa2
Signed-off-by: Evan Lojewski <github@meklort.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Remove flash API compatibility shim

The flash API compatibility was kept to reduce the line noise in the
previous backend patch. Remove the compatibility layer now and convert
the remaining c

mboxd: Remove flash API compatibility shim

The flash API compatibility was kept to reduce the line noise in the
previous backend patch. Remove the compatibility layer now and convert
the remaining call-sites.

Change-Id: I4b6e54f4463059a7804918add81e7572db7b7c21
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Add a backend abstraction layer to mboxd.

Introduce a backend abstraction, enabling multiple implementations to be
compiled in at once. This change formally abstracts the two existing
backend

mboxd: Add a backend abstraction layer to mboxd.

Introduce a backend abstraction, enabling multiple implementations to be
compiled in at once. This change formally abstracts the two existing
backends, mtd and vpnor.

With the backend abstraction in place, subsequent backends are easier to
implement.

This change is based of Evan's work and he retains authorship credit. I
(AJ) have reworked the patch to pass the vpnor tests, refactored some
parts to enable broader use of const structures and others to clarify
the initialisation sequences.

Due to the existing lack of abstraction the patch has unfortunately
wide-ranging impacts. I've whittled it down as much as I consider
reasonable.

Change-Id: I29984a36dae4ea86ec00b853d2a756f0b9afb3ec
Signed-off-by: Evan Lojewski <github@meklort.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

protocol: Introduce protocol_reset()

protocol_reset() encapsulates the actions necessary to return the LPC
state to what's required to boot the host. This is backend dependent;
for the mtd backend w

protocol: Introduce protocol_reset()

protocol_reset() encapsulates the actions necessary to return the LPC
state to what's required to boot the host. This is backend dependent;
for the mtd backend we can simply point the bridge at the host flash
AHB mapping, and for the virtual pnor we want to rearrange the content
of the LPC reserved memory (leaving the bridge pointed there). In either
case the state of the FWH address space is distured, so inform the host
as necessary.

Change-Id: Ie8efd1f703a3616c33f76f4e735c1efea039146c
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Set BMC_EVENT_PROTOCOL_RESET on startup

This resolves skiboot failing to receive the mboxd termination message
if mboxd has been killed unexpectedly or due to some race on shutdown
where the

mboxd: Set BMC_EVENT_PROTOCOL_RESET on startup

This resolves skiboot failing to receive the mboxd termination message
if mboxd has been killed unexpectedly or due to some race on shutdown
where the message fails to propagate.

Change-Id: I7a0974fc17f6853ac62c1f5f7b43d2e367260cf6
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

protocol: Rework publication of events over DBus transport

A set of races was discovered around the propagation of HIOMAP protocol
BMC status events during BMC shutdown. In particular the change imp

protocol: Rework publication of events over DBus transport

A set of races was discovered around the propagation of HIOMAP protocol
BMC status events during BMC shutdown. In particular the change impacts
the design of the DBus transport defined in the protocol specification,
as signalling of both acknowledgeable and non-acknowledgeable events
could not be made atomic.

A particular case where this matters is when the daemon is terminated,
at which point it should simultaneously clear BMC_EVENT_DAEMON_READY and
set BMC_EVENT_PROTOCOL_RESET. The DBus interface as designed required
this be done as two separate messages, which lead to races propagating
the complete state update to the host during shutdown of ipmid.

Change-Id: Iaf38f77c28b8e4e4dd092b0de97dc7e777bfac65
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Mark the protocol as reset on shutdown

This is necessary for the host firmware to properly recover from a
daemon restart event, as it needs to re-perform the GET_INFO handshake
and re-establi

mboxd: Mark the protocol as reset on shutdown

This is necessary for the host firmware to properly recover from a
daemon restart event, as it needs to re-perform the GET_INFO handshake
and re-establish any window it had active prior to the daemon
restarting.

While we're here, rename the symbol to align with the documentation.

Change-Id: I628d2ee5972177b7ad78392a86122d16104e7011
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

mboxd: Broadcast the daemon is ready on all transports

The code as it stood only sent the state update at startup on the active
transport, which is somewhat arbitrarily chosen as an implementation
d

mboxd: Broadcast the daemon is ready on all transports

The code as it stood only sent the state update at startup on the active
transport, which is somewhat arbitrarily chosen as an implementation
detail of the mbox initialisation function.

If the host firmware is using IPMI, it will not learn of the update
unless it attempts to contact mboxd, which it won't do if it knows the
daemon isn't there, which it may have learned of by receiving a state
update from the daemon's shutdown path. In this circumstance the host
firmware is now stuck.

Relieve the host firmware of this problem by always sending the daemon
state on all supported transports. To avoid some insanity we introduce a
new callback in struct transport_ops that allows use to send the BMC's
entire event state rather than just set or clear updates.

Change-Id: I094ff4089eeebd8be99fbd343b94f7bbef023fb1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

daemon: Remove the concept of triggering or suppressing events

Rather, trigger them (or not) as necessary in the relevant code-paths.
This ensures that any call to one of protocol_events_{set,clear}

daemon: Remove the concept of triggering or suppressing events

Rather, trigger them (or not) as necessary in the relevant code-paths.
This ensures that any call to one of protocol_events_{set,clear}()
actually has a consequence that we can set about dealing with in the
transport layer.

Change-Id: If64733fa53ed9def0da8330c99cbe48327bab934
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

transport: dbus: Add get_info

Change-Id: I59cf0155b3f8600527cca23a14f41ef7d916a831
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

mboxd: Refactor and rename mbox.h to mboxd.h

Refine the purpose of the header file to represent what's required for
the daemon itself, not its constituent pieces. Rather, split those
definitions out

mboxd: Refactor and rename mbox.h to mboxd.h

Refine the purpose of the header file to represent what's required for
the daemon itself, not its constituent pieces. Rather, split those
definitions out to their respective header files and include them as
necessary.

Finally the header file is renamed to better reflect its purpose.

Change-Id: I48c409f57d96c844589cd865b24f197477dfe87c
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

transport: mbox: Rename free_mbox_dev to transport_mbox_free

Change-Id: Ie400ccfada8890ddc626deadabd58fe4bf4c3cd3
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

transport: mbox: Rename *init_mbox_dev to *transport_mbox_init

Change-Id: I212277ed2462089b905546ef048308d24a5489d0
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

transport: mbox: Rename dispatch_mbox to transport_mbox_dispatch

Change-Id: I7c5457099e90ea4ed498fd1898b4504f051d13fc
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

protocol: Provide abstraction over event notification

How this works will be transport-dependent. Move the event notification
helpers into the protocol abstraction and call-back through the
register

protocol: Provide abstraction over event notification

How this works will be transport-dependent. Move the event notification
helpers into the protocol abstraction and call-back through the
registered flush handler as necessary.

Change-Id: I29e3a9a9785b92de46a2b2750257fb7f8480a184
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

protocol: Add get_info

Change-Id: Ie3338714813bb65f5d37fcd046dd5bebc0ba21f0
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

mboxd: Rename mboxd_msg to transport_mbox

Change-Id: I0f4e206648c02a98fa938a7e582470c8d90904f1
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>