| Message ID | 20190510044459.9612-1-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | opal-gard: Account for ECC size when clearingpartition | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (e6e70ea51c6594290602e76102149ee6dcfc66ee) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On Fri, 10 May 2019 at 04:45, Oliver O'Halloran <oohall@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@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@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@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
"Oliver O'Halloran" <oohall@gmail.com> writes: > 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@gmail.com> > --- > external/gard/gard.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Merged to master as of 27d1ef2ebabb07a1958b94049cac3d90a101c3d5, and should no doubt also go to stable.
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");
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@gmail.com> --- external/gard/gard.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)