[Skiboot] [PATCH] opal-gard: Account for ECC size when clearing partition

[Joel Stanley]

On Fri, 10 May 2019 at 04:45, Oliver O'Halloran <oohall at gmail.com> wrote:
>
> When 'opal-gard clear all' is run, it works by erasing the GUARD then
> using blockevel_smart_write() to write nothing to the partition. This
> second write call is needed because we rely on libflash to set the ECC
> bits appropriately when the partition contained ECCed data.
>
> The API for this is a little odd with the caller specifying how much
> actual data to write, and libflash writing size + size/8 bytes
> since there is one additional ECC byte for every eight bytes of data.
>
> We currently do not account for the extra space consumed by the ECC data
> in reset_partition() which is used to handle the 'clear all' command.
> Which results in the paritition following the GUARD partition being
> partially overwritten when the command is used. This patch fixes the
> problem by reducing the length we would normally write by the number
> of ECC bytes required.
>
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>

I tested this as follows:

$ wget https://openpower.xyz/job/openpower/job/openpower-op-build/label=slave,target=palmetto/lastSuccessfulBuild/artifact/images/palmetto.pnor
$ hexdump palmetto.pnor > orig
$ external/gard/gard -f palmetto.pnor clear all
$ hexdump palmetto.pnor > new
$ pflash -F palmetto.pnor -i |grep GUARD
ID=12           GUARD 0x00938000..0x0093d000 (actual=0x00005000) [E--P--F---]
$ diff orig new | tail -n1
> 093cff0 00ff ffff ffff ffff ffff ff00 ffff ffff

Before the patch the tool would clear data past 0x0093d000. Here we
can see that 093cff0 < 0x0093d000.

Tested-by: Joel Stanley <joel at jms.id.au>

This should go to all of our supported stable trees too.

Cheers,

Joel

> ---
>  external/gard/gard.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/external/gard/gard.c b/external/gard/gard.c
> index 16da76a3149a..ba15e877d1a9 100644
> --- a/external/gard/gard.c
> +++ b/external/gard/gard.c
> @@ -598,6 +598,7 @@ static int do_clear_i(struct gard_ctx *ctx, int pos, struct gard_record *gard, v
>
>  static int reset_partition(struct gard_ctx *ctx)
>  {
> +       int no_ecc_len = (ctx->gard_data_len / 9) * 8;
>         struct gard_record *gard;
>         int rc = 0;
>
> @@ -613,7 +614,7 @@ static int reset_partition(struct gard_ctx *ctx)
>                 goto out;
>         }
>
> -       rc = blocklevel_write(ctx->bl, ctx->gard_data_pos, gard, ctx->gard_data_len);
> +       rc = blocklevel_write(ctx->bl, ctx->gard_data_pos, gard, no_ecc_len);
>         if (rc)
>                 fprintf(stderr, "Couldn't reset the entire gard partition. Bailing out\n");
>
> --
> 2.20.1
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot