[PATCH] env: spi: Fix gd->env_valid for the first write
Michal Simek
michal.simek at amd.com
Thu Aug 21 08:47:05 CEST 2025
On 8/20/25 22:59, Marek Vasut wrote:
> On 8/19/25 4:25 PM, Michal Simek wrote:
> [...]
>
>> ZynqMP> mtd read "U-Boot storage variables" 1000
>
> It would be better to spray the 0x1000 area with pattern first, otherwise, if
> the 'mtd read' fails, such failure might go undetected.
can be done.
>
>> Reading 131072 byte(s) at offset 0x00000000
>> ZynqMP> md 1000
>> 00001000: ffffffff ffffffff ffffffff ffffffff ................
>> 00001010: ffffffff ffffffff ffffffff ffffffff ................
>> 00001020: ffffffff ffffffff ffffffff ffffffff ................
>
> [...]
>
>> ZynqMP> savee
>
> "env save" is the modern equivalent.
>
>> Saving Environment to SPIFlash... Erasing SPI flash...Writing to SPI flash...
>> offset 2200000
>> done
>> Valid environment: 2
>> OK
>> ZynqMP> mtd read "U-Boot storage variables" 1000
>> Reading 131072 byte(s) at offset 0x00000000
>
> This here looks odd:
> - Where is the "offset 2200000" print coming from ?
I had additional debugs in the code when this output was created.
diff --git a/env/sf.c b/env/sf.c
index 0b70e18b9afa..961e853eaef7 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -127,7 +127,7 @@ static int env_sf_save(void)
if (ret)
goto done;
- puts("Writing to SPI flash...");
+ printf("Writing to SPI flash... offset %x\n", env_new_offset);
> - The valid environment is indicated to be 2 , shouldn't that be 1
> (primary copy = 1, redundant copy = 2) ?
yes it should be 1 but this is the code before this patch is applied and it is
saying 2 but actual write is happening to 1.
> - The dump of what looks like primary copy also seems valid, so
> shouldn't Valid environment be 1 (see previous bullet)
yes - dump is valid because the first write is happening to primary copy but
logic itself is saying that it was saved to redundant copy. And that's exactly
what my patch is fixing.
>
>> ZynqMP> md 1000
>> 00001000: 5d691224 63726101 72613d68 7561006d $.i].arch=arm.au
>> 00001010: 6f6c6f74 6e3d6461 6162006f 61726475 toload=no.baudra
>> 00001020: 313d6574 30323531 6f620030 3d647261 te=115200.board=
>> 00001030: 716e797a 6200706d 6472616f 6e616d5f zynqmp.board_man
>> 00001040: 63616675 65727574 49583d72 584e494c ufacturer=XILINX
>> 00001050: 616f6200 6e5f6472 3d656d61 4b2d4d53 .board_name=SM-K
>> 00001060: 582d3632 47324c43 44452d43 616f6200 26-XCL2GC-ED.boa
>> 00001070: 725f6472 423d7665 616f6200 735f6472 rd_rev=B.board_s
>> 00001080: 61697265 30353d6c 42323735 46313131 erial=50572B111F
>> 00001090: 62004832 6472616f 7465735f 723d7075 2H.board_setup=r
>> 000010a0: 64206374 30207665 797a203b 706d716e tc dev 0; zynqmp
>> 000010b0: 696d6d20 72775f6f 20657469 46467830 mmio_write 0xFF
>> 000010c0: 30304143 30203031 66666678 203b3020 CA0010 0xfff 0;
>> 000010d0: 74206669 20747365 61637b24 5f316472 if test ${card1_
>> 000010e0: 656d616e 203d207d 2d4b4353 472d564b name} = SCK-KV-G
>> 000010f0: 6874203b 72206e65 74206e75 6b5f6d70 ; then run tpm_k
>> ZynqMP> mtd read "U-Boot storage variables backup" 1000
>> Reading 131072 byte(s) at offset 0x00000000
>> ZynqMP> md 1000
>> 00001000: ffffffff ffffff00 ffffffff ffffffff ................
>> 00001010: ffffffff ffffffff ffffffff ffffffff ................
>> 00001020: ffffffff ffffffff ffffffff ffffffff ................
>> 00001030: ffffffff ffffffff ffffffff ffffffff ................
>> 00001040: ffffffff ffffffff ffffffff ffffffff ................
>> 00001050: ffffffff ffffffff ffffffff ffffffff ................
>> 00001060: ffffffff ffffffff ffffffff ffffffff ................
>> 00001070: ffffffff ffffffff ffffffff ffffffff ................
>> 00001080: ffffffff ffffffff ffffffff ffffffff ................
>> 00001090: ffffffff ffffffff ffffffff ffffffff ................
>> 000010a0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010b0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010c0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010d0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010e0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010f0: ffffffff ffffffff ffffffff ffffffff ................
>> ZynqMP> savee
>> Saving Environment to SPIFlash... Erasing SPI flash...Writing to SPI flash...
>> offset 2200000
>> done
>> Valid environment: 1
>> OK
>> ZynqMP> mtd read "U-Boot storage variables" 1000
>
> This behavior here looks correct .
yes for primary yes but secondary should be populated already based on log above
but it is not.
On the first save you should save variables to primary, on second save to
secondary.
As you see from logs. On the first save - the first location is used but u-boot
reports that it was saved to 2 redundant location. On second save it is saved to
the first location and also said that it is the first.
But after this patch applied. On the first save - the first is used (unchanged
behavior) and it is reported as the first (fix the report). On second saving the
second location is used (fix) and it is reported as 2 (fix).
>
>> Reading 131072 byte(s) at offset 0x00000000
>> ZynqMP> md 1000
>> 00001000: 5d691224 63726101 72613d68 7561006d $.i].arch=arm.au
>> 00001010: 6f6c6f74 6e3d6461 6162006f 61726475 toload=no.baudra
>> 00001020: 313d6574 30323531 6f620030 3d647261 te=115200.board=
>> 00001030: 716e797a 6200706d 6472616f 6e616d5f zynqmp.board_man
>> 00001040: 63616675 65727574 49583d72 584e494c ufacturer=XILINX
>> 00001050: 616f6200 6e5f6472 3d656d61 4b2d4d53 .board_name=SM-K
>> 00001060: 582d3632 47324c43 44452d43 616f6200 26-XCL2GC-ED.boa
>> 00001070: 725f6472 423d7665 616f6200 735f6472 rd_rev=B.board_s
>> 00001080: 61697265 30353d6c 42323735 46313131 erial=50572B111F
>> 00001090: 62004832 6472616f 7465735f 723d7075 2H.board_setup=r
>> 000010a0: 64206374 30207665 797a203b 706d716e tc dev 0; zynqmp
>> 000010b0: 696d6d20 72775f6f 20657469 46467830 mmio_write 0xFF
>> 000010c0: 30304143 30203031 66666678 203b3020 CA0010 0xfff 0;
>> 000010d0: 74206669 20747365 61637b24 5f316472 if test ${card1_
>> 000010e0: 656d616e 203d207d 2d4b4353 472d564b name} = SCK-KV-G
>> 000010f0: 6874203b 72206e65 74206e75 6b5f6d70 ; then run tpm_k
>> ZynqMP> mtd read "U-Boot storage variables backup" 1000
>> Reading 131072 byte(s) at offset 0x00000000
>> ZynqMP> md 1000
>> 00001000: ffffffff ffffff00 ffffffff ffffffff ................
>> 00001010: ffffffff ffffffff ffffffff ffffffff ................
>> 00001020: ffffffff ffffffff ffffffff ffffffff ................
>> 00001030: ffffffff ffffffff ffffffff ffffffff ................
>> 00001040: ffffffff ffffffff ffffffff ffffffff ................
>> 00001050: ffffffff ffffffff ffffffff ffffffff ................
>> 00001060: ffffffff ffffffff ffffffff ffffffff ................
>> 00001070: ffffffff ffffffff ffffffff ffffffff ................
>> 00001080: ffffffff ffffffff ffffffff ffffffff ................
>> 00001090: ffffffff ffffffff ffffffff ffffffff ................
>> 000010a0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010b0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010c0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010d0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010e0: ffffffff ffffffff ffffffff ffffffff ................
>> 000010f0: ffffffff ffffffff ffffffff ffffffff ................
>> ZynqMP> savee
>> Saving Environment to SPIFlash... Erasing SPI flash...Writing to SPI flash...
>> offset 2220000
>> done
>> Valid environment: 2
>> OK
>> ZynqMP> savee
>> Saving Environment to SPIFlash... Erasing SPI flash...Writing to SPI flash...
>> offset 2200000
>> done
>> Valid environment: 1
> It would be good to validate that both copies of the env are actually written
> where they are supposed to be written . This part is missing from this test. And
> in fact -- it would be good if this fix did have a proper test .
I can validate it again but even now log is too long now.
It is just demonstration that at start you get two saves to the same location.
When 2 saves happens behavior is correct.
I think it will be good if you can try it on any platform which this enabled to
see if you can demonstrate the same problem.
Love: Can you please take a look at if this can be added to pytest?
Thanks,
Michal
More information about the U-Boot
mailing list