`read firmware` command uses 4B mode for flash chips >32MB

[prockallsyms]

When attempting to ./culvert -vv read firmware, it seems as though everything is working as intended:

[root@localhost ~]# ./culvert -vv read firmware
[*] Found 5 registered bridge drivers
[*] Trying bridge driver l2a
[*] Failed to initialise L2A bridge: -95
[*] Trying bridge driver ilpc
[*] Probing ilpc
[*] Probing 0x2e for SuperIO
[*] Unlocking SuperIO: 0
[*] Selecting SuperIO device 2 (SUART1): 0
[*] Found device 255 selected: 0
[*] Locking SuperIO
[*] Probing 0x4e for SuperIO
[*] Unlocking SuperIO: 0
[*] Selecting SuperIO device 2 (SUART1): 0
[*] Found device 2 selected: 0
[*] Selecting SuperIO device 12 (SUART4): 0
[*] Found device 12 selected: 0
[*] Locking SuperIO
[*] Found SuperIO device at 0x4e
[*] Probing for SoC revision registers
[*] ahb_readl: 0x1e6e2004: 0xf70ea198
[*] ahb_readl: 0x1e6e207c: 0x04030303
[*] Found revision 0x4030303
[*] Trying bridge driver devmem
[*] failed to initialise devmem bridge: -1
[*] Trying bridge driver debug-uart
[*] Unrecognised argument list for debug interface (0)
[*] Trying bridge driver p2a
[*] Failed to initialise P2A bridge: -2
[*] Accessing the BMC's AHB via the ilpc bridge
[*] Probing for SoC revision registers
[*] ahb_readl: 0x1e6e2004: 0xf70ea198
[*] ahb_readl: 0x1e6e207c: 0x04030303
[*] Found revision 0x4030303
[*] Selected devicetree for SoC 'aspeed,ast2500'
[*] Found 16 registered drivers
[*] Processing devicetree node at /aliases
[*] Processing devicetree node at /memory@80000000
[*] Processing devicetree node at /ahb
[*] Processing devicetree node at /ahb/sram@1e720000
[*] Processing devicetree node at /ahb/bus-controller@1e600000
[*] Bound trace driver to /ahb/bus-controller@1e600000
[*] Processing devicetree node at /ahb/apb
[*] Processing devicetree node at /ahb/apb/spi@1e620000
[*] Bound sfc driver to /ahb/apb/spi@1e620000
[*] Processing devicetree node at /ahb/apb/spi@1e630000
[*] Bound sfc driver to /ahb/apb/spi@1e630000
[*] Processing devicetree node at /ahb/apb/spi@1e631000
[*] Bound sfc driver to /ahb/apb/spi@1e631000
[*] Processing devicetree node at /ahb/apb/memory-controller@1e6e0000
[*] Bound sdmc driver to /ahb/apb/memory-controller@1e6e0000
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000/clock
[*] Bound clk driver to /ahb/apb/syscon@1e6e2000/clock
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000/strapping
[*] Bound strap driver to /ahb/apb/syscon@1e6e2000/strapping
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000/superio
[*] Bound sioctl driver to /ahb/apb/syscon@1e6e2000/superio
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000/bridge-controller
[*] Bound bridge-controller driver to /ahb/apb/syscon@1e6e2000/bridge-controller
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000/debug-bridge-controller
[*] Bound debugctl driver to /ahb/apb/syscon@1e6e2000/debug-bridge-controller
[*] Processing devicetree node at /ahb/apb/syscon@1e6e2000/pcie-bridge-controller
[*] Bound pciectl driver to /ahb/apb/syscon@1e6e2000/pcie-bridge-controller
[*] Bound scu driver to /ahb/apb/syscon@1e6e2000
[*] Processing devicetree node at /ahb/apb/jtag@1e6e4000
[*] Bound jtag driver to /ahb/apb/jtag@1e6e4000
[*] Processing devicetree node at /ahb/apb/watchdog@1e785000
[*] Bound wdt driver to /ahb/apb/watchdog@1e785000
[*] Processing devicetree node at /ahb/apb/watchdog@1e785020
[*] Bound wdt driver to /ahb/apb/watchdog@1e785020
[*] Processing devicetree node at /ahb/apb/watchdog@1e785040
[*] Bound wdt driver to /ahb/apb/watchdog@1e785040
[*] Processing devicetree node at /ahb/apb/serial@1e787000
[*] Bound vuart driver to /ahb/apb/serial@1e787000
[*] Processing devicetree node at /ahb/apb/lpc@1e789000
[*] Processing devicetree node at /ahb/apb/lpc@1e789000/bridge-controller
[*] Bound ilpcctl driver to /ahb/apb/lpc@1e789000/bridge-controller
[*] Bound uart-mux driver to /ahb/apb/lpc@1e789000
[*] Initialising flash controller
[*] fdt: Looking up device name 'fmc'
[*] fdt: Locating node with device path '/ahb/apb/spi@1e620000'
[*] ahb_readl: 0x1e6e2000: 0x00000000
[*] Unlocking SCU
[*] ahb_writel: 0x1e6e2000: 0x1688a8a8
[*] Initialised scu driver
[*] Initialised clk driver
[*] ahb_readl: 0x1e6e2070: 0xf40f92be
[*] ahb_readl: 0x1e620010: 0x30bb2441
[*] ahb_readl: 0x1e620000: 0x8007002a
[*] ahb_writel: 0x1e620000: 0x8007002a
[*] ahb_writel: 0x1e620010: 0x00000400
[*] ahb_writel: 0x1e620094: 0x00000000
[*] Initialised sfc driver
[*] Initialising flash chip
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000403
[*] ahb_readl: 0x20000000: 0x00000000
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000400
[*] LIBFLASH: Init status: 00
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000403
[*] ahb_readl: 0x20000000: 0x001940ef
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000400
[*] LIBFLASH: Flash ID: ef.40.19 (ef4019)
[*] LIBFLASH: Found chip Winbond W25Q256BV size 32M erase granule: 4K
[*] LIBFLASH: Flash >16MB, enabling 4B mode...
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000403
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000400
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000403
[*] ahb_readl: 0x20000000: 0x02020202
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000400
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000403
[*] ahb_writel: 0x1e620010: 0x00000407
[*] ahb_writel: 0x1e620010: 0x00000400
[*] LIBFLASH: Enabling controller 4B mode...
[*] ahb_readl: 0x1e620004: 0x00000701
[*] ahb_writel: 0x1e620010: 0x00002400
[*] ahb_writel: 0x1e620004: 0x00000701
[*] Write-protecting all chip-selects
[*] ahb_readl: 0x1e620000: 0x8007002a
[*] ahb_writel: 0x1e620000: 0x8007002a
[*] Exfiltrating BMC flash to stdout

However, the resulting 32MB firmware blob seems to be a sequence of 4-byte repeats:

[root@localhost ~]# head -c 512 fw.bin | hexdump -C
00000000  15 15 15 15 14 14 14 14  14 14 14 14 14 14 14 14  |................|
00000010  14 14 14 14 14 14 14 14  14 14 14 14 14 14 14 14  |................|
00000020  c0 c0 c0 c0 20 20 20 20  80 80 80 80 e0 e0 e0 e0  |....    ........|
00000030  40 40 40 40 a0 a0 a0 a0  00 00 00 00 78 78 78 78  |@@@@........xxxx|
00000040  00 00 00 00 90 90 90 90  d8 d8 d8 d8 00 00 00 00  |................|
00000050  de de de de de de de de  de de de de 00 00 00 00  |................|
00000060  3f 3f 3f 3f d3 d3 d3 d3  00 00 00 00 00 00 00 00  |????............|
00000070  17 17 17 17 17 17 17 17  10 10 10 10 23 23 23 23  |............####|
00000080  87 87 87 87 02 02 02 02  01 01 01 01 03 03 03 03  |................|
00000090  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
000000a0  10 10 10 10 00 00 00 00  00 00 00 00 ef ef ef ef  |................|
000000b0  4a 4a 4a 4a 0e 0e 0e 0e  00 00 00 00 00 00 00 00  |JJJJ............|
000000c0  70 70 70 70 00 00 00 00  00 00 00 00 04 04 04 04  |pppp............|
000000d0  13 13 13 13 0d 0d 0d 0d  0f 0f 0f 0f 0e 0e 0e 0e  |................|
000000e0  48 48 48 48 ff ff ff ff  98 98 98 98 0c 0c 0c 0c  |HHHH............|
000000f0  48 48 48 48 34 34 34 34  0e 0e 0e 0e 0f 0f 0f 0f  |HHHH4444........|
00000100  0d 0d 0d 0d 9a 9a 9a 9a  00 00 00 00 00 00 00 00  |................|
00000110  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000120  04 04 04 04 00 00 00 00  d8 d8 d8 d8 00 00 00 00  |................|
00000130  00 00 00 00 04 04 04 04  00 00 00 00 00 00 00 00  |................|
00000140  04 04 04 04 48 48 48 48  ff ff ff ff fc fc fc fc  |....HHHH........|
00000150  0c 0c 0c 0c 48 48 48 48  34 34 34 34 0e 0e 0e 0e  |....HHHH4444....|
00000160  0f 0f 0f 0f 0d 0d 0d 0d  89 89 89 89 00 00 00 00  |................|
00000170  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
00000180  30 30 30 30 00 00 00 00  00 00 00 00 04 04 04 04  |0000............|
00000190  13 13 13 13 0d 0d 0d 0d  0f 0f 0f 0f 0e 0e 0e 0e  |................|
000001a0  48 48 48 48 ff ff ff ff  58 58 58 58 0c 0c 0c 0c  |HHHH....XXXX....|
000001b0  48 48 48 48 34 34 34 34  0e 0e 0e 0e 0f 0f 0f 0f  |HHHH4444........|
000001c0  0d 0d 0d 0d 7a 7a 7a 7a  00 00 00 00 00 00 00 00  |....zzzz........|
000001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
000001e0  90 90 90 90 00 00 00 00  00 00 00 00 04 04 04 04  |................|
000001f0  13 13 13 13 0d 0d 0d 0d  0f 0f 0f 0f 0e 0e 0e 0e  |................|

I was hoping to inquire what 4B mode is, and how this could possibly be affecting the firmware read process?

[amboar]

I was hoping to inquire what 4B mode is, and how this could possibly be affecting the firmware read process?

@prockallsyms 4B mode use 4-byte addressing, which allows complete reads of larger flashes. However, this mode is chip-specific and so needs to be enabled selectively.

More info in section 6.1.5 of the W25Q256FV datasheet (hopefully similar to the reported W25Q256BV chip?)

[prockallsyms]

@amboar Thanks for the speedy reply! Just perusing, I saw the definition for this chip on the following line:

culvert/src/flash.c

Line 28 in 40bb728

{ 0xef4019, 0x02000000, FL_ERASE_ALL | FL_ERASE_64K | FL_CAN_4B |

I (naively) tried commenting out the FL_CAN_4B and recompiling, but I'm running into the same issue. Do you know the correct implementation to disable 4B mode? Also, in doing so, could I harm the system (I'm only attempting a read)?

EDIT:
Alternatively, would it be a huge lift to modify this function to use ahb_readl instead of ahb_read? Seems like once the function calls make their way down to the ilpcb_* functions, the implementations slightly differ:

culvert/src/ahb.c

Line 18 in 40bb728

ssize_t ahb_siphon_out(struct ahb *ctx, uint32_t phys, ssize_t len, int outfd)

culvert/src/bridge/ilpc.c

Line 49 in 40bb728

ssize_t ilpcb_read(struct ahb *ahb, uint32_t addr, void *buf, size_t len)

culvert/src/bridge/ilpc.c

Line 180 in 40bb728

int ilpcb_readl(struct ahb *ahb, uint32_t addr, uint32_t *val)

[amboar]

I'm not confident that using the ilpc bridge to read the flash is known to work; there could be bugs in the bridge implementation that are causing you grief. I think it's unlikely that it's been verified because the ilpc bridge is horrendously slow, and usually the PCIe bridge (p2a) has been available on systems I've played with. If you have access to the datasheet I think it would be worth validating the accesses in the ilpcb_read() match what is required. For instance, notice that the data dumped out repeats byte values in groups of 4. I wonder if the data is sensible if you collapse the 4-byte runs down to one byte?

You could try switching the ilpc bridge to doing 4-byte accesses rather than 1, and then trimming the data. You'll just need to be careful about address alignment.

[prockallsyms]

While this is by no means a permanent solution, a small patch to src/ahb.c (specifically ahb_siphon_out) enabled me to get a FW dump:

ssize_t ahb_siphon_out(struct ahb *ctx, uint32_t phys, ssize_t len, int outfd)
{
        ssize_t ingress, egress;
        void *chunk;
        uint32_t val = -1;
        int rc = 0;

        if (!len)
                return 0;

        chunk = malloc(AHB_CHUNK);
        if (!chunk)
                return -errno;

        do {
                memset(chunk, 0, AHB_CHUNK);
                ingress = (len > AHB_CHUNK || len == -1) ? AHB_CHUNK : len;

                for(int i = 0; i < ingress; i += 4) {
                        rc = ahb_readl(ctx, phys, &val);
                        if(rc) {
                                logt("%s: Error reading 0x%08x got val %08x\n", __func__, phys+i, val);
                                return rc;
                        }

                        memcpy(chunk + i, &val, sizeof(val));
                        len -= 4;
                        phys += 4;
                }

                while (ingress) {
                        egress = write(outfd, chunk, ingress);
                        if (egress == -1) {
                                logt("%s: Error writing to output FD", __func__);
                                return -errno;
                        }

                        ingress -= egress;
                }

                fprintf(stderr, "len left: %08lx\n", len);
        } while (!!len);
        fprintf(stderr, "\n");
        free(chunk);

        return rc;
}

Some further testing would be required to understand how the chipset works when configuring 1-byte, 2-byte, and 4-byte mode through the SIO iLPC2AHB interface (pages 734-736 of the AST2500 datasheet).

[amboar]

@prockallsyms do you mind pasting a diff so it's easier to understand the changes?

[prockallsyms]

@amboar Yeah of course. I've attached it below. A TDLR is that instead of relying on ahb_read (which in my specific case eventually trickles down to ilpcb_read) to read blocks, I've lifted the logic to this function by reading 4 bytes at a time with ahb_readl.

diff --git a/src/ahb.c b/src/ahb.c
index e0c2530..cf9cb9b 100644
--- a/src/ahb.c
+++ b/src/ahb.c
@@ -18,40 +18,49 @@
 ssize_t ahb_siphon_out(struct ahb *ctx, uint32_t phys, ssize_t len, int outfd)
 {
        ssize_t ingress, egress;
-    void *chunk, *cursor;
+       void *chunk;
+       uint32_t val = -1;
        int rc = 0;

-    if (!len)
+       if (!len) {
+               fprintf(stderr, "len not set right\n");
                return 0;
+       }

        chunk = malloc(AHB_CHUNK);
-    if (!chunk)
+       if (!chunk) {
+               fprintf(stderr, "chunk not set right\n");
                return -errno;
+       }

        do {
+               memset(chunk, 0, AHB_CHUNK);
                ingress = (len > AHB_CHUNK || len == -1) ? AHB_CHUNK : len;

-        ingress = ahb_read(ctx, phys, chunk, ingress);
-        if (ingress < 0) { rc = -EIO; goto done; }
+               for(int i = 0; i < ingress; i += 4) {
+                       rc = ahb_readl(ctx, phys, &val);
+                       if(rc) {
+                               logt("%s: Error reading 0x%08x got val %08x\n", __func__, phys+i, val);
+                               return rc;
+                       }

-        phys += ingress;
-        if (len > 0) {
-            len -= ingress;
+                       memcpy(chunk + i, &val, sizeof(val));
+                       len -= 4;
+                       phys += 4;
                }

-        cursor = chunk;
                while (ingress) {
-            egress = write(outfd, cursor, ingress);
-            if (egress == -1) { rc = -errno; goto done; }
+                       egress = write(outfd, chunk, ingress);
+                       if (egress == -1) {
+                               logt("%s: Error writing to output FD", __func__);
+                               return -errno;
+                       }

-            cursor += egress;
                        ingress -= egress;
                }

-        fprintf(stderr, ".");
+               fprintf(stderr, "len left: %08lx\n", len);
        } while (!!len);
-
-done:
        fprintf(stderr, "\n");
        free(chunk);

The reason the ilpcb_read function may be misbehaving is that it attempts to read in 1-byte increments by incrementing the memory address and re-issuing a SPI read request. My hypothesis is that the address must be word-aligned because the SPI flash only allows 4-byte reads, or that there is some sort of incompatibility between this specific SPI flash and the AST2500 that causes this weird behavior. I could be way off here though, not too familiar with this domain. 😄

[amboar]

Right, I expect it's actually the way the bridge is implemented. See e.g. this insightful comment regarding the P2A in the driver for the flash controller:

https://github.com/amboar/culvert/blob/main/src/soc/sfc.c#L250-L257

Perhaps modify ilpcb_read() to use 4 byte reads rather than reimplementing ahb_siphon_out()?

Edit: See also https://github.com/amboar/culvert/blob/main/src/bridge/ilpc.c#L145 - probably also an issue of correctness there 😅