[Skiboot] [PATCH v4 09/18] core/flash.c: add SECBOOT read and write support

[Oliver O'Halloran]

On Tue, May 12, 2020 at 7:35 AM Eric Richter <erichte at linux.ibm.com> wrote:
>
> From: Claudio Carvalho <cclaudio at linux.ibm.com>
>
> In secure boot enabled systems, the petitboot linux kernel verifies the
> OS kernel against x509 certificates that are wrapped in secure variables
> controlled by OPAL. These secure variables are stored in the PNOR SECBOOT
> partition, as well as the updates submitted for them using userspace
> tools.
>
> This patch adds read and write support to the PNOR SECBOOT partition in
> a similar fashion to that of NVRAM, so that OPAL can handle the secure
> variables.

Is there any reason to have this as a pile of platform callbacks
rather than calling the flash_read, etc from the storage driver? The
platform is already choosing the storage driver so handling it there
seems cleaner IMO. And I'd really like it if we could stop solving
every problem by adding callbacks to the platform structure...

*snip*

> +static int flash_secboot_probe(struct flash *flash, struct ffs_handle *ffs)
> +{
> +       uint32_t start, size, part;
> +       bool ecc;
> +       int rc;
> +
> +       prlog(PR_DEBUG, "FLASH: probing for SECBOOT\n");
> +
> +       rc = ffs_lookup_part(ffs, "SECBOOT", &part);
> +       if (rc) {
> +               prlog(PR_WARNING, "FLASH: no SECBOOT partition found\n");
> +               return OPAL_HARDWARE;
> +       }
> +
> +       rc = ffs_part_info(ffs, part, NULL,
> +                          &start, &size, NULL, &ecc);
> +       if (rc) {
> +               /**
> +                * @fwts-label SECBOOTNoPartition
> +                * @fwts-advice OPAL could not find an SECBOOT partition
> +                *     on the system flash. Check that the system flash
> +                *     has a valid partition table, and that the firmware
> +                *     build process has added a SECBOOT partition.
> +                */
> +               prlog(PR_ERR, "FLASH: Can't parse ffs info for SECBOOT\n");
> +               return OPAL_HARDWARE;
> +       }
> +
> +       secboot_flash = flash;
> +       secboot_offset = start;
> +       secboot_size = ecc ? ecc_buffer_size_minus_ecc(size) : size;

IMO the sec var partition should always be ECC protected. Even if the
contents of flash is fine we have seen errors introduced while
transferring data over the LPC bus and if a single bit is wrong we'll
the wrong hash.