Testing showed that failing to free DBus message replies was leaking
800-900kiB per boot. After adding the appropriate unrefs, mboxd memory
consumption stayed stable at 71920kiB across multiple reboots of the
host.

The root cause was identified using a DBus capture of the host's IPMI
traffic during boot, then reducing the output to mboxd-specific messages
and turning them into commands that could be run with ipmitool. Adding
all of these commands to a script and running `pmap` between ipmitool
invocations showed the growth in memory usage across the "boot" process,
but this did not correlate with any particular set of commands to mboxd.
The lack of correlation lead to the hypothesis that we might be able to
reproduce by sending a lot of dbus messages, such as:

```
root@witherspoon:/tmp# for i in `seq 1 10000`; do \
busctl call xyz.openbmc_project.Hiomapd \
	/xyz/openbmc_project/Hiomapd \
	xyz.openbmc_project.Hiomapd.Protocol.V2 \
	GetInfo y 2; \
done
```

Spamming the daemon in this way demonstrated the growth in memory seen
during a regular boot process, confirming that just sending DBus
messages was enough.

Add the necessary unrefs for the replies at the end of each method
handler to ensure the replies are appropriately freed.

Testing and confirmation of the fix were performed on a Witherspoon
system.

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

GetDaemonState and GetLpcState were converted from methods to properties
and renamed to DaemonState and LpcState respectively in mboxd. mboxctl
was overlooked in the change, so update it to match.

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

The grass was greener :)

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

The tool refernced in table.hpp is renamed to generate-tar.
Update the comment in code.

Change-Id: I67f75dbe5018ab71d65f87ee14093c3972eb5846
Signed-off-by: Lei YU <mine260309@gmail.com>

Commit e50e654b ("Add --trace support (in blktrace format)") is a broken
version of commit ef0c8360 ("Add --trace support (in blktrace
format)") that appears in master's linear history. Merging e50e654b
preserves the ability to bisect the openbmc/openbmc respository.

The merge was performed as below, which discards any changes performed
in e50e654b:

> $ git merge -s ours e50e654b

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

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>

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>

The merge of phosphor-mboxd and mboxbridge blew away the content. No-one
has complained, so remove the broken file.

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

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

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

Better that we try to keep all the conditional junk near the top of the
tree.

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

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

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

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

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

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

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

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>

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>

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>

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>

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>

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>

If a host firmware image is provided where the placement of HBB exceeds
the reserved memory size then an out-of-bounds write would occur.

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

Otherwise we observe invalid memory accesses due to uninitialised
variables.

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

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

No need to try to exit gracefully in the test cases.

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

Include ordering and whether or not C linkage is forced by `extern "C"`
blocks can cause headaches at link time. Ensure that all C dependencies
are included in an `extern C` block before other includes occur. Also
include the C++ versions of string.h and assert.h

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

ax_pkg_check_modules provides a wrapper to pkg_check_modules, such that
one can specify private and public dependencies.  This is not used,
therefore use the underlying macro.

Change-Id: I556f9e29da236e069b7a9b69448fb4cbcba7bbe1
Signed-off-by: Patrick Venture <venture@google.com>

Use the defaults in the pkg check where the default error message is
sufficient to identify which package is missing.

Change-Id: Ide33a6c676d6b4db07a10aeb35e5076f9b3aeb91
Signed-off-by: Patrick Venture <venture@google.com>

These are replaced by the equivalent properties.

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

The virtual PNOR implementation never properly honoured writes to the
FFS ToC, as the ToC's binary representation is generated from the CSV
file shipped in the PNOR squashfs image.

What *did* happen was that opening a write window to the ToC region
succeeded and writes could be flushed, but the flushed writes were never
read, and ToC representation internal to mboxd was never updated to
match the written state. Thus the written values "persisted" until the
ToC's window fell out of the cache (with 64MiB reserved regions,
probably on a host reboot).

Short circuit the insanity of handling FFS more than we have to by
forcefully marking the ToC as read-only, regardless of the flag
configuration shipped in the CSV representation. This prevents the host
from successfully opening a write window and thus the host can have no
expectation of write persistence.

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

This ensures that the ToC presented to the host indicates that it is not
writable. The virtual PNOR implementation has never properly honoured
writes to the ToC, so lets at least tell the host.

As the code has not yet been fixed to implement the desired behaviour,
add the test to XFAIL_TESTS.

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

Tests if mutations survive re-opening the window. They don't, so add the
test to XFAIL_TESTS.

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

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>

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>

When building outside of OpenBMC, none of the C++ libraries are
required.

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

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>

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>

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>

The daemon implements the argument with a boolean type parameter but we
were passing a char from mboxctl, which makes the call fail.

Fix the parameter type in mboxctl to match mboxd.

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