summaryrefslogtreecommitdiffstats
path: root/libflash
Commit message (Collapse)AuthorAgeFilesLines
...
* test-ipmi-hiomap: Ensure the completion code is setAndrew Jeffery2019-02-211-0/+2
| | | | | | | | | | ipmi_queue_msg_sync() intercepts and implements the IPMI transfers for the test scenarios. In some scenarios we want to return IPMI error codes, so make sure the msg->cc field is set. Cc: stable Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* test-ipmi-hiomap: Dump unexpected IPMI messagesAndrew Jeffery2019-02-211-0/+3
| | | | | | | | | These indicate an implementation bug or broken scenario. Either way it's helpful to know what arrived given it wasn't expected. Cc: stable Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* test-ipmi-hiomap: Add ability to delay some IPMI messagesAndrew Jeffery2019-02-211-1/+24
| | | | | | | | | | | The initial implementation delivered all BMC-initiated SELs immediately after the delivery of the last response to the host. In some circumstances we want slightly more control over how this works, so introduce a means to manually advance the scenario in the test case. Cc: stable Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* ffspart, libflash: Fix stack size warningsAndrew Jeffery2019-02-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | libflash/file.c: In function 'file_erase': libflash/file.c:134:1: error: the frame size of 4128 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] } ^ and ffspart.c: In function ‘main’: ffspart.c:529:1: error: the frame size of 4864 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] } ^ In both cases, mark the local variables as static to avoid the stack. The static approach is valid for file.c as the buffer is always filled with `~0`. Given it's now going to be in .bss due to static we have to still perform the memset(), but racing memset()s in this fashion won't be harmful, just wasteful. For ffspart.c's main(), there are bigger problems if that needs to be re-entrant. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/test: Generate header dependencies for testsAndrew Jeffery2019-02-211-1/+4
| | | | | | Cc: stable Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ecc: Fix compilation warningVasant Hegde2019-02-181-1/+1
| | | | | | | | | | | | | | | | | | We are hitting below warning on gcc9. gcc -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mcpu=power8 -mtune=power8 -fasynchronous-unwind-tables -fstack-clash-protection -O2 -Wall -Werror -Wno-stringop-truncation -I. -c libflash/ecc.c -o libflash-ecc.o libflash/ecc.c: In function 'memcpy_to_ecc_unaligned': libflash/ecc.c:419:24: error: taking address of packed member of 'struct ecc64' may result in an unaligned pointer value [-Werror=address-of-packed-member] 419 | memcpy(inc_uint64_by(&ecc_word.data, alignment), src, bytes_wanted); | ^~~~~~~~~~~~~~ libflash/ecc.c:448:24: error: taking address of packed member of 'struct ecc64' may result in an unaligned pointer value [-Werror=address-of-packed-member] 448 | memcpy(inc_uint64_by(&ecc_word.data, len), inc_ecc64_by(dst, len), | ^~~~~~~~~~~~~~ cc1: all warnings being treated as errors Fixes: https://github.com/open-power/skiboot/issues/218 Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/file: greatly increase perf of file_erase()Stewart Smith2018-12-111-3/+5
| | | | | | | | | | Do 4096 byte chunks not 8 byte chunks. A ffspart invocation constructing a 64MB PNOR goes from a couple of seconds to ~0.1seconds with this patch. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash: Don't merge ECC-protected rangesSamuel Mendoza-Jonas2018-11-212-41/+3
| | | | | | | | | | | | | | | | | | | | | | | Libflash currently merges contiguous ECC-protected ranges, but doesn't check that the ECC bytes at the end of the first and start of the second range actually match sanely. More importantly, if blocklevel_read() is called with a position at the start of a partition that is contained somewhere within a region that has been merged it will update the position assuming ECC wasn't being accounted for. This results in the position being somewhere well after the actual start of the partition which is incorrect. For now, remove the code merging ranges. This means more ranges must be held and checked however it prevents incorrectly reading ECC-correct regions like below: [ 174.334119453,7] FLASH: CAPP partition has ECC [ 174.437349574,3] ECC: uncorrectable error: ffffffffffffffff ff [ 174.437426306,3] FLASH: failed to read the first 0x1000 from CAPP partition, rc 14 [ 174.439919343,3] CAPP: Error loading ucode lid. index=201d1 Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash: Restore blocklevel testsSamuel Mendoza-Jonas2018-11-211-0/+5
| | | | | | | | | This fell out in f58be46 "libflash/test: Rewrite Makefile.check to improve scalability". Add it back in as test-blocklevel. Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com> Acked-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* hiomap: quieten warning on failing to move a windowStewart Smith2018-11-081-1/+1
| | | | | | | | | | | | | | This isn't *necessarily* an error that we should complain loudly about. If, for example, the BMC enforces the Read Only flag on a FFS partition, opening a write window *should* fail, and we do indeed test this in op-test. Thus we deal with the error in a well known path: returning an error code and then it's eventually a userspace problem. Signed-off-by: Stewart Smith <stewart@linux.ibm.com> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Respect daemon presence and flash controlAndrew Jeffery2018-11-082-1/+326
| | | | | | | | | | | | | | | | Fix the fix of ORing in the BMC state - we only want to retain state covered by the ack mask as this is something we still need to handle. Critically, we must not retain state not covered by the ack mask as this may lead to host firmware attempting to communicate with a dead daemon or attempting to access the PNOR whilst the daemon is not in control of the flash. Further, add unit tests to capture the desired (and now implemented) behaviour. Fixes: 34cffed2ccf3 ("libflash/ipmi-hiomap: Improve event handling") Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Add support for unit testsAndrew Jeffery2018-11-086-31/+374
| | | | | | | | | | Lay the ground work for unit testing the ipmi-hiomap implementation. The design hooks a subset of the IPMI interface to move through a data-driven "scenario" of IPMI message exchanges. Two basic tests are added exercising the initialsation path of the protocol implementation. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Fix argument type warning on x86-64Andrew Jeffery2018-11-081-1/+1
| | | | | | | | | | | | | | | | | | | | | | | libflash/ipmi-hiomap.c: In function ‘hiomap_window_move’: libflash/ipmi-hiomap.c:17:21: error: format ‘%llu’ expects argument of type ‘long long unsigned int’, but argument 3 has type ‘uint64_t’ {aka ‘long unsigned int’} [-Werror=format=] #define pr_fmt(fmt) "HIOMAP: " fmt ^~~~~~~~~~ include/skiboot.h:93:41: note: in expansion of macro ‘pr_fmt’ #define prlog(l, f, ...) do { _prlog(l, pr_fmt(f), ##__VA_ARGS__); } while(0) ^~~~~~ include/skiboot.h:94:30: note: in expansion of macro ‘prlog’ #define prerror(fmt...) do { prlog(PR_ERR, fmt); } while(0) ^~~~~ libflash/ipmi-hiomap.c:291:3: note: in expansion of macro ‘prerror’ prerror("Invalid window properties: len: %llu, size: %llu\n", ^~~~~~~ libflash/ipmi-hiomap.c:291:47: note: format string is defined here prerror("Invalid window properties: len: %llu, size: %llu\n", ~~~^ %lu Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/test: Rewrite Makefile.check to improve scalabilityAndrew Jeffery2018-11-081-26/+137
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The current implementation makes it hard to expand the list of tests if we want to build anything that doesn't link to mbox-server. This is a consequence of embedding the $(LIBFLASH_TEST_EXTRA) variable inside the recipes for building test executables, which makes the makefile a bit of a maze to navigate. To address this we could go the route of duplicating the $(LIBFLASH_TEST), $(LIBFLASH_TEST_EXTRA) and the corresponding make directives (targets/prerequisites/recipes) each time we want to link a binary against a new set of objects, but that seems ham-fisted. Further, $(LIBFLASH_TEST_EXTRA) is defined in terms of the relevant object (.o) files, but the recipes it is used in otherwise use source (.c) paths for compilation. These other paths are typically to non-test code that needs to be compiled into the test executable, but we can't use object files at the usual path because we will typically have a conflict of architectures (PPC64 for the skiboot object, x86_64 for the test object). This in turn means that we will compile source files multiple times (once for each test binary it is required in) rather than re-using an existing object file. Further, the current structure of the Makefile requires we #include the .c file under test directly into the test source if we want it in a specific test case due to the relationship of the prerequisites to the build (only the first source prerequisite is included in the build). The include-the-c-file approach can have some annoying side-effects with respect to macros, typically errors regarding redefinition. While it is useful for testing static functions in the source under test, it would be nice if this approach was optional rather than required. This change attempts to address all of these issues. The outcome is we have precise control of which objects get linked into each test binary, we avoid the architecture clash problem, we re-use existing compiled objects (avoiding recompilation), and we make the include-the-c-file approach optional. The general approach is to generate a new directory hierarchy of object files under a `$(HOSTCC) -dumpmachine` directory in the repository root and use these for linking the test cases. Objects that land in this segregated tree are described by a _SOURCES variable for each test, similar in structure and behaviour to automake's _SOURCES variables. Again similar to automake, a check_PROGRAMS variable is used that describes the path of each test binary to be built. The test binary paths are mapped to the corresponding _SOURCES variable by some secondary-evaluation wizardry that no-one has to pay any attention to once it is written. Whilst the implementation is perhaps slightly tricky, it allows us to avoid the recipe headache of unconditionally linking in objects defined in variables that don't directly participate in the target's prerequisites, and so prevents the explosion of variables as we implement tests that require disjoint sets of dependencies. This is initially intended as an isolated experiment with the libflash test makefile, but it's feasible that the scope of the concept could be expanded to other test Makefiles. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Use error codes rather than abort()Andrew Jeffery2018-11-011-6/+10
| | | | | | | | | | Admittedly the situations are pretty dire, and usually indicate a programming failure on the BMC's part, but abort() seems a bit over the top. The technique was useful for development but shouldn't have made it into production. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Restore window state on window/protocol resetAndrew Jeffery2018-11-011-3/+39
| | | | | | | | | | | | | | | The initial implementation of ipmi-hiomap left a bit to be desired when it came to event handling: it didn't completely restore the state of the system to what it was before events like a hiomap protocol or window reset take place. The result is the host cannot recover from e.g. the BMC being rebooted underneath it. Take the only step required in the event of window reset, or the final step after performing the handshake in the event of a protocol reset, and re-open the previously active window if there was one. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Improve event handlingAndrew Jeffery2018-11-011-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The host firmware side of the hiomap protocol has two input sources: 1. Requests to adjust the flash mappings from itself or the kernel 2. State change events received from the BMC The handling of BMC state change events (2.) is asynchronous in two ways: a. The BMC pushes the state change event to the host, which is recorded but not acted on b. When handling requests to adjust the flash mapping, skiboot first addresses any new BMC state changes before servicing the mapping request Further, the hiomap protocol sends a mix of ackable and stateful events, where ackable events are only relevant until skiboot's hiomap event handler (b. above) cleans them up, whereas stateful events persist until the BMC provides a subsequent state change event. As we handle the ackable events asynchronous to receiving notification (b. vs a. above), OR in the received event state rather than directly assign to ensure we don't lose events that we must not miss. As an example, without the OR we may lose ackable events if the daemon restarts and pushes a new state change event during initialisation, which will necessarily bear no relation to the previous state change event value. Similarly, don't close active windows in a. based on the event content, as we need the window type information to handle state restoration in b. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ipmi-hiomap: Cleanup allocation on init failureAndrew Jeffery2018-11-011-4/+12
| | | | | | | | | Previously we were leaking the memory pointed by ctx if an IPMI error occurred during protocol initialisation. Make sure we free the memory if an error occurs. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* hiomap: fix missing newline at end of 'Flushing writes' prlog()Stewart Smith2018-10-301-1/+1
| | | | | Fixes: 529bdca0bc546a7ae3ecbd2c3134b7260072d8b0 Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* hiomap: free ipmi message in callbackStewart Smith2018-10-251-0/+3
| | | | | | | | Otherwise we'd slowly leak memory on each hiomap operation. Fixes: 529bdca0bc546a7ae3ecbd2c3134b7260072d8b0 Tested-by: Andrew Jeffery <andrew@aj.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* platform: Restructure bmc_platform typeAndrew Jeffery2018-10-111-15/+16
| | | | | | | | | | Segregate the BMC platform configuration into hardware and software components. This allows population of platform default values for hardware configuration that may no-longer be accessible by the host. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> [stewart: fixup pci-quirk unit test] Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* astbmc: Prefer ipmi-hiomap for PNOR accessAndrew Jeffery2018-10-112-0/+10
| | | | | | | | | If the IPMI command is not available, fall back to the mailbox interface. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> [stewart: fix up mbox test] Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash: Add ipmi-hiomapAndrew Jeffery2018-10-113-1/+884
| | | | | | | | | | | | | | | | | | | | | | ipmi-hiomap implements the PNOR access control protocol formerly known as "the mbox protocol" but uses IPMI instead of the AST LPC mailbox as a transport. As there is no-longer any mailbox involved in this alternate implementation the old protocol name is quite misleading, and so it has been renamed to "the hiomap protoocol" (Host I/O Mapping protocol). The same commands and events are used though this client-side implementation assumes v2 of the protocol is supported by the BMC. The code is a heavily-reworked copy of the mbox-flash source and is introduced this way to allow for the mbox implementation's eventual removal. mbox-flash should in theory be renamed to mbox-hiomap for consistency, but as it is on life-support effective immediately we may as well just remove it entirely when the time is right. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> [stewart: prlog debug over prerror for mbox fallback, fix indent] Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash: quieten our loggingStewart Smith2018-06-182-3/+1
| | | | | Suggested-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash: fix gcov buildStewart Smith2018-06-051-1/+1
| | | | Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/blocklevel.c: Remove unused store to ecc_lenBalbir singh2018-05-241-1/+1
| | | | | | | | Caught by scan-build. We rewrite ecc_len with a different value prior to use Signed-off-by: Balbir singh <bsingharora@gmail.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/blocklevel_write: Fix missing error handlingBalbir singh2018-05-241-0/+2
| | | | | | | | Caught by scan-build, we seem to trap the errors in rc, but not take any recovery action during blocklevel_write. Signed-off-by: Balbir singh <bsingharora@gmail.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* mbox/flash: Remove dead codeBalbir singh2018-05-241-3/+0
| | | | | | | | Caught by scan-build, attn is passed in by value and modified but not read after that. Signed-off-by: Balbir singh <bsingharora@gmail.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ecc: Remove hand rolled parity asmJoel Stanley2018-05-041-17/+1
| | | | | | | | | | | | | | | | | | | | | | | | We get the same code generation using the builtin: GCC 7.3: a3584: 7d 29 00 f4 popcntb r9,r9 a3588: 7d 29 01 74 prtyd r9,r9 GCC 6.3: a0bfc: 7d 29 00 f4 popcntb r9,r9 a0c00: 7d 29 01 74 prtyd r9,r9 Clang 7 (and clang 6): bd48c: 7c e7 03 f4 popcntd r7,r7 bd490: 54 e7 07 fe clrlwi r7,r7,31 (Not sure why the clang builtin generates different code) Signed-off-by: Joel Stanley <joel@jms.id.au> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/blocklevel: Add missing newline to debug messagesPridhiviraj Paidipeddi2018-04-151-2/+2
| | | | | Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/blocklevel: Make read/write be ECC agnostic for callersCyril Bur2018-04-092-36/+350
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The blocklevel abstraction allows for regions of the backing store to be marked as ECC protected so that blocklevel can decode/encode the ECC bytes into the buffer automatically without the caller having to be ECC aware. Unfortunately this abstraction is far from perfect, this is only useful if reads and writes are performed at the start of the ECC region or in some circumstances at an ECC aligned position - which requires the caller be aware of the ECC regions. The problem that has arisen is that the blocklevel abstraction is initialised somewhere but when it is later called the caller is unaware if ECC exists in the region it wants to arbitrarily read and write to. This should not have been a problem since blocklevel knows. Currently misaligned reads will fail ECC checks and misaligned writes will overwrite ECC bytes and the backing store will become corrupted. This patch add the smarts to blocklevel_read() and blocklevel_write() to cope with the problem. Note that ECC can always be bypassed by calling blocklevel_raw_() functions. All this work means that the gard tool can can safely call blocklevel_read() and blocklevel_write() and as long as the blocklevel knows of the presence of ECC then it will deal with all cases. This also commit removes code in the gard tool which compensated for inadequacies no longer present in blocklevel. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Tested-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> [stewart: core/flash: Adapt to new libflash ECC API Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/blocklevel: Return region start from ecc_protected()Cyril Bur2018-04-092-22/+63
| | | | | | | | | | | | | | Currently all ecc_protected() does is say if a region is ECC protected or not. Knowing a region is ECC protected is one thing but there isn't much that can be done afterwards if this is the only known fact. A lot more can be done if the caller is told where the ECC region begins. Knowing where the ECC region start it allows to caller to align its read/and writes. This allows for more flexibility calling read and write without knowing exactly how the backing store is organised. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ecc: Add helpers to align a position within an ecc bufferCyril Bur2018-04-092-0/+36
| | | | | | | | | | As part of ongoing work to make ECC invisible to higher levels up the stack this function converts a 'position' which should be ECC agnostic to the equivalent position within an ECC region starting at a specified location. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/ecc: Add functions to deal with unaligned ECC memcpyCyril Bur2018-04-092-17/+234
| | | | | Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libffs: Fix bad checks for partition overlapCyril Bur2018-04-091-9/+20
| | | | | | | Not all TOCs are written at zero Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* external/ffspart: Use new interfaceCyril Bur2018-04-091-1/+4
| | | | | | | This also updated the pflash tests which use ffspart to generate pnors Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Allow caller to specifiy header partitionCyril Bur2018-04-093-10/+21
| | | | | | | | | | | | | | | | | | | | | | | | | An FFS TOC is comprised of two parts. A small header which has a magic and very minimmal information about the TOC which will be common to all partitions, things like number of patritions, block sizes and the like. Following this small header are a series of entries. Importantly there is always an entry which encompases the TOC its self, this is usually called the 'part' partition. Currently libffs always assumes that the 'part' partition is at zero. While there is always a TOC and zero there doesn't actually have to be. PNORs may have multiple TOCs within them, therefore libffs needs to be flexible enough to allow callers to specify TOCs not at zero. The 'part' partition is otherwise a regular partition which may have flags associated with it. libffs should allow the user to set the flags for the 'part' partition. This patch achieves both by allowing the caller to specify the 'part' partition. The caller can not and libffs will provide a sensible default. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Refcount ffs entriesCyril Bur2018-04-093-8/+36
| | | | | | | | | | | | | | | | | Currently consumers can add an new ffs entry to multiple headers, this is fine but freeing any of the headers will cause the entry to be freed, this causes double free problems. Even if only one header is uses, the consumer of the library still has a reference to the entry, which they may well reuse at some other point. libffs will now refcount entries and only free when there are no more references. This patch also removes the pointless return value of ffs_hdr_free() Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Switch to storing header entries in an arrayCyril Bur2018-04-092-66/+70
| | | | | | | | | Since the libffs no longer needs to sort the entries as they get added it makes little sense to have the complexity of a linked list when an array will suffice. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Remove backup partition from TOC generation codeCyril Bur2018-04-093-55/+1
| | | | | | | | | | | | It turns out this code was messy and not all that reliable. Doing it at the library level adds complexity to the library and restrictions to the caller. A simpler approach can be achived with the just instantiating multiple ffs_header structures pointing to different parts of the same file. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Remove the 'sides' from the FFS TOC generation codeCyril Bur2018-04-093-78/+4
| | | | | | | | | | | | It turns out this code was messy and not all that reliable. Doing it at the library level adds complexity to the library and restrictions to the caller. A simpler approach can be achived with the just instantiating multiple ffs_header structures pointing to different parts of the same file. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Always add entries to the end of the TOCCyril Bur2018-04-091-19/+1
| | | | | | | | | It turns out that sorted order isn't the best idea. This removes flexibility from the caller. If the user wants their partitions in sorted order, they should insert them in sorted order. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: ffs_close() should use ffs_hdr_free()Cyril Bur2018-04-091-20/+20
| | | | | Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libflash/libffs: Add setter for a partitions actual sizeCyril Bur2018-04-092-0/+15
| | | | | Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* libffs: Standardise ffs partition flagsCyril Bur2018-04-093-1/+114
| | | | | | | | | | | | It seems we've developed a character respresentation for ffs partition flags. Currently only pflash really prints them so it hasn't been a problem but now ffspart wants to read them in from user input. It is important that what libffs reads and what pflash prints remain consistent, we should move the code into libffs to avoid problems. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.ibm.com>
* mbox: Reduce default BMC timeoutsCyril Bur2018-03-271-4/+3
| | | | | | | | | | | | | | | | | | | Rebooting a BMC can take 70 seconds. Skiboot cannot possibly spin for 70 seconds waiting for a BMC to come back. This also makes the current default of 30 seconds a bit pointless, is it far too short to be a worse case wait time but too long to avoid hitting hardlockup detectors and wrecking havoc inside host linux. Just change it to three seconds so that host linux will survive and that, reads and writes will fail but at least the host stays up. Also refactored the waiting loop just a bit so that it's easier to read. Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Tested-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Reviewed-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
* mbox: Harden against BMC daemon errorsCyril Bur2018-03-271-0/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bugs present in the BMC daemon mean that skiboot gets presented with mbox windows of size zero. These windows cannot be valid and skiboot already detects these conditions. Currently skiboot warns quite strongly about the occurrence of these problems. The problem for skiboot is that it doesn't take any action. Initially I wanting to avoid putting policy like this into skiboot but since these bugs aren't going away and skiboot barfing is leading to lockups and ultimately the host going down something needs to be done. I propose that when we detect the problem we fail the mbox call and punt the problem back up to Linux. I don't like it but at least it will cause errors to cascade and won't bring the host down. I'm not sure how Linux is supposed to detect this or what it can even do but this is better than a crash. Diagnosing a failure to boot if skiboot its self fails to read flash may be marginally more difficult with this patch. This is because skiboot will now only print one warning about the zero sized window rather than continuously spitting it out. Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Tested-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Reviewed-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
* build: use thin archives rather than incremental linkingNicholas Piggin2018-02-281-1/+1
| | | | | | | | | | | | | | | | | | | | This changes to build system to use thin archives rather than incremental linking for built-in.o, similar to recent change to Linux. built-in.o is renamed to built-in.a, and is created as a thin archive with no index, for speed and size. All built-in.a are aggregated into a skiboot.tmp.a which is a thin archive built with an index, making it suitable or linking. This is input into the final link. The advantags of build size and linker code placement flexibility are not as great with skiboot as a bigger project like Linux, but it's a conceptually better way to build, and is more compatible with link time optimisation in toolchains which might be interesting for skiboot particularly for size reductions. Size of build tree before this patch is 34.4MB, afterwards 23.1MB. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
* libflash/blocklevel: Correct miscalculation in blocklevel_smart_erase()Cyril Bur2018-02-281-1/+1
| | | | | | | | | | | | | | If blocklevel_smart_erase() detects that the smart erase fits entire in one erase block, it has an early bail path. In this path it miscaculates where in the buffer the backend needs to read from to perform the final write. Fixes: d6a5b53f ("libflash/blocklevel: Add blocklevel_smart_erase()") Fixes: https://github.com/open-power/skiboot/issues/151 Reported-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Tested-by: Pridhiviraj Paidipeddi <ppaidipe@linux.vnet.ibm.com> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> Signed-off-by: Stewart Smith <stewart@linux.vnet.ibm.com>
* libflash/mbox-flash: fallback to requesting lower MBOX versions from BMCStewart Smith2017-12-201-0/+7
| | | | | | | | | | | | 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>
OpenPOWER on IntegriCloud