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 ...

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: 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 ...

flash: Introduce flash_validate()

Clean up the protocol_negotiate_version() mess. The existing approach
came about due to viewing the vpnor implementation as an edge case in
its own right. The code

flash: Introduce flash_validate()

Clean up the protocol_negotiate_version() mess. The existing approach
came about due to viewing the vpnor implementation as an edge case in
its own right. The code becomes much neater if we consider all backends
as equal and afford them the callbacks necessary for correct behaviour.

Change-Id: Ifaeee9da459818cf22b2f137ddc5b8d0356b9be9
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: 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 ...

transport: Fix event handling

Events were not quite being handled as per the intent of the recent
refactor: The protocol layer was meant to record the raw set of events
and provide the protocol-vers

transport: Fix event handling

Events were not quite being handled as per the intent of the recent
refactor: The protocol layer was meant to record the raw set of events
and provide the protocol-version-specific mask to the transport layer,
which the transport layer would then use to flush out the state in
accordance with its implementation-specific requirements.

What was going wrong was that the transport implementations were
overwriting the raw set of events with the protocol-specific masked set
of events, meaning that we'd lose the raw state and provide an
incomplete BMC state value on protocol upgrade.

Rework the event handling to make sure the responsibilities are properly
split between the layers.

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

show more ...

transport: Rework BMC event handling

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

transport: Retain knowledge of setting and clearing of events

The protocol layer now just filters the events based on the version of
the protocol in use, and leaves it to the transport layer to mana

transport: Retain knowledge of setting and clearing of events

The protocol layer now just filters the events based on the version of
the protocol in use, and leaves it to the transport layer to manage how
the resulting state is represented. For the moment this simply moves
manipulation of bmc_events in struct mbox_context down in to the
transport layer.

Change-Id: Iff1df934505dc9c769be3d376396d425fb4e8264
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 ...

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 ...

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 ...

flash: Move access rate estimate from transport header

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

transport: mbox: Drop handler functions from header

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

vpnor: Hijack protocol rather than transport

By hijacking the transport the changes in behaviour were limited to the
mailbox interface. Now that we have a DBus interface as well this would
lead to i

vpnor: Hijack protocol rather than transport

By hijacking the transport the changes in behaviour were limited to the
mailbox interface. Now that we have a DBus interface as well this would
lead to inconsistent behaviour dependent on the transport.

Instead of hooking the transport, push the hook down to the protocol
level where we will achieve consistent behaviour across all transports.

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

show more ...

protocol: Add ack

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

protocol: Add close

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

protocol: Add flush

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

protocol: Add erase

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

protocol: Add mark_dirty

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

protocol: Add create_write_window

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

protocol: Add create_read_window

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

protocol: Add get_flash_info

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

protocol: Add reset

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