pnor_partition_table: Parse all miscellaneous user flags from TOC
Add support for the remaining miscellaneous flags specified in the Hostboot FFS
header, specifically the CLEARECC flag:
https://github.com/open-power/hostboot/blob/master/src/usr/pnor/common/ffs_hb.H
This requires a change to how the properties string was being parsed so we can
differentiate between the ECC and CLEARECC options.
In detail:
Issue openbmc/openbmc#2704 captures a failure where Hostboot detects flash
corruption on an FFS partition with ECC enabled. In some instances,
configuration of FFS partitions allows Hostboot to "fix" ECC errors by clearing
the entire partition and trying again. In this case, the attempt fails:
--== Welcome to Hostboot hostboot-e223708/hbicore.bin ==--
4.01970|secure|SecureROM valid - enabling functionality
4.01974|secure|Booting in non-secure mode.
5.93358|ISTEP 6. 5 - host_init_fsi
6.06661|ISTEP 6. 6 - host_set_ipl_parms
6.07597|ECC error in PNOR flash in section offset 0x00026000
6.07604|System shutting down with error status 0x60F
Looking at the TOC, we find 0x26000 is in HBEL:
# cat /tmp/pnor.toc
version=IBM-witherspoon-ibm-OP9_v1.19_1.121
extended_version=op-build-v1.19-326-g20bf99a-dirty,buildroot-2017.08-8-g5e23247,skiboot-v5.9.8,hostboot-7f4ced1,linux-4.13.16-openpower1-p686edfa,petitboot-v1.6.3-p191b3ea,machine-xml-758eb02,occ-84f3564,hostboot-binaries-38248b4,capp-ucode-p9-dd2-v2,sbe-99e2fe2
partition00=part,0x00000000,0x00001000,00,READWRITE
partition01=HBEL,0x00008000,0x0002c000,00,ECC,REPROVISION,READWRITE
partition02=GUARD,0x0002c000,0x00031000,00,ECC,PRESERVED,REPROVISION
Except, Dan Crowell identified that the reported offset is merely *indirectly*
related to the offending location in the PNOR layout description:
1) There is a bug in the openbmc code that causes them to remove space from
our pnor layout. We don't think that has a functional problem but it does
confuse things. The number pointed out in the printk was pointing me at
the HBEL partition but the problem is really inside the GUARD partition.
Adriana is going to be fixing this issue.
The GUARD partition requires the ClearOnEccErr flag[1], however Hostboot's
output indicates this action isn't taken; phosphor-mboxd failed to add this
information to the dynamically generated TOC data structure presented to
Hostboot.
Thus, add support for all miscellaneous flags to the ToC generator, allowing
the ClearOnEccErr (CLEARECC) flag to be propagated.
[1] https://github.com/open-power/pnor/blob/22a9eadc0b2afbd2aca1e054faa2cca90e7760c2/p9Layouts/defaultPnorLayout_64.xml#L90
The bug was confirmed on a Witherspoon system by creating a dummy GUARD
partition from /dev/urandom and beginning IPL. Leaving the dummy GUARD in
place, mboxd was replaced with the patched build and the host rebooted.
Hostboot successfully cleared the partition and triggered a reboot, then
successfully booted to Petitboot.
Resolves openbmc/openbmc#2704
Tested: As described above
Change-Id: I21c9bbc60b8c503194fcea03e74ab1d08aff57fe
Signed-off-by: Adriana Kobylak <anoo@us.ibm.com>
[arj: Reworked the commit message to describe the bug and fallout]
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/pnor_partition_defs.h b/pnor_partition_defs.h
index fe2a338..ab2ca6b 100644
--- a/pnor_partition_defs.h
+++ b/pnor_partition_defs.h
@@ -28,11 +28,22 @@
/* The partition structure has 16 'user data' words, which can be used to store
* miscellaneous information. This is typically used to store bits that state
* whether a partition is ECC protected, is read-only, is preserved across
- * updates, etc. */
+ * updates, etc.
+ *
+ * TODO: Replace with libflash (!) or at least refactor the data structures to
+ * better match hostboot's layout[1]. The latter would avoid the headache of
+ * verifying these flags match the expected functionality (taking into account
+ * changes in endianness).
+ *
+ * [1] https://github.com/open-power/hostboot/blob/9acfce99596f12dcc60952f8506a77e542609cbf/src/usr/pnor/common/ffs_hb.H#L81
+ */
#define PARTITION_USER_WORDS 16
#define PARTITION_ECC_PROTECTED 0x8000
#define PARTITION_PRESERVED 0x00800000
#define PARTITION_READONLY 0x00400000
+#define PARTITION_REPROVISION 0x00100000
+#define PARTITION_VOLATILE 0x00080000
+#define PARTITION_CLEARECC 0x00040000
#define PARTITION_VERSION_CHECK_SHA512 0x80000000
#define PARTITION_VERSION_CHECK_SHA512_PER_EC 0x40000000
diff --git a/pnor_partition_table.cpp b/pnor_partition_table.cpp
index eafdca5..bb9e05d 100644
--- a/pnor_partition_table.cpp
+++ b/pnor_partition_table.cpp
@@ -104,20 +104,38 @@
uint32_t version,
const std::string& data)
{
- if (std::string::npos != data.find("ECC"))
+ std::istringstream stream(data);
+ std::string flag {};
+ auto perms = 0;
+
+ while (std::getline(stream, flag, ','))
{
- part.data.user.data[0] = PARTITION_ECC_PROTECTED;
+ if (flag == "ECC")
+ {
+ part.data.user.data[0] = PARTITION_ECC_PROTECTED;
+ }
+ else if (flag == "READONLY")
+ {
+ perms |= PARTITION_READONLY;
+ }
+ else if (flag == "PRESERVED")
+ {
+ perms |= PARTITION_PRESERVED;
+ }
+ else if (flag == "REPROVISION")
+ {
+ perms |= PARTITION_REPROVISION;
+ }
+ else if (flag == "VOLATILE")
+ {
+ perms |= PARTITION_VOLATILE;
+ }
+ else if (flag == "CLEARECC")
+ {
+ perms |= PARTITION_CLEARECC;
+ }
}
- auto perms = 0;
- if (std::string::npos != data.find("READONLY"))
- {
- perms |= PARTITION_READONLY;
- }
- if (std::string::npos != data.find("PRESERVED"))
- {
- perms |= PARTITION_PRESERVED;
- }
part.data.user.data[1] = perms;
part.data.user.data[1] |= version;