m-labs / misoc

The original high performance and small footprint system-on-chip based on Migen™
https://m-labs.hk
Other
306 stars 86 forks source link

spi_flash: Add little endian support #118

Closed occheung closed 2 years ago

occheung commented 2 years ago

Changes

Issue with flashing a byte-swapped binary

111 byte-swaps the BIOS binary into little-endian, if the target CPU was configured with vexriscv.

Although it ensure the instructions read from SPI flash is with the correct endianness, configuring the flash becomes messy.

Configure for read

This can be demonstrated with reference to artiq_mkfs, suppose we have an entry of {"foo": "1"} in the flash with swapped endianness.

00000000  00 00 00 09 66 6f 6f 00  31 ff ff ff ff           |....foo.1....|
0000000d

A little endian machine will interpret this as a slice with the wrong byte order.

One solution is to byte-swap the entire binary before flash (e.g. modify artiq_mkfs.py). It is a somewhat clean solution as long as the flash content need not be modified, which brings us into the next part.

Modifying the flash

To make our {"foo": "1"} understandable by Rust (or other platforms), the bytes needs to be rewritten into the following hexdump.

00000000  09 00 00 00 00 6f 6f 66  ff ff ff 31 ff           |.....oof...1.|
0000000d

(Note: The terminating 0xffs might need to be padded to align with 16 bytes, but for the sake of simplicity, let's ignore the termination all together.) Suppose the flash is to be modified on runtime, e.g. append one more entry {"bar": "2"}. One naive solution is to write the binary representation from address 0x0a onward, the following might be the result.

00000000  09 00 00 00 00 6f 6f 66  ff 00 00 00 09 62 61 72          |.....oof.....bar|
...

(With the encoding beyond "bar" omitted, this should give a clear enough picture.)

Modifying existing code to fit this case is totally doable. However, adding little endian support to the SPI flash would be a cleaner solution.

Notes

I think there are too many "set little-endian if it uses vexriscv, otherwise big-endian" statements (including previous patches e.g. #117), should we refactor the endianness argument at some point?