mboxd: Drop unused sdbusplus configuration

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

README: Add build documentation

Particularly, demonstrate how to avoid linking with a C++ compiler if
not configured with `--enable-virtual-pnor`.

Change-Id: I7fd0da535cd436085c2adcd3cdd058f1fc0762

README: Add build documentation

Particularly, demonstrate how to avoid linking with a C++ compiler if
not configured with `--enable-virtual-pnor`.

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

show more ...

configure: Don't enable the virtual PNOR by default

To date, most systems are not configured with the virtual PNOR feature.
Turn it off by default, so only systems that want it enable it.

See vpnor

configure: Don't enable the virtual PNOR by default

To date, most systems are not configured with the virtual PNOR feature.
Turn it off by default, so only systems that want it enable it.

See vpnor/README.md to understand whether this is a feature you want for
your system design.

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

show more ...

mbox: Forward-declare struct mbox_context

Avoid compilation errors such as:

$ make
make all-am
make[1]: Entering directory '/home/andrew/src/openbmc/phosphor-mboxd'
CC mbox

mbox: Forward-declare struct mbox_context

Avoid compilation errors such as:

$ make
make all-am
make[1]: Entering directory '/home/andrew/src/openbmc/phosphor-mboxd'
CC mboxd-common.o
CC mboxd-mboxd_dbus.o
CC mboxd-mboxd_lpc.o
CC mboxd-mboxd_msg.o
CC mboxd-mboxd_windows.o
CC mboxd-mtd.o
CC mboxd-mboxd_flash.o
CC mboxctl-mboxctl.o
CC mboxd-mboxd_lpc_reset.o
In file included from mboxd_dbus.c:28:0:
mbox.h:133:42: error: ‘struct mbox_context’ declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
typedef int (*mboxd_mbox_handler)(struct mbox_context *, union mbox_regs *,
^~~~~~~~~~~~
cc1: all warnings being treated as errors
Makefile:2201: recipe for target 'mboxd-mboxd_dbus.o' failed
make[1]: *** [mboxd-mboxd_dbus.o] Error 1
make[1]: *** Waiting for unfinished jobs....

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

show more ...

tests: Ensure we don't disable asserts

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

Add MAINTAINERS file

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

Add MAINTAINERS file

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

vpnor: Catch ReasonedError in vpnor_copy_bootloader_partition

It was observed that mboxd cored on systems that didn't have a virtual
PNOR deployed:

```
May 09 10:53:48 witherspoon systemd[1]: Start

vpnor: Catch ReasonedError in vpnor_copy_bootloader_partition

It was observed that mboxd cored on systems that didn't have a virtual
PNOR deployed:

```
May 09 10:53:48 witherspoon systemd[1]: Starting Updates symlinks for active PNOR version...
May 09 10:53:49 witherspoon obmc-flash-bios[1238]: ls: /media/pnor-ro-*: No such file or directory
May 09 10:53:49 witherspoon systemd[1]: Started Updates symlinks for active PNOR version.
May 09 10:53:49 witherspoon systemd[1]: Started Phosphor MBOX Daemon.
May 09 10:53:51 witherspoon mboxd[1242]: terminate called after throwing an instance of 'openpower::virtual_pnor::UnknownPartition'
May 09 10:53:51 witherspoon mboxd[1242]: what(): Partition HBB is not listed in the table of contents
May 09 10:53:51 witherspoon systemd[1]: Created slice system-systemd\x2dcoredump.slice.
May 09 10:53:51 witherspoon systemd[1]: Started Process Core Dump (PID 1243/UID 0).
May 09 10:53:51 witherspoon phosphor-host-state-manager[1237]: Initial Host State will be Off
May 09 10:53:51 witherspoon phosphor-host-state-manager[1237]: External request to reset reboot count
May 09 10:53:51 witherspoon systemd[1]: Started Phosphor Host State Manager.
May 09 10:53:51 witherspoon systemd[1]: Started Witherspoon UART render controller.
May 09 10:53:53 witherspoon systemd[1]: Started Wait for /xyz/openbmc_project/state/host0.
May 09 10:53:53 witherspoon systemd[1]: Starting Reset host sensors...
May 09 10:53:53 witherspoon systemd[1]: Starting Reboot If Enabled...
May 09 10:53:53 witherspoon systemd[1]: Starting Phosphor Time Manager daemon...
May 09 10:53:55 witherspoon systemd[1]: mboxd.service: Main process exited, code=dumped, status=6/ABRT
May 09 10:53:56 witherspoon systemd[1]: mboxd.service: Unit entered failed state.
May 09 10:53:56 witherspoon systemd[1]: mboxd.service: Failed with result 'core-dump'.
```

The ToC parsing and partition lookup functions can throw a variety of
exceptions, though the ones we care about are all derived from
ReasonedError. Catch ReasonedError to avoid core-dumping mboxd when it is
launched without a virtual PNOR available or symlinks in place th point it to
the virtual PNOR root directory.

Further, explicitly ignore the failure in the start-up path. This keeps
mboxd alive, which will recover on receiving a RESET if the appropriate
images and symlinks have been put in place.

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

show more ...

vpnor: Honour return codes in reset_lpc()

Without inspecting the return codes we have no way to communicate up the
call stack whether the operation was successful.

Change-Id: If8af2995522901ec8fcfc

vpnor: Honour return codes in reset_lpc()

Without inspecting the return codes we have no way to communicate up the
call stack whether the operation was successful.

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

show more ...

vpnor: Fix formatting in pnor_partition.cpp

Found in the process of resolving openbmc/openbmc#3125.

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

vpnor: Fix formatting in pnor_partition.cpp

Found in the process of resolving openbmc/openbmc#3125.

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

show more ...

mboxd_msg: Fix overflow in get_suggested_timeout()

The suggested timeout is a two-byte unsigned value, but when introducing the
debug message it was stored in a one-byte unsigned value. This can lea

mboxd_msg: Fix overflow in get_suggested_timeout()

The suggested timeout is a two-byte unsigned value, but when introducing the
debug message it was stored in a one-byte unsigned value. This can lead
to overflow under some circumstances.

Change-Id: If4f86a8dc812ae27d703a10fb3d21ba3c2541e65
Fixes: 285195987721 ("mboxd: Introduce a new DEBUG log level")
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

show more ...

version: Update version to v2.1.1

The previous patch introduces a backwards compatible bug fix.

Bump the version to reflect this.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Cha

version: Update version to v2.1.1

The previous patch introduces a backwards compatible bug fix.

Bump the version to reflect this.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Change-Id: If864822bbe0ef923cae43fda3313f4a43254bf60

show more ...

vpnor: Add README describing the expected behaviours

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

test: vpnor: Use MAP_SHARED for read-only file memory map

It's not well defined whether a private mapping of a resource that is
MAP_SHARED on a separate file descriptor will see the updates done via

test: vpnor: Use MAP_SHARED for read-only file memory map

It's not well defined whether a private mapping of a resource that is
MAP_SHARED on a separate file descriptor will see the updates done via
the shared mapping, though under Linux if the private mapping is not
written then we can expect updates from the shared mapping to propagate
(copy-on-write). However, we get a concrete guarantee of the desired
behaviour (observing the effects of the write) if we use MAP_SHARED.

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

show more ...

vpnor: Add handler for CREATE_WRITE_WINDOW

The virtual PNOR implementation enforces the read-only attribute of FFS
partitions, which is a departure from how things were handled
previously. In the pa

vpnor: Add handler for CREATE_WRITE_WINDOW

The virtual PNOR implementation enforces the read-only attribute of FFS
partitions, which is a departure from how things were handled
previously. In the past it was purely up to the host to respect the
flags set on the partition, but nothing prevented the host from
modifying it. Now it's possible for errors to occur when the host
attempts to flush changes back to the flash: mboxd can deny the change.
This denial can happen in a number of circumstances:

1. An explicit WRITE_FLUSH command from the host
2. An implicit WRITE_FLUSH via an explicit CLOSE_WINDOW command
3. An implicit WRITE_FLUSH via CREATE_{READ,WRITE}_WINDOW, which happens
via the implicit CLOSE_WINDOW

All of these attempts will fail if the write to the currently open
window cannot be allowed to succeed. Failing to open a read window due
to failure to flush pending writes is particularly painful, as we are
not able to ever successfully open a window again.

Instead, detect when the host attempts to open a write window over a
anything but a writeable partition. If this case is detected, return an
error for the CREATE_WRITE_WINDOW operation to prevent systemic failures
later on.

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

show more ...

vpnor: Configure a handler table in init_vpnor()

Currently the table just mirrors that which is set by the regular
implementation, however getting to that point requires massaging the
code a little.

vpnor: Configure a handler table in init_vpnor()

Currently the table just mirrors that which is set by the regular
implementation, however getting to that point requires massaging the
code a little. Thus, separate out this change from one that changes the
behaviour of the commands to improve the readability of the latter
change.

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

show more ...

mbox_msg: Move handler table to struct mbox_context

This allows us to provide alternative implementations for the handlers
as necessary. The vpnor feature, which enforces the read-only property
of F

mbox_msg: Move handler table to struct mbox_context

This allows us to provide alternative implementations for the handlers
as necessary. The vpnor feature, which enforces the read-only property
of FFS partitions, requires this for handling CREATE_WRITE_WINDOW.

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

show more ...

test: Move vpnor tests to vpnor directory

In the spirit of things that are together should be kept together. The
repository layout now better corresponds to upstream with the exception
of the vpnor

test: Move vpnor tests to vpnor directory

In the spirit of things that are together should be kept together. The
repository layout now better corresponds to upstream with the exception
of the vpnor directory and some modifications to Makefile.am

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

show more ...

vpnor: Isolate relevant code in vpnor directory

This is prepatory work for introducing more vpnor-specific behaviours to
window handling. We will be introducing more objects to link, in order
to hoo

vpnor: Isolate relevant code in vpnor directory

This is prepatory work for introducing more vpnor-specific behaviours to
window handling. We will be introducing more objects to link, in order
to hook some of the window command handlers.

This change takes the opportunity to revert back to the upstream names
for some of the original C files.

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

show more ...

test: vpnor: Add create_write_window_unmapped

Deny attempts to open write windows to flash space that is unmapped in
the ToC. This gives explicit feedback that any data written would not be
persiste

test: vpnor: Add create_write_window_unmapped

Deny attempts to open write windows to flash space that is unmapped in
the ToC. This gives explicit feedback that any data written would not be
persisted if it were possible create the write window in the first
place.

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

show more ...

test: vpnor: Add create_write_window_rw_partition

This test case should always pass. Ensure it does in the face of
modifications to how the request is processed.

Change-Id: I090aa6518750615c6b93140

test: vpnor: Add create_write_window_rw_partition

This test case should always pass. Ensure it does in the face of
modifications to how the request is processed.

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

show more ...

test: vpnor: Add create_write_window_ro_partition

The virtual PNOR implementation enforces the read-only attribute of
partitions out of the box. This causes trouble when the host requests a
write wi

test: vpnor: Add create_write_window_ro_partition

The virtual PNOR implementation enforces the read-only attribute of
partitions out of the box. This causes trouble when the host requests a
write window over a read-only partition, as the flush command will fail.
Further, by design, we have open-implies-close-implies-flush semantics,
which means once a flush fails, any subsequent request to open a window
also fails.

We want the daemon to deny attempts to open write windows over a
read-only partition during the CREATE_WRITE_WINDOW request, to avoid
the cascading failures later on.

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

show more ...

mboxd_windows: Shrink windows accessing the end of flash

The host may request a window over the end of the flash where the window
size combined with the requested offset exceeds the limit of the fla

mboxd_windows: Shrink windows accessing the end of flash

The host may request a window over the end of the flash where the window
size combined with the requested offset exceeds the limit of the flash.
This issue was introduced with the virtual PNOR, as copy_flash() now may
return a size less than requested. This leads to offset requests that
are still block aligned, but the windows may no longer be aligned with
respect to the flash size.

This issue triggers the read error reported from the Petitboot
environment in an earlier commit message:

/ # cat /dev/mtd0 > /dev/null
[ 501.061616288,3] MBOX-FLASH: Bad response code from BMC 2
[ 501.150405995,3] MBOX-FLASH: Error waiting for BMC
cat: read error: Input/output error
/ # echo $?
1
/ #

With the corresponding mboxd trace on the BMC:

[ 1519966031.652036815] Received MBOX command: 4
[ 1519966031.652272613] Host requested flash @ 0x03f1a000
[ 1519966031.652411603] Tried to open read window past flash limit
[ 1519966031.652500088] Couldn't create window mapping for offset 0x03f1a000
[ 1519966031.652607966] Error handling mbox cmd: 4
[ 1519966031.652661421] Writing MBOX response: 2
[ 1519966031.652762229] Error handling MBOX event

Instead, shrink the request such that the resulting window exactly maps
the flash limit, and no further.

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

show more ...

mboxd_windows: Reset evicted windows

After copying a portion of the backing store to a window,
create_map_window() "resizes" the window to the aligned-up size reported
by copy_flash(). This allows u

mboxd_windows: Reset evicted windows

After copying a portion of the backing store to a window,
create_map_window() "resizes" the window to the aligned-up size reported
by copy_flash(). This allows use of the window size as the content size
elsewhere in the codebase.

However, if we needed to evict a window to satisfy a request, the window
properties were not reset. This lead to inefficient use of the reserved
memory by limiting the effective window size to the minimum size of all
requests that were previously allocated the window in question.

Inefficient use of reserved memory isn't the only side effect; the host
takes an eye-watering hit to throughput that gets exponentionally
worse over time:

From the petitboot shell without the patch applied:

/ # time cat /dev/mtd0 > /dev/null
real 0m 49.77s
user 0m 0.00s
sys 0m 49.76s
/ # time cat /dev/mtd0 > /dev/null
real 1m 33.57s
user 0m 0.00s
sys 1m 33.55s
/ # time cat /dev/mtd0 > /dev/null
real 4m 45.37s
user 0m 0.00s
sys 4m 45.35s
/ # time cat /dev/mtd0 > /dev/null
real 9m 17.77s
user 0m 0.00s
sys 9m 17.76s
/ #

And with the patch applied:

/ # time cat /dev/mtd0 > /dev/null
real 0m 43.00s
user 0m 0.00s
sys 0m 42.99s
/ # time cat /dev/mtd0 > /dev/null
real 0m 42.40s
user 0m 0.00s
sys 0m 42.39s
/ # time cat /dev/mtd0 > /dev/null
real 0m 42.41s
user 0m 0.00s
sys 0m 42.39s
/ #

Reset the properties to allow use of the entire reserved memory region
allocated to the window, improving memory efficiency, throughput, and
minimising throughput variance.

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

show more ...

test: vpnor: Add create_read_window_remap

Sanity check that requesting arbitrary offsets inside a partition will
map to an existing window containing that partitions data. This ensures
we don't have

test: vpnor: Add create_read_window_remap

Sanity check that requesting arbitrary offsets inside a partition will
map to an existing window containing that partitions data. This ensures
we don't have multiple windows mapping the same content and shooting
ourselves in the foot with coherency issues.

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

show more ...