| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Some BMC mbox implementations seem to sometimes mysteriously fail when trying
to negotiate v3 when they only support v2. To work around this, we
can fall back to requesting lower mbox protocol versions until we find
one that works.
In theory, this should already "just work", but we have a counter example,
which this patch fixes.
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
This makes the mbox unit test run 300x quicker and seems to
shave about 6 seconds from boot time on Witherspoon.
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
A first basic set of tests for mbox-flash. These tests do their testing
by stubbing out or otherwise replacing functions not in
libflash/mbox-flash.c. The stubbed out version of the function can then
be used to emulate a BMC mbox daemon talking to back to the code in
mbox-flash and it can ensure that there is some adherence to the
protocol and that from a blocklevel api point of view the world appears
sane.
This makes these tests simple to run and they have been integrated into
`make check`. The down side is that these tests rely on duplicated
feature incomplete BMC daemon behaviour. Therefore these tests are a
strong indicator of broken behaviour but a very unreliable indicator of
correctness.
Full integration tests with a 'real' BMC daemon are probably beyond the
scope of this repository.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
[stewart: fix TESTS_LOOPS printf]
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
hw/lpc-mbox no longer requires that the memory associated with messages
exist for the lifetime of the message. Once it has been sent to the BMC,
that is bmc_mbox_enqueue() returns, lpc-mbox does not need the message
to continue to exist. On the reciving side, lpc-mbox will ensure that a
message exists for the receving callback function.
Remove all code to deal with allocating messages.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Currently when mbox-flash decides that a message times out the driver
has no way of knowing to drop the message and will continue waiting for
a response indefinitely preventing more messages from ever being sent.
This is a problem if the BMC crashes or has some other issue where it
won't ever respond to our outstanding message.
This patch provides a method for mbox-flash to tell the driver how long
it should wait before it no longer needs to care about the response.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
The MBOX protocol states that if an open window command fails then all
open windows are closed. Currently, if an open window command fails
mbox-flash will erroneously assume that the previously open window is
still open.
The solution to this is to mark all windows as closed before issuing an
open window command and then on success we'll mark the new window as
open.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Add the misc flag clear_on_ecc_error to libflash/pflash. This was
the only missing flag. The generator of the virtual pnor image
relies on libflash/pflash to provide the partition information,
so all flags are needed to build an accurate virtual pnor partition
table.
Signed-off-by: Adriana Kobylak <anoo@linux.vnet.ibm.com>
Reviewed-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
It is possible to erase within a single erase block. Currently the
pflash code assumes that if the erase starts part way into an erase
block it is because it needs to be aligned up to the boundary with the
next erase block.
Doing an erase smaller than a single erase block will cause underflows
and looping forever on erase.
Fixes: ae6cb86c2 ("external/pflash: Reinstate the progress bars")
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
To calculate the ECC syndrome we need to calculate the parity of a 64bit
number. On non-powerpc platforms we use the gcc builtin function
__builtin_parityl() to do this calculation. This is broken on 32bit ARM
where sizeof(unsigned long) is four bytes. Using __builtin_parityll()
instead cures this.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
The volatile flag was added to the pnor image to
indicate partitions that are cleared during a host
power off. Display this flag from the pflash command.
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
while this is PR_DEBUG, and we shouldn't be printing it to the
console, we do because of a long standing bug in how we do
log priorities. So, for the moment at least, just don't print
it at all.
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Currently we don't move the buffer along for a short read() or write()
and nor do we request only the remaining amount.
Fixes: c7c3a4cd53d libflash/file: Add a file access backend to for the blocklevel interface.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
==8304==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x7f70eda8f850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x408ba0 in main libflash/test/test-blocklevel.c:298
#2 0x7f70ec8e1509 in __libc_start_main (/lib64/libc.so.6+0x20509)
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
LeakSanitizer caught this with libflash/test/test-flash.c:
Direct leak of 4096 byte(s) in 1 object(s) allocated from:
#0 0x7f72546ee850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x405ff0 in flash_init libflash/test/../libflash.c:830
#2 0x408632 in main libflash/test/test-flash.c:382
#3 0x7f7253540509 in __libc_start_main (/lib64/libc.so.6+0x20509)
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
LeakSanitizer spotted this:
Direct leak of 131072 byte(s) in 1 object(s) allocated from:
#0 0x7fb99e42b850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x408612 in main libflash/test/test-flash.c:380
#2 0x7fb99d27d509 in __libc_start_main (/lib64/libc.so.6+0x20509)
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
If we extend the ECC protection array and subsequently decide to merge
regions in one call then there would be a use after free bug. While this
exists in theory and was caught by Coverity, it should never happen
since we only merge regions if we're low on space but the cause of the
use after free is due to having just created more space.
Nevertheless, this is the kind of ticking timebomb that simply requires
some code rearrangement or different 'optimisations' to become possible.
Best to just make it impossible.
Fixes CID 145924
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
libflash/libflash.c:
line 369: chunk = 0x100 - (d & 0xff);
line 370: if (chunk > 0x100)
At condition chunk > 256U, the value of chunk must be between 1 and 256.
Fixes CID 97821
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Fixes: CID 142226 (#1 of 1):
overflow_before_widen: Potentially overflowing expression
`1 << mbox_flash->shift` with type int (32 bits, signed) is evaluated using
32-bit arithmetic, and then used in a context that expects an expression
of type uint64_t (64 bits, unsigned).
Fixes: CID 142226
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
New code that is very much pflash functionality was added in commit
f2c87a3d2f6 "pflash option to retrieve PNOR partition flags".
Unfortunately at the time there wasn't an easy way to test pflash.
The previous patch adds pflash infrastructure and plumbs it into
`make check` nicely. This commit converts the tests originally added to
libflash tests.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Currently the FFS header/TOC generation code requires that consumers
know the size of their TOC beforehand. While this may be advantageous in
some circumstances if there are known limitations of other software. It
should not be a requirement.
Knowing the size of the FFS header/TOC partially breaks the abstraction
since it would require consumers of the library to be aware of/have some
idea of the on flash structure and size.
Future work may introduce functions to force sizes but the default
behaviour should be to calculate it behind the scenes.
This patch also addresses an off by one issue in checking for TOC
overflow.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Unfortunately not all drivers are created equal and several drivers on
which pflash relies block in the kernel for quite some time and ignore
signals.
This is really only a problem if pflash is to perform large erases. So
don't, perform these ops in small chunks.
An in kernel fix is possible in most cases but it takes time and systems
will be running older drivers for quite some time. Since sector erases
aren't significantly slower than whole chip erases there isn't much of a
performance penalty to breaking up the erase ioctl()s.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Also add usage text to pflash.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
mostly missing prototypes and unused parameters.
Reviewed-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
This commit extends pflash with an option to retrieve and print
information for a particular partition, including the content from
"pflash -i" and a verbose list of set miscellaneous flags. -i option
is also updated to print a short list of flags in addition to the
ECC flag, with one character per flag. A test of the new option is
included in libflash/test.
Signed-off-by: Michael Tritz <mtritz@us.ibm.com>
Reviewed-by: Cyril Bur <cyril.bur@au1.ibm.com>
[stewart@linux.vnet.ibm.com: various test fixes, enable gcov]
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
On writing ffs entries to flash libffs doesn't zero checksum words
before calculating the checksum across the entire structure. This causes
an inaccurate calculation of the checksum as it may calculate a checksum
on non-zero checksum bytes.
This patch solves this by zeroing the entire structure which is to be
written to the flash before calculating the checksum across the struct.
Fixes: 602dee45 libflash/libffs: Rework libffs
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
It would return success when the part wasn't found
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
libffs has been updating FFS partition information in the wrong place
which leads to incomplete erases and corruption.
Fixes: 602dee45 libflash/libffs: Rework libffs
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
In the bail-out path we call ffs_close() to tear down the partially
initialised ffs_handle. ffs_close() expects the entries list to be
initialised so we need to do that earlier to prevent a null pointer
dereference.
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Version two of the mbox-flash protocol defines a new command:
MARK_WRITE_ERASED.
This command provides a simple way to mark a region of flash as all 0xff
without the need to go and write all 0xff. This is an optimisation as
there is no need for an erase before a write, it is the responsibility of
the BMC to deal with the flash correctly, however in v1 it was ambiguous
what a client should do if the flash should be erased but not actually
written to. This allows of a optimal path to resolve this problem.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Acked-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Updated version 2 of the protocol can be found at:
https://github.com/openbmc/mboxbridge/blob/master/Documentation/mbox_protocol.md
This commit changes mbox-flash such that it will preferentially talk
version 2 to any capable daemon but still remain capable of talking to
v1 daemons.
Version two changes some of the command definitions for increased
consistency and usability.
Version two includes more attention bits - these are now dealt with at a
simple level.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Acked-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
- Warn if flushing with closed write window.
- Call msg_free_memory() in mbox_flash_init() before a successful
return. No leak is present as the current allocation theme is from
static memory. However as this is likely to change in the future,
best to ensure that msg_free_memory() is called after every
allocation.
- Fix bug where len argument may be incorrect in mark dirty command.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Acked-by: Cyril Bur <cyri.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
We recently made MTD 64 bit safe in e5720d3fe94 which now requires the
64 bit MTD erase ioctl. Unfortunately this ioctl is not present in
older kernels used by some BMC vendors that use pflash.
This patch addresses this by only using the 64bit version of the erase
ioctl() if the parameters exceed 32bit in size.
If an erase requires the 64bit ioctl() on a kernel which does not
support it, the code will still attempt it. There is no way of knowing
beforehand if the kernel supports it. The ioctl() will fail and an error
will be returned from from the function.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Michael Neuling <mikey@neuling.org>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
With recent changes to flash drivers in linux not all erase blocks are
4K anymore. While most level of the pflash/gard tool stacks were written
to not mind, it turns out there are bugs which means not 4K erase block
backing stores aren't handled all that well. Part of the problem is the
FFS layout that is 4K aligned and with larger block sizes pflash and the
gard tool don't check if their erase commands are erase block aligned -
which they are usually not with 64K erase blocks.
This patch aims to add common functionality to blocklevel so that (at
least) pflash and the gard tool don't need to worry about the problem
anymore.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
pflash -i is currently broken due to this commit
commit 602dee4505cd0ceb5b69f056ec403f982b585791
Author: Cyril Bur <cyril.bur@au1.ibm.com>
libflash/libffs: Rework libffs
It's output doesn't correctly detect the last partition and continues
printing forever.
This fixes it by returning null when we don't find a partition in
ffs_get_part().
Signed-off-by: Michael Neuling <mikey@neuling.org>
Acked-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
This patch attempts a rework of libffs to prepare it for future changes.
Firstly the types are split in two:
1. Packed, big endian structures used to map exactly how the data is on flash.
2. CPU endian, sane valued, not packed structures used to manipulate FFS data.
Secondly:
The packed struct can use BE types so that in future tools like sparse can be
run over the code to check for endian conversion bugs.
Thirdly:
defines of sizeof(struct ...) were removed for clarity.
Finally:
For ease of manipulation, the in memory FFS structures contain a linked list of
entries as this will make addition and removal operations much easier.
This patch should be invisible to consumers of libffs.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
The use MBOX protocol to request flash access from the BMC. Then
read/write to the 'flash' through windows it creates on LPC FW space.
Reference implementation of the mbox flash daemon for BMC userspace:
https://github.com/cyrilbur-ibm/mboxbridge
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
A format string had a 0x prefix on an integer but printed in decimal.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
While we'll 'never' have flash chips larger than 32bit, work was
recently done to blocklevel for it to be 64bit compatible for other
backends.
Since there is a 64bit ioctl() lets use it. There is currently no known
case where 32bit is not sufficient but this doesn't mean someone might
not do something strange in the future.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
These OPAL calls are exposing the host flash chip to linux as a flash
device. The host is expected to understand that it is raw flash and use
it appropriately, it isn't skiboots place to strip ecc bytes or perform
erase before writes.
Over the course of other developments blocklevel has gotten quite clever
but in this case the cleverness is inappropriate.
Fixes: https://github.com/open-power/skiboot/issues/44
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
|
Currently the policy for calling ECC protecting a range at the
blocklevel layer is that the requested region be completely
unprotected otherwise the call will return an error. It turns out that
duplicate calls to ffs_init() with true as the last parameter (for the
same blocklevel structure) will cause duplicate attempts to
ecc_protect() ranges.
Change the policy within blocklevel to allow duplicate protecting.
In fact the new policy almost guarantees no failure (baring something
odd like malloc() failing). It will detect that the range is currently
already fully protected and do nothing, detect that part of the range
is or is not and extend the existing range or detect that a range fits
perfectly between two ranges in which case it will merge the ranges.
Also adjust tests to match the new policy.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
|
