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



Reply via email to