[PATCH v2 00/25] hw/riscv: sifive_u: Add missing SPI support

[PATCH v2 00/25] hw/riscv: sifive_u: Add missing SPI support

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds the missing SPI support to the `sifive_u` machine in the QEMU
mainline. With this series, upstream U-Boot for the SiFive HiFive Unleashed
board can boot on QEMU `sifive_u` out of the box. This allows users to
develop and test the recommended RISC-V boot flow with a real world use
case: ZSBL (in QEMU) loads U-Boot SPL from SD card or SPI flash to L2LIM,
then U-Boot SPL loads the payload from SD card or SPI flash that is a
combination of OpenSBI fw_dynamic firmware and U-Boot proper.

The m25p80 model is updated to support ISSI flash series. A bunch of
ssi-sd issues are fixed, and writing to SD card in SPI mode is supported.

reST documentation for RISC-V is added. Currently only `sifive_u`
machine is documented, but more to come.

Changes in v2:
- Mention QPI (Quad Peripheral Interface) mode is not supported
- Add a debug printf in the state handling codes
- Fix several typos in the commit message
- new patch: add a state representing Nac
- Make this fix a separate patch from the CMD18 support
- Fix 2 typos in the commit message
- Add a comment block to explain the CMD12 timing
- Catch CMD12 in all of the data read states per the timing requirement
- Move multiple write token definitions out of this patch
- Correct the "coding" typo in the commit message
- Correct the "token" typo in the commit message
- Add 'write_bytes' in vmstate_ssi_sd
- Correct the "token" typo in the commit message
- Introduce multiple write token definitions in this patch
- new patch: bump up version ids of VMStateDescription
- Log guest error when trying to write reserved registers
- Log guest error when trying to access out-of-bounds registers
- log guest error when writing to reserved bits for chip select
  registers and watermark registers
- Log unimplemented warning when trying to write direct-map flash
  interface registers
- Add test tx fifo full logic in sifive_spi_read(), hence remove
  setting the tx fifo full flag in sifive_spi_write().
- Populate register with their default value
- Correct the "connects" typo in the commit message
- Mention in the commit message that <reg> property does not populate
  the second group which represents the memory mapped address of the
  SPI flash
- Correct the "connects" typo in the commit message
- Correct several typos in sifive_u.rst
- Update doc to mention U-Boot v2021.01

Bin Meng (25):
  hw/block: m25p80: Add ISSI SPI flash support
  hw/block: m25p80: Add various ISSI flash information
  hw/sd: ssi-sd: Fix incorrect card response sequence
  hw/sd: sd: Support CMD59 for SPI mode
  hw/sd: sd: Drop sd_crc16()
  util: Add CRC16 (CCITT) calculation routines
  hw/sd: ssi-sd: Suffix a data block with CRC16
  hw/sd: ssi-sd: Add a state representing Nac
  hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
  hw/sd: ssi-sd: Support multiple block read
  hw/sd: ssi-sd: Use macros for the dummy value and tokens in the
    transfer
  hw/sd: sd: Remove duplicated codes in single/multiple block read/write
  hw/sd: sd: Allow single/multiple block write for SPI mode
  hw/sd: sd.h: Cosmetic change of using spaces
  hw/sd: Introduce receive_ready() callback
  hw/sd: ssi-sd: Support single block write
  hw/sd: ssi-sd: Support multiple block write
  hw/sd: ssi-sd: Bump up version ids of VMStateDescription
  hw/ssi: Add SiFive SPI controller support
  hw/riscv: sifive_u: Add QSPI0 controller and connect a flash
  hw/riscv: sifive_u: Add QSPI2 controller and connect an SD card
  hw/riscv: sifive_u: Change SIFIVE_U_GEM_IRQ to decimal value
  docs/system: Sort targets in alphabetical order
  docs/system: Add RISC-V documentation
  docs/system: riscv: Add documentation for sifive_u machine

 docs/system/riscv/sifive_u.rst | 336 ++++++++++++++++++++++++++++++
 docs/system/target-riscv.rst   |  72 +++++++
 docs/system/targets.rst        |  20 +-
 include/hw/riscv/sifive_u.h    |   9 +-
 include/hw/sd/sd.h             |  44 ++--
 include/hw/ssi/sifive_spi.h    |  47 +++++
 include/qemu/crc-ccitt.h       |  33 +++
 hw/block/m25p80.c              |  57 ++++-
 hw/riscv/sifive_u.c            |  91 ++++++++
 hw/sd/core.c                   |  13 ++
 hw/sd/sd.c                     |  82 +-------
 hw/sd/ssi-sd.c                 | 166 +++++++++++++--
 hw/ssi/sifive_spi.c            | 367 +++++++++++++++++++++++++++++++++
 util/crc-ccitt.c               | 127 ++++++++++++
 hw/riscv/Kconfig               |   3 +
 hw/ssi/Kconfig                 |   4 +
 hw/ssi/meson.build             |   1 +
 util/meson.build               |   1 +
 18 files changed, 1347 insertions(+), 126 deletions(-)
 create mode 100644 docs/system/riscv/sifive_u.rst
 create mode 100644 docs/system/target-riscv.rst
 create mode 100644 include/hw/ssi/sifive_spi.h
 create mode 100644 include/qemu/crc-ccitt.h
 create mode 100644 hw/ssi/sifive_spi.c
 create mode 100644 util/crc-ccitt.c

-- 
2.25.1

[PATCH v2 01/25] hw/block: m25p80: Add ISSI SPI flash support

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds the ISSI SPI flash support. The number of dummy cycles in
fast read, fast read dual output and fast read quad output commands
is currently using the default 8. Likewise, the same default value
is used for fast read dual/quad I/O command. Per the datasheet [1],
the number of dummy cycles is configurable, but this is not modeled
at present.

For flash whose size is larger than 16 MiB, the sequence of 3-byte
address along with EXTADD bit in the bank address register (BAR) is
not supported. We assume that guest software always uses op codes
with 4-byte address sequence. Fortunately, this is the case for both
U-Boot and Linux spi-nor drivers.

QPI (Quad Peripheral Interface) that supports 2-cycle instruction
has different default values for dummy cycles of fast read family
commands, and is unsupported at the time being.

[1] http://www.issi.com/WW/pdf/25LP-WP256.pdf

Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- Mention QPI (Quad Peripheral Interface) mode is not supported

 hw/block/m25p80.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index b744a58d1c..217c130f56 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -412,6 +412,7 @@ typedef enum {
     MAN_NUMONYX,
     MAN_WINBOND,
     MAN_SST,
+    MAN_ISSI,
     MAN_GENERIC,
 } Manufacturer;
 
@@ -487,6 +488,8 @@ static inline Manufacturer get_man(Flash *s)
         return MAN_MACRONIX;
     case 0xBF:
         return MAN_SST;
+    case 0x9D:
+        return MAN_ISSI;
     default:
         return MAN_GENERIC;
     }
@@ -706,6 +709,9 @@ static void complete_collecting_data(Flash *s)
         case MAN_SPANSION:
             s->quad_enable = !!(s->data[1] & 0x02);
             break;
+        case MAN_ISSI:
+            s->quad_enable = extract32(s->data[0], 6, 1);
+            break;
         case MAN_MACRONIX:
             s->quad_enable = extract32(s->data[0], 6, 1);
             if (s->len > 1) {
@@ -895,6 +901,19 @@ static void decode_fast_read_cmd(Flash *s)
                                     SPANSION_DUMMY_CLK_LEN
                                     );
         break;
+    case MAN_ISSI:
+        /*
+         * The Fast Read instruction code is followed by address bytes and
+         * dummy cycles, transmitted via the SI line.
+         *
+         * The number of dummy cycles is configurable but this is currently
+         * unmodeled, hence the default value 8 is used.
+         *
+         * QPI (Quad Peripheral Interface) mode has different default value
+         * of dummy cycles, but this is unsupported at the time being.
+         */
+        s->needed_bytes += 1;
+        break;
     default:
         break;
     }
@@ -934,6 +953,16 @@ static void decode_dio_read_cmd(Flash *s)
             break;
         }
         break;
+    case MAN_ISSI:
+        /*
+         * The Fast Read Dual I/O instruction code is followed by address bytes
+         * and dummy cycles, transmitted via the IO1 and IO0 line.
+         *
+         * The number of dummy cycles is configurable but this is currently
+         * unmodeled, hence the default value 4 is used.
+         */
+        s->needed_bytes += 1;
+        break;
     default:
         break;
     }
@@ -974,6 +1003,19 @@ static void decode_qio_read_cmd(Flash *s)
             break;
         }
         break;
+    case MAN_ISSI:
+        /*
+         * The Fast Read Quad I/O instruction code is followed by address bytes
+         * and dummy cycles, transmitted via the IO3, IO2, IO1 and IO0 line.
+         *
+         * The number of dummy cycles is configurable but this is currently
+         * unmodeled, hence the default value 6 is used.
+         *
+         * QPI (Quad Peripheral Interface) mode has different default value
+         * of dummy cycles, but this is unsupported at the time being.
+         */
+        s->needed_bytes += 3;
+        break;
     default:
         break;
     }
@@ -1132,7 +1174,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
     case RDSR:
         s->data[0] = (!!s->write_enable) << 1;
-        if (get_man(s) == MAN_MACRONIX) {
+        if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
             s->data[0] |= (!!s->quad_enable) << 6;
         }
         if (get_man(s) == MAN_SST) {
-- 
2.25.1

[PATCH v2 02/25] hw/block: m25p80: Add various ISSI flash information

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This updates the flash information table to include various ISSI
flashes that are supported by upstream U-Boot and Linux kernel.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 hw/block/m25p80.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 217c130f56..4bf8aa8158 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -210,6 +210,19 @@ static const FlashPartInfo known_devices[] = {
     { INFO("640s33b",     0x898913,      0,  64 << 10, 128, 0) },
     { INFO("n25q064",     0x20ba17,      0,  64 << 10, 128, 0) },
 
+    /* ISSI */
+    { INFO("is25lq040b",  0x9d4013,      0,  64 << 10,   8, ER_4K) },
+    { INFO("is25lp080d",  0x9d6014,      0,  64 << 10,  16, ER_4K) },
+    { INFO("is25lp016d",  0x9d6015,      0,  64 << 10,  32, ER_4K) },
+    { INFO("is25lp032",   0x9d6016,      0,  64 << 10,  64, ER_4K) },
+    { INFO("is25lp064",   0x9d6017,      0,  64 << 10, 128, ER_4K) },
+    { INFO("is25lp128",   0x9d6018,      0,  64 << 10, 256, ER_4K) },
+    { INFO("is25lp256",   0x9d6019,      0,  64 << 10, 512, ER_4K) },
+    { INFO("is25wp032",   0x9d7016,      0,  64 << 10,  64, ER_4K) },
+    { INFO("is25wp064",   0x9d7017,      0,  64 << 10, 128, ER_4K) },
+    { INFO("is25wp128",   0x9d7018,      0,  64 << 10, 256, ER_4K) },
+    { INFO("is25wp256",   0x9d7019,      0,  64 << 10, 512, ER_4K) },
+
     /* Macronix */
     { INFO("mx25l2005a",  0xc22012,      0,  64 << 10,   4, ER_4K) },
     { INFO("mx25l4005a",  0xc22013,      0,  64 << 10,   8, ER_4K) },
-- 
2.25.1

[PATCH v2 03/25] hw/sd: ssi-sd: Fix incorrect card response sequence

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Per the "Physical Layer Specification Version 8.00" chapter 7.5.1,
"Command/Response", there is a minimum 8 clock cycles (Ncr) before
the card response shows up on the data out line. However current
implementation jumps directly to the sending response state after
all 6 bytes command is received, which is a spec violation.

Add a new state PREP_RESP in the ssi-sd state machine to handle it.

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>

---

Changes in v2:
- Add a debug printf in the state handling codes

 hw/sd/ssi-sd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 9a75e0095c..043e270066 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -36,6 +36,7 @@ do { fprintf(stderr, "ssi_sd: error: " fmt , ## __VA_ARGS__);} while (0)
 typedef enum {
     SSI_SD_CMD = 0,
     SSI_SD_CMDARG,
+    SSI_SD_PREP_RESP,
     SSI_SD_RESPONSE,
     SSI_SD_DATA_START,
     SSI_SD_DATA_READ,
@@ -163,12 +164,16 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
                 s->response[1] = status;
                 DPRINTF("Card status 0x%02x\n", status);
             }
-            s->mode = SSI_SD_RESPONSE;
+            s->mode = SSI_SD_PREP_RESP;
             s->response_pos = 0;
         } else {
             s->cmdarg[s->arglen++] = val;
         }
         return 0xff;
+    case SSI_SD_PREP_RESP:
+        DPRINTF("Prepare card response (Ncr)\n");
+        s->mode = SSI_SD_RESPONSE;
+        return 0xff;
     case SSI_SD_RESPONSE:
         if (s->stopping) {
             s->stopping = 0;
-- 
2.25.1

Re: [PATCH v2 03/25] hw/sd: ssi-sd: Fix incorrect card response sequence

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Per the "Physical Layer Specification Version 8.00" chapter 7.5.1,
> "Command/Response", there is a minimum 8 clock cycles (Ncr) before
> the card response shows up on the data out line. However current
> implementation jumps directly to the sending response state after
> all 6 bytes command is received, which is a spec violation.
> 
> Add a new state PREP_RESP in the ssi-sd state machine to handle it.
> 
> Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> 
> ---
> 
> Changes in v2:
> - Add a debug printf in the state handling codes
> 
>  hw/sd/ssi-sd.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 9a75e0095c..043e270066 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -36,6 +36,7 @@ do { fprintf(stderr, "ssi_sd: error: " fmt , ## __VA_ARGS__);} while (0)
>  typedef enum {
>      SSI_SD_CMD = 0,
>      SSI_SD_CMDARG,
> +    SSI_SD_PREP_RESP,

Another migration change.

Otherwise (trusting you with the spec):
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>      SSI_SD_RESPONSE,
>      SSI_SD_DATA_START,
>      SSI_SD_DATA_READ,
> @@ -163,12 +164,16 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
>                  s->response[1] = status;
>                  DPRINTF("Card status 0x%02x\n", status);
>              }
> -            s->mode = SSI_SD_RESPONSE;
> +            s->mode = SSI_SD_PREP_RESP;
>              s->response_pos = 0;
>          } else {
>              s->cmdarg[s->arglen++] = val;
>          }
>          return 0xff;
> +    case SSI_SD_PREP_RESP:
> +        DPRINTF("Prepare card response (Ncr)\n");
> +        s->mode = SSI_SD_RESPONSE;
> +        return 0xff;
>      case SSI_SD_RESPONSE:
>          if (s->stopping) {
>              s->stopping = 0;
> 

[PATCH v2 04/25] hw/sd: sd: Support CMD59 for SPI mode

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

After the card is put into SPI mode, CRC check for all commands
including CMD0 will be done according to CMD59 setting. But this
command is currently unimplemented. Simply allow the decoding of
CMD59, but the CRC remains unchecked.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>
---

(no changes since v1)

 hw/sd/sd.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4375ed5b8b..bfea5547d5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1517,18 +1517,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         if (!sd->spi) {
             goto bad_cmd;
         }
-        goto unimplemented_spi_cmd;
+        return sd_r1;
 
     default:
     bad_cmd:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
         return sd_illegal;
-
-    unimplemented_spi_cmd:
-        /* Commands that are recognised but not yet implemented in SPI mode.  */
-        qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
-                      req.cmd);
-        return sd_illegal;
     }
 
     qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd);
-- 
2.25.1

Re: [PATCH v2 04/25] hw/sd: sd: Support CMD59 for SPI mode

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> After the card is put into SPI mode, CRC check for all commands
> including CMD0 will be done according to CMD59 setting. But this
> command is currently unimplemented. Simply allow the decoding of
> CMD59, but the CRC remains unchecked.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> ---
> 
> (no changes since v1)
> 
>  hw/sd/sd.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH v2 05/25] hw/sd: sd: Drop sd_crc16()

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

commit f6fb1f9b319f ("sdcard: Correct CRC16 offset in sd_function_switch()")
changed the 16-bit CRC to be stored at offset 64. In fact, this CRC
calculation is completely wrong. From the original codes, it wants
to calculate the CRC16 of the first 64 bytes of sd->data[], however
passing 64 as the `width` to sd_crc16() actually counts 256 bytes
starting from the `message` for the CRC16 calculation, which is not
what we want.

Besides that, it seems existing sd_crc16() algorithm does not match
the SD spec, which says CRC16 is the CCITT one but the calculation
does not produce expected result. It turns out the CRC16 was never
transferred outside the sd core, as in sd_read_byte() we see:

    if (sd->data_offset >= 64)
        sd->state = sd_transfer_state;

Given above reasons, let's drop it.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>

---

Changes in v2:
- Fix several typos in the commit message

 hw/sd/sd.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bfea5547d5..b3952514fe 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -271,23 +271,6 @@ static uint8_t sd_crc7(const void *message, size_t width)
     return shift_reg;
 }
 
-static uint16_t sd_crc16(const void *message, size_t width)
-{
-    int i, bit;
-    uint16_t shift_reg = 0x0000;
-    const uint16_t *msg = (const uint16_t *)message;
-    width <<= 1;
-
-    for (i = 0; i < width; i ++, msg ++)
-        for (bit = 15; bit >= 0; bit --) {
-            shift_reg <<= 1;
-            if ((shift_reg >> 15) ^ ((*msg >> bit) & 1))
-                shift_reg ^= 0x1011;
-        }
-
-    return shift_reg;
-}
-
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
 
 FIELD(OCR, VDD_VOLTAGE_WINDOW,          0, 24)
@@ -843,7 +826,6 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
         sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4);
     }
     memset(&sd->data[17], 0, 47);
-    stw_be_p(sd->data + 64, sd_crc16(sd->data, 64));
 }
 
 static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
-- 
2.25.1

Re: [PATCH v2 05/25] hw/sd: sd: Drop sd_crc16()

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> commit f6fb1f9b319f ("sdcard: Correct CRC16 offset in sd_function_switch()")
> changed the 16-bit CRC to be stored at offset 64. In fact, this CRC
> calculation is completely wrong. From the original codes, it wants
> to calculate the CRC16 of the first 64 bytes of sd->data[], however
> passing 64 as the `width` to sd_crc16() actually counts 256 bytes
> starting from the `message` for the CRC16 calculation, which is not
> what we want.
> 
> Besides that, it seems existing sd_crc16() algorithm does not match
> the SD spec, which says CRC16 is the CCITT one but the calculation
> does not produce expected result. It turns out the CRC16 was never
> transferred outside the sd core, as in sd_read_byte() we see:
> 
>     if (sd->data_offset >= 64)
>         sd->state = sd_transfer_state;
> 
> Given above reasons, let's drop it.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> Tested-by: Pragnesh Patel <pragnesh.patel@sifive.com>
> 
> ---
> 
> Changes in v2:
> - Fix several typos in the commit message
> 
>  hw/sd/sd.c | 18 ------------------
>  1 file changed, 18 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Import CRC16 calculation routines from Linux kernel v5.10:

  include/linux/crc-ccitt.h
  lib/crc-ccitt.c

to QEMU:

  include/qemu/crc-ccitt.h
  util/crc-ccitt.c

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 include/qemu/crc-ccitt.h |  33 ++++++++++
 util/crc-ccitt.c         | 127 +++++++++++++++++++++++++++++++++++++++
 util/meson.build         |   1 +
 3 files changed, 161 insertions(+)
 create mode 100644 include/qemu/crc-ccitt.h
 create mode 100644 util/crc-ccitt.c

diff --git a/include/qemu/crc-ccitt.h b/include/qemu/crc-ccitt.h
new file mode 100644
index 0000000000..06ee55b159
--- /dev/null
+++ b/include/qemu/crc-ccitt.h
@@ -0,0 +1,33 @@
+/*
+ * CRC16 (CCITT) Checksum Algorithm
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng <bin.meng@windriver.com>
+ *
+ * From Linux kernel v5.10 include/linux/crc-ccitt.h
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef _CRC_CCITT_H
+#define _CRC_CCITT_H
+
+extern uint16_t const crc_ccitt_table[256];
+extern uint16_t const crc_ccitt_false_table[256];
+
+extern uint16_t crc_ccitt(uint16_t crc, const uint8_t *buffer, size_t len);
+extern uint16_t crc_ccitt_false(uint16_t crc, const uint8_t *buffer, size_t len);
+
+static inline uint16_t crc_ccitt_byte(uint16_t crc, const uint8_t c)
+{
+    return (crc >> 8) ^ crc_ccitt_table[(crc ^ c) & 0xff];
+}
+
+static inline uint16_t crc_ccitt_false_byte(uint16_t crc, const uint8_t c)
+{
+    return (crc << 8) ^ crc_ccitt_false_table[(crc >> 8) ^ c];
+}
+
+#endif /* _CRC_CCITT_H */
diff --git a/util/crc-ccitt.c b/util/crc-ccitt.c
new file mode 100644
index 0000000000..b981d8ac55
--- /dev/null
+++ b/util/crc-ccitt.c
@@ -0,0 +1,127 @@
+/*
+ * CRC16 (CCITT) Checksum Algorithm
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng <bin.meng@windriver.com>
+ *
+ * From Linux kernel v5.10 lib/crc-ccitt.c
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/crc-ccitt.h"
+
+/*
+ * This mysterious table is just the CRC of each possible byte. It can be
+ * computed using the standard bit-at-a-time methods. The polynomial can
+ * be seen in entry 128, 0x8408. This corresponds to x^0 + x^5 + x^12.
+ * Add the implicit x^16, and you have the standard CRC-CCITT.
+ */
+uint16_t const crc_ccitt_table[256] = {
+    0x0000, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
+    0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
+    0x1081, 0x0108, 0x3393, 0x221a, 0x56a5, 0x472c, 0x75b7, 0x643e,
+    0x9cc9, 0x8d40, 0xbfdb, 0xae52, 0xdaed, 0xcb64, 0xf9ff, 0xe876,
+    0x2102, 0x308b, 0x0210, 0x1399, 0x6726, 0x76af, 0x4434, 0x55bd,
+    0xad4a, 0xbcc3, 0x8e58, 0x9fd1, 0xeb6e, 0xfae7, 0xc87c, 0xd9f5,
+    0x3183, 0x200a, 0x1291, 0x0318, 0x77a7, 0x662e, 0x54b5, 0x453c,
+    0xbdcb, 0xac42, 0x9ed9, 0x8f50, 0xfbef, 0xea66, 0xd8fd, 0xc974,
+    0x4204, 0x538d, 0x6116, 0x709f, 0x0420, 0x15a9, 0x2732, 0x36bb,
+    0xce4c, 0xdfc5, 0xed5e, 0xfcd7, 0x8868, 0x99e1, 0xab7a, 0xbaf3,
+    0x5285, 0x430c, 0x7197, 0x601e, 0x14a1, 0x0528, 0x37b3, 0x263a,
+    0xdecd, 0xcf44, 0xfddf, 0xec56, 0x98e9, 0x8960, 0xbbfb, 0xaa72,
+    0x6306, 0x728f, 0x4014, 0x519d, 0x2522, 0x34ab, 0x0630, 0x17b9,
+    0xef4e, 0xfec7, 0xcc5c, 0xddd5, 0xa96a, 0xb8e3, 0x8a78, 0x9bf1,
+    0x7387, 0x620e, 0x5095, 0x411c, 0x35a3, 0x242a, 0x16b1, 0x0738,
+    0xffcf, 0xee46, 0xdcdd, 0xcd54, 0xb9eb, 0xa862, 0x9af9, 0x8b70,
+    0x8408, 0x9581, 0xa71a, 0xb693, 0xc22c, 0xd3a5, 0xe13e, 0xf0b7,
+    0x0840, 0x19c9, 0x2b52, 0x3adb, 0x4e64, 0x5fed, 0x6d76, 0x7cff,
+    0x9489, 0x8500, 0xb79b, 0xa612, 0xd2ad, 0xc324, 0xf1bf, 0xe036,
+    0x18c1, 0x0948, 0x3bd3, 0x2a5a, 0x5ee5, 0x4f6c, 0x7df7, 0x6c7e,
+    0xa50a, 0xb483, 0x8618, 0x9791, 0xe32e, 0xf2a7, 0xc03c, 0xd1b5,
+    0x2942, 0x38cb, 0x0a50, 0x1bd9, 0x6f66, 0x7eef, 0x4c74, 0x5dfd,
+    0xb58b, 0xa402, 0x9699, 0x8710, 0xf3af, 0xe226, 0xd0bd, 0xc134,
+    0x39c3, 0x284a, 0x1ad1, 0x0b58, 0x7fe7, 0x6e6e, 0x5cf5, 0x4d7c,
+    0xc60c, 0xd785, 0xe51e, 0xf497, 0x8028, 0x91a1, 0xa33a, 0xb2b3,
+    0x4a44, 0x5bcd, 0x6956, 0x78df, 0x0c60, 0x1de9, 0x2f72, 0x3efb,
+    0xd68d, 0xc704, 0xf59f, 0xe416, 0x90a9, 0x8120, 0xb3bb, 0xa232,
+    0x5ac5, 0x4b4c, 0x79d7, 0x685e, 0x1ce1, 0x0d68, 0x3ff3, 0x2e7a,
+    0xe70e, 0xf687, 0xc41c, 0xd595, 0xa12a, 0xb0a3, 0x8238, 0x93b1,
+    0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
+    0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
+    0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
+};
+
+/*
+ * Similar table to calculate CRC16 variant known as CRC-CCITT-FALSE
+ * Reflected bits order, does not augment final value.
+ */
+uint16_t const crc_ccitt_false_table[256] = {
+    0x0000, 0x1021, 0x2042, 0x3063, 0x4084, 0x50A5, 0x60C6, 0x70E7,
+    0x8108, 0x9129, 0xA14A, 0xB16B, 0xC18C, 0xD1AD, 0xE1CE, 0xF1EF,
+    0x1231, 0x0210, 0x3273, 0x2252, 0x52B5, 0x4294, 0x72F7, 0x62D6,
+    0x9339, 0x8318, 0xB37B, 0xA35A, 0xD3BD, 0xC39C, 0xF3FF, 0xE3DE,
+    0x2462, 0x3443, 0x0420, 0x1401, 0x64E6, 0x74C7, 0x44A4, 0x5485,
+    0xA56A, 0xB54B, 0x8528, 0x9509, 0xE5EE, 0xF5CF, 0xC5AC, 0xD58D,
+    0x3653, 0x2672, 0x1611, 0x0630, 0x76D7, 0x66F6, 0x5695, 0x46B4,
+    0xB75B, 0xA77A, 0x9719, 0x8738, 0xF7DF, 0xE7FE, 0xD79D, 0xC7BC,
+    0x48C4, 0x58E5, 0x6886, 0x78A7, 0x0840, 0x1861, 0x2802, 0x3823,
+    0xC9CC, 0xD9ED, 0xE98E, 0xF9AF, 0x8948, 0x9969, 0xA90A, 0xB92B,
+    0x5AF5, 0x4AD4, 0x7AB7, 0x6A96, 0x1A71, 0x0A50, 0x3A33, 0x2A12,
+    0xDBFD, 0xCBDC, 0xFBBF, 0xEB9E, 0x9B79, 0x8B58, 0xBB3B, 0xAB1A,
+    0x6CA6, 0x7C87, 0x4CE4, 0x5CC5, 0x2C22, 0x3C03, 0x0C60, 0x1C41,
+    0xEDAE, 0xFD8F, 0xCDEC, 0xDDCD, 0xAD2A, 0xBD0B, 0x8D68, 0x9D49,
+    0x7E97, 0x6EB6, 0x5ED5, 0x4EF4, 0x3E13, 0x2E32, 0x1E51, 0x0E70,
+    0xFF9F, 0xEFBE, 0xDFDD, 0xCFFC, 0xBF1B, 0xAF3A, 0x9F59, 0x8F78,
+    0x9188, 0x81A9, 0xB1CA, 0xA1EB, 0xD10C, 0xC12D, 0xF14E, 0xE16F,
+    0x1080, 0x00A1, 0x30C2, 0x20E3, 0x5004, 0x4025, 0x7046, 0x6067,
+    0x83B9, 0x9398, 0xA3FB, 0xB3DA, 0xC33D, 0xD31C, 0xE37F, 0xF35E,
+    0x02B1, 0x1290, 0x22F3, 0x32D2, 0x4235, 0x5214, 0x6277, 0x7256,
+    0xB5EA, 0xA5CB, 0x95A8, 0x8589, 0xF56E, 0xE54F, 0xD52C, 0xC50D,
+    0x34E2, 0x24C3, 0x14A0, 0x0481, 0x7466, 0x6447, 0x5424, 0x4405,
+    0xA7DB, 0xB7FA, 0x8799, 0x97B8, 0xE75F, 0xF77E, 0xC71D, 0xD73C,
+    0x26D3, 0x36F2, 0x0691, 0x16B0, 0x6657, 0x7676, 0x4615, 0x5634,
+    0xD94C, 0xC96D, 0xF90E, 0xE92F, 0x99C8, 0x89E9, 0xB98A, 0xA9AB,
+    0x5844, 0x4865, 0x7806, 0x6827, 0x18C0, 0x08E1, 0x3882, 0x28A3,
+    0xCB7D, 0xDB5C, 0xEB3F, 0xFB1E, 0x8BF9, 0x9BD8, 0xABBB, 0xBB9A,
+    0x4A75, 0x5A54, 0x6A37, 0x7A16, 0x0AF1, 0x1AD0, 0x2AB3, 0x3A92,
+    0xFD2E, 0xED0F, 0xDD6C, 0xCD4D, 0xBDAA, 0xAD8B, 0x9DE8, 0x8DC9,
+    0x7C26, 0x6C07, 0x5C64, 0x4C45, 0x3CA2, 0x2C83, 0x1CE0, 0x0CC1,
+    0xEF1F, 0xFF3E, 0xCF5D, 0xDF7C, 0xAF9B, 0xBFBA, 0x8FD9, 0x9FF8,
+    0x6E17, 0x7E36, 0x4E55, 0x5E74, 0x2E93, 0x3EB2, 0x0ED1, 0x1EF0
+};
+
+/**
+ * crc_ccitt - recompute the CRC (CRC-CCITT variant)
+ * for the data buffer
+ *
+ * @crc: previous CRC value
+ * @buffer: data pointer
+ * @len: number of bytes in the buffer
+ */
+uint16_t crc_ccitt(uint16_t crc, uint8_t const *buffer, size_t len)
+{
+    while (len--) {
+        crc = crc_ccitt_byte(crc, *buffer++);
+    }
+    return crc;
+}
+
+/**
+ * crc_ccitt_false - recompute the CRC (CRC-CCITT-FALSE variant)
+ * for the data buffer
+ *
+ * @crc: previous CRC value
+ * @buffer: data pointer
+ * @len: number of bytes in the buffer
+ */
+uint16_t crc_ccitt_false(uint16_t crc, uint8_t const *buffer, size_t len)
+{
+    while (len--) {
+        crc = crc_ccitt_false_byte(crc, *buffer++);
+    }
+    return crc;
+}
diff --git a/util/meson.build b/util/meson.build
index 540a605b78..05a376ae02 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -29,6 +29,7 @@ util_ss.add(files('qemu-config.c', 'notify.c'))
 util_ss.add(files('qemu-option.c', 'qemu-progress.c'))
 util_ss.add(files('keyval.c'))
 util_ss.add(files('crc32c.c'))
+util_ss.add(files('crc-ccitt.c'))
 util_ss.add(files('uuid.c'))
 util_ss.add(files('getauxval.c'))
 util_ss.add(files('rcu.c'))
-- 
2.25.1

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Import CRC16 calculation routines from Linux kernel v5.10:
> 
>   include/linux/crc-ccitt.h
>   lib/crc-ccitt.c
> 
> to QEMU:
> 
>   include/qemu/crc-ccitt.h
>   util/crc-ccitt.c
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> 
> (no changes since v1)
> 
>  include/qemu/crc-ccitt.h |  33 ++++++++++
>  util/crc-ccitt.c         | 127 +++++++++++++++++++++++++++++++++++++++
>  util/meson.build         |   1 +
>  3 files changed, 161 insertions(+)
>  create mode 100644 include/qemu/crc-ccitt.h
>  create mode 100644 util/crc-ccitt.c
...

> diff --git a/util/meson.build b/util/meson.build
> index 540a605b78..05a376ae02 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -29,6 +29,7 @@ util_ss.add(files('qemu-config.c', 'notify.c'))
>  util_ss.add(files('qemu-option.c', 'qemu-progress.c'))
>  util_ss.add(files('keyval.c'))
>  util_ss.add(files('crc32c.c'))
> +util_ss.add(files('crc-ccitt.c'))

OK but we can restrict this to system-mode emulation, as user-mode
and tools don't require it.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Richard Henderson]

On 1/24/21 8:59 AM, Philippe Mathieu-Daudé wrote:
> On 1/23/21 11:39 AM, Bin Meng wrote:
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> Import CRC16 calculation routines from Linux kernel v5.10:
>>
>>   include/linux/crc-ccitt.h
>>   lib/crc-ccitt.c
>>
>> to QEMU:
>>
>>   include/qemu/crc-ccitt.h
>>   util/crc-ccitt.c
>>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>> Acked-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>
>> (no changes since v1)
>>
>>  include/qemu/crc-ccitt.h |  33 ++++++++++
>>  util/crc-ccitt.c         | 127 +++++++++++++++++++++++++++++++++++++++
>>  util/meson.build         |   1 +
>>  3 files changed, 161 insertions(+)
>>  create mode 100644 include/qemu/crc-ccitt.h
>>  create mode 100644 util/crc-ccitt.c
> ...
> 
>> diff --git a/util/meson.build b/util/meson.build
>> index 540a605b78..05a376ae02 100644
>> --- a/util/meson.build
>> +++ b/util/meson.build
>> @@ -29,6 +29,7 @@ util_ss.add(files('qemu-config.c', 'notify.c'))
>>  util_ss.add(files('qemu-option.c', 'qemu-progress.c'))
>>  util_ss.add(files('keyval.c'))
>>  util_ss.add(files('crc32c.c'))
>> +util_ss.add(files('crc-ccitt.c'))
> 
> OK but we can restrict this to system-mode emulation, as user-mode
> and tools don't require it.

Doesn't this get put in libutil, where it is only pulled from the archive when
needed?  Also, libutil is built once, not per-target.

r~

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Philippe Mathieu-Daudé]

On 1/24/21 9:07 PM, Richard Henderson wrote:
> On 1/24/21 8:59 AM, Philippe Mathieu-Daudé wrote:
>> On 1/23/21 11:39 AM, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@windriver.com>
>>>
>>> Import CRC16 calculation routines from Linux kernel v5.10:
>>>
>>>   include/linux/crc-ccitt.h
>>>   lib/crc-ccitt.c
>>>
>>> to QEMU:
>>>
>>>   include/qemu/crc-ccitt.h
>>>   util/crc-ccitt.c
>>>
>>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>> Acked-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>  include/qemu/crc-ccitt.h |  33 ++++++++++
>>>  util/crc-ccitt.c         | 127 +++++++++++++++++++++++++++++++++++++++
>>>  util/meson.build         |   1 +
>>>  3 files changed, 161 insertions(+)
>>>  create mode 100644 include/qemu/crc-ccitt.h
>>>  create mode 100644 util/crc-ccitt.c
>> ...
>>
>>> diff --git a/util/meson.build b/util/meson.build
>>> index 540a605b78..05a376ae02 100644
>>> --- a/util/meson.build
>>> +++ b/util/meson.build
>>> @@ -29,6 +29,7 @@ util_ss.add(files('qemu-config.c', 'notify.c'))
>>>  util_ss.add(files('qemu-option.c', 'qemu-progress.c'))
>>>  util_ss.add(files('keyval.c'))
>>>  util_ss.add(files('crc32c.c'))
>>> +util_ss.add(files('crc-ccitt.c'))
>>
>> OK but we can restrict this to system-mode emulation, as user-mode
>> and tools don't require it.
> 
> Doesn't this get put in libutil, where it is only pulled from the archive when
> needed?  Also, libutil is built once, not per-target.

Hmm I just sent a pull request with this change squashed in.
Should I cancel it?

My view is we don't install libqemuutil.a anywhere, so why overload
building unused objects when some configuration don't need it?

Some of the configurations I test:
- build tools only
- build linux-user only

dbus.o and yank.o are also restricted to have_system.
various objects are restricted to have_block (which is
have_system or have_tools).

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Philippe Mathieu-Daudé]

On 1/24/21 9:07 PM, Richard Henderson wrote:
> On 1/24/21 8:59 AM, Philippe Mathieu-Daudé wrote:
>> On 1/23/21 11:39 AM, Bin Meng wrote:
>>> From: Bin Meng <bin.meng@windriver.com>
>>>
>>> Import CRC16 calculation routines from Linux kernel v5.10:
>>>
>>>   include/linux/crc-ccitt.h
>>>   lib/crc-ccitt.c
>>>
>>> to QEMU:
>>>
>>>   include/qemu/crc-ccitt.h
>>>   util/crc-ccitt.c
>>>
>>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>> Acked-by: Alistair Francis <alistair.francis@wdc.com>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>  include/qemu/crc-ccitt.h |  33 ++++++++++
>>>  util/crc-ccitt.c         | 127 +++++++++++++++++++++++++++++++++++++++
>>>  util/meson.build         |   1 +
>>>  3 files changed, 161 insertions(+)
>>>  create mode 100644 include/qemu/crc-ccitt.h
>>>  create mode 100644 util/crc-ccitt.c
>> ...
>>
>>> diff --git a/util/meson.build b/util/meson.build
>>> index 540a605b78..05a376ae02 100644
>>> --- a/util/meson.build
>>> +++ b/util/meson.build
>>> @@ -29,6 +29,7 @@ util_ss.add(files('qemu-config.c', 'notify.c'))
>>>  util_ss.add(files('qemu-option.c', 'qemu-progress.c'))
>>>  util_ss.add(files('keyval.c'))
>>>  util_ss.add(files('crc32c.c'))
>>> +util_ss.add(files('crc-ccitt.c'))
>>
>> OK but we can restrict this to system-mode emulation, as user-mode
>> and tools don't require it.
> 
> Doesn't this get put in libutil, where it is only pulled from the archive when
> needed?  Also, libutil is built once, not per-target.

Hmm I just sent a pull request with this change squashed in.
Should I cancel it?

My view is we don't install libqemuutil.a anywhere, so why overload
building unused objects when some configuration don't need it?

Some of the configurations I test:
- build tools only
- build linux-user only

dbus.o and yank.o are also restricted to have_system.
various objects are restricted to have_block (which is
have_system or have_tools).

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Richard Henderson]

On 1/24/21 10:24 AM, Philippe Mathieu-Daudé wrote:
> On 1/24/21 9:07 PM, Richard Henderson wrote:
>> Doesn't this get put in libutil, where it is only pulled from the archive when
>> needed?  Also, libutil is built once, not per-target.
> 
> Hmm I just sent a pull request with this change squashed in.
> Should I cancel it?

I guess not.

> My view is we don't install libqemuutil.a anywhere, so why overload
> building unused objects when some configuration don't need it?

My view is that util/meson.build should not be overly complicated with
conditionals.

If we were building objects per-target, that would be one thing.  If we were
doing --whole-archive, and including unused code in the executables, that would
be another thing.  But otherwise...


r~

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Richard Henderson]

On 1/24/21 10:24 AM, Philippe Mathieu-Daudé wrote:
> On 1/24/21 9:07 PM, Richard Henderson wrote:
>> Doesn't this get put in libutil, where it is only pulled from the archive when
>> needed?  Also, libutil is built once, not per-target.
> 
> Hmm I just sent a pull request with this change squashed in.
> Should I cancel it?

I guess not.

> My view is we don't install libqemuutil.a anywhere, so why overload
> building unused objects when some configuration don't need it?

My view is that util/meson.build should not be overly complicated with
conditionals.

If we were building objects per-target, that would be one thing.  If we were
doing --whole-archive, and including unused code in the executables, that would
be another thing.  But otherwise...


r~

Re: [PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Philippe Mathieu-Daudé]

On 1/24/21 10:41 PM, Richard Henderson wrote:
> On 1/24/21 10:24 AM, Philippe Mathieu-Daudé wrote:
>> On 1/24/21 9:07 PM, Richard Henderson wrote:
>>> Doesn't this get put in libutil, where it is only pulled from the archive when
>>> needed?  Also, libutil is built once, not per-target.
>>
>> Hmm I just sent a pull request with this change squashed in.
>> Should I cancel it?
> 
> I guess not.
> 
>> My view is we don't install libqemuutil.a anywhere, so why overload
>> building unused objects when some configuration don't need it?
> 
> My view is that util/meson.build should not be overly complicated with
> conditionals.

Well I do see an impact when building on slow hosts.

> If we were building objects per-target, that would be one thing.  If we were
> doing --whole-archive, and including unused code in the executables, that would
> be another thing.  But otherwise...

I understand your position :)

Thanks,

Phil.

[PATCH v2 07/25] hw/sd: ssi-sd: Suffix a data block with CRC16

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Per the SD spec, a valid data block is suffixed with a 16-bit CRC
generated by the standard CCITT polynomial x16+x12+x5+1. This part
is currently missing in the ssi-sd state machine. Without it, all
data block transfer fails in guest software because the expected
CRC16 is missing on the data out line.

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 hw/sd/ssi-sd.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 043e270066..8bccedfab2 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -17,6 +17,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/sd/sd.h"
 #include "qapi/error.h"
+#include "qemu/crc-ccitt.h"
 #include "qemu/module.h"
 #include "qom/object.h"
 
@@ -40,6 +41,7 @@ typedef enum {
     SSI_SD_RESPONSE,
     SSI_SD_DATA_START,
     SSI_SD_DATA_READ,
+    SSI_SD_DATA_CRC16,
 } ssi_sd_mode;
 
 struct ssi_sd_state {
@@ -48,6 +50,7 @@ struct ssi_sd_state {
     int cmd;
     uint8_t cmdarg[4];
     uint8_t response[5];
+    uint16_t crc16;
     int32_t arglen;
     int32_t response_pos;
     int32_t stopping;
@@ -194,12 +197,24 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
     case SSI_SD_DATA_START:
         DPRINTF("Start read block\n");
         s->mode = SSI_SD_DATA_READ;
+        s->response_pos = 0;
         return 0xfe;
     case SSI_SD_DATA_READ:
         val = sdbus_read_byte(&s->sdbus);
+        s->crc16 = crc_ccitt_false(s->crc16, (uint8_t *)&val, 1);
         if (!sdbus_data_ready(&s->sdbus)) {
             DPRINTF("Data read end\n");
+            s->mode = SSI_SD_DATA_CRC16;
+        }
+        return val;
+    case SSI_SD_DATA_CRC16:
+        val = (s->crc16 & 0xff00) >> 8;
+        s->crc16 <<= 8;
+        s->response_pos++;
+        if (s->response_pos == 2) {
+            DPRINTF("CRC16 read end\n");
             s->mode = SSI_SD_CMD;
+            s->response_pos = 0;
         }
         return val;
     }
@@ -237,6 +252,7 @@ static const VMStateDescription vmstate_ssi_sd = {
         VMSTATE_INT32(cmd, ssi_sd_state),
         VMSTATE_UINT8_ARRAY(cmdarg, ssi_sd_state, 4),
         VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
+        VMSTATE_UINT16(crc16, ssi_sd_state),
         VMSTATE_INT32(arglen, ssi_sd_state),
         VMSTATE_INT32(response_pos, ssi_sd_state),
         VMSTATE_INT32(stopping, ssi_sd_state),
@@ -288,6 +304,7 @@ static void ssi_sd_reset(DeviceState *dev)
     s->cmd = 0;
     memset(s->cmdarg, 0, sizeof(s->cmdarg));
     memset(s->response, 0, sizeof(s->response));
+    s->crc16 = 0;
     s->arglen = 0;
     s->response_pos = 0;
     s->stopping = 0;
-- 
2.25.1

Re: [PATCH v2 07/25] hw/sd: ssi-sd: Suffix a data block with CRC16

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Per the SD spec, a valid data block is suffixed with a 16-bit CRC
> generated by the standard CCITT polynomial x16+x12+x5+1. This part
> is currently missing in the ssi-sd state machine. Without it, all
> data block transfer fails in guest software because the expected
> CRC16 is missing on the data out line.
> 
> Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Acked-by: Alistair Francis <alistair.francis@wdc.com>
> ---
> 
> (no changes since v1)
> 
>  hw/sd/ssi-sd.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

Bumping migration version:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH v2 08/25] hw/sd: ssi-sd: Add a state representing Nac

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Per the "Physical Layer Specification Version 8.00" chapter 7.5.2,
"Data Read", there is a minimum 8 clock cycles (Nac) after the card
response and before data block shows up on the data out line. This
applies to both single and multiple block read operations.

Current implementation of single block read already satisfies the
timing requirement as in the RESPONSE state after all responses are
transferred the state remains unchanged. In the next 8 clock cycles
it jumps to DATA_START state if data is ready.

However we need an explicit state when expanding our support to
multiple block read in the future. Let's add a new state PREP_DATA
explicitly in the ssi-sd state machine to represent Nac.

Note we don't change the single block read state machine to let it
jump from RESPONSE state to DATA_START state as that effectively
generates a 16 clock cycles Nac, which might not be safe. As the
spec says the maximum Nac shall be calculated from several fields
encoded in the CSD register, we don't want to bother updating CSD
to ensure our Nac is within range to complicate things.

Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- new patch: add a state representing Nac

 hw/sd/ssi-sd.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 8bccedfab2..5763afeba0 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -39,6 +39,7 @@ typedef enum {
     SSI_SD_CMDARG,
     SSI_SD_PREP_RESP,
     SSI_SD_RESPONSE,
+    SSI_SD_PREP_DATA,
     SSI_SD_DATA_START,
     SSI_SD_DATA_READ,
     SSI_SD_DATA_CRC16,
@@ -194,6 +195,10 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
             s->mode = SSI_SD_CMD;
         }
         return 0xff;
+    case SSI_SD_PREP_DATA:
+        DPRINTF("Prepare data block (Nac)\n");
+        s->mode = SSI_SD_DATA_START;
+        return 0xff;
     case SSI_SD_DATA_START:
         DPRINTF("Start read block\n");
         s->mode = SSI_SD_DATA_READ;
-- 
2.25.1

Re: [PATCH v2 08/25] hw/sd: ssi-sd: Add a state representing Nac

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Per the "Physical Layer Specification Version 8.00" chapter 7.5.2,
> "Data Read", there is a minimum 8 clock cycles (Nac) after the card
> response and before data block shows up on the data out line. This
> applies to both single and multiple block read operations.
> 
> Current implementation of single block read already satisfies the
> timing requirement as in the RESPONSE state after all responses are
> transferred the state remains unchanged. In the next 8 clock cycles
> it jumps to DATA_START state if data is ready.
> 
> However we need an explicit state when expanding our support to
> multiple block read in the future. Let's add a new state PREP_DATA
> explicitly in the ssi-sd state machine to represent Nac.
> 
> Note we don't change the single block read state machine to let it
> jump from RESPONSE state to DATA_START state as that effectively
> generates a 16 clock cycles Nac, which might not be safe. As the
> spec says the maximum Nac shall be calculated from several fields
> encoded in the CSD register, we don't want to bother updating CSD
> to ensure our Nac is within range to complicate things.

As I don't have access to that part of the spec, I'm going to trust you.

Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - new patch: add a state representing Nac
> 
>  hw/sd/ssi-sd.c | 5 +++++
>  1 file changed, 5 insertions(+)

Re: [PATCH v2 08/25] hw/sd: ssi-sd: Add a state representing Nac

[Philippe Mathieu-Daudé]

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> Per the "Physical Layer Specification Version 8.00" chapter 7.5.2,
> "Data Read", there is a minimum 8 clock cycles (Nac) after the card
> response and before data block shows up on the data out line. This
> applies to both single and multiple block read operations.
> 
> Current implementation of single block read already satisfies the
> timing requirement as in the RESPONSE state after all responses are
> transferred the state remains unchanged. In the next 8 clock cycles
> it jumps to DATA_START state if data is ready.
> 
> However we need an explicit state when expanding our support to
> multiple block read in the future. Let's add a new state PREP_DATA
> explicitly in the ssi-sd state machine to represent Nac.
> 
> Note we don't change the single block read state machine to let it
> jump from RESPONSE state to DATA_START state as that effectively
> generates a 16 clock cycles Nac, which might not be safe. As the
> spec says the maximum Nac shall be calculated from several fields
> encoded in the CSD register, we don't want to bother updating CSD
> to ensure our Nac is within range to complicate things.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - new patch: add a state representing Nac
> 
>  hw/sd/ssi-sd.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 8bccedfab2..5763afeba0 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -39,6 +39,7 @@ typedef enum {
>      SSI_SD_CMDARG,
>      SSI_SD_PREP_RESP,
>      SSI_SD_RESPONSE,
> +    SSI_SD_PREP_DATA,

Hmm yet another change breaking migration.

>      SSI_SD_DATA_START,
>      SSI_SD_DATA_READ,
>      SSI_SD_DATA_CRC16,

[PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This fixes the wrong command index for STOP_TRANSMISSION, the
required command to interrupt the multiple block read command,
in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- Make this fix a separate patch from the CMD18 support

 hw/sd/ssi-sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 5763afeba0..9e2f13374a 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
     ssi_sd_state *s = SSI_SD(dev);
 
     /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
-    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
+    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
         s->mode = SSI_SD_CMD;
         /* There must be at least one byte delay before the card responds.  */
         s->stopping = 1;
-- 
2.25.1

Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Philippe Mathieu-Daudé]

On 1/23/21 11:40 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> This fixes the wrong command index for STOP_TRANSMISSION, the
> required command to interrupt the multiple block read command,
> in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> 
> Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - Make this fix a separate patch from the CMD18 support
> 
>  hw/sd/ssi-sd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 5763afeba0..9e2f13374a 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
>      ssi_sd_state *s = SSI_SD(dev);
>  
>      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {

Patch is correct, but I wonder if we couldn't improve using instead:

     if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {

>          s->mode = SSI_SD_CMD;
>          /* There must be at least one byte delay before the card responds.  */
>          s->stopping = 1;
> 

Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Philippe Mathieu-Daudé]

On 1/24/21 6:33 PM, Philippe Mathieu-Daudé wrote:
> On 1/23/21 11:40 AM, Bin Meng wrote:
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> This fixes the wrong command index for STOP_TRANSMISSION, the
>> required command to interrupt the multiple block read command,
>> in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
>>
>> Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>
>> ---
>>
>> Changes in v2:
>> - Make this fix a separate patch from the CMD18 support
>>
>>  hw/sd/ssi-sd.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
>> index 5763afeba0..9e2f13374a 100644
>> --- a/hw/sd/ssi-sd.c
>> +++ b/hw/sd/ssi-sd.c
>> @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
>>      ssi_sd_state *s = SSI_SD(dev);
>>  
>>      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
>> -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
>> +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
> 
> Patch is correct,

Forgot to amend:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> but I wonder if we couldn't improve using instead:
> 
>      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
> 
>>          s->mode = SSI_SD_CMD;
>>          /* There must be at least one byte delay before the card responds.  */
>>          s->stopping = 1;
>>
> 

Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Bin Meng]

On Mon, Jan 25, 2021 at 1:33 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/23/21 11:40 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This fixes the wrong command index for STOP_TRANSMISSION, the
> > required command to interrupt the multiple block read command,
> > in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> >
> > Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > Changes in v2:
> > - Make this fix a separate patch from the CMD18 support
> >
> >  hw/sd/ssi-sd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > index 5763afeba0..9e2f13374a 100644
> > --- a/hw/sd/ssi-sd.c
> > +++ b/hw/sd/ssi-sd.c
> > @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
> >      ssi_sd_state *s = SSI_SD(dev);
> >
> >      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> > -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> > +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
>
> Patch is correct, but I wonder if we couldn't improve using instead:
>
>      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
>

(val & 0x3f == 12) isn't correct because it can catch values equal to
0x7c or 0xfc.

Regards,
Bin

Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Bin Meng]

On Mon, Jan 25, 2021 at 1:33 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/23/21 11:40 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This fixes the wrong command index for STOP_TRANSMISSION, the
> > required command to interrupt the multiple block read command,
> > in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> >
> > Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> >
> > ---
> >
> > Changes in v2:
> > - Make this fix a separate patch from the CMD18 support
> >
> >  hw/sd/ssi-sd.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > index 5763afeba0..9e2f13374a 100644
> > --- a/hw/sd/ssi-sd.c
> > +++ b/hw/sd/ssi-sd.c
> > @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
> >      ssi_sd_state *s = SSI_SD(dev);
> >
> >      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> > -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> > +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
>
> Patch is correct, but I wonder if we couldn't improve using instead:
>
>      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
>

(val & 0x3f == 12) isn't correct because it can catch values equal to
0x7c or 0xfc.

Regards,
Bin

Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Bin Meng]

On Mon, Jan 25, 2021 at 8:33 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, Jan 25, 2021 at 1:33 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > This fixes the wrong command index for STOP_TRANSMISSION, the
> > > required command to interrupt the multiple block read command,
> > > in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> > >
> > > Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Make this fix a separate patch from the CMD18 support
> > >
> > >  hw/sd/ssi-sd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > > index 5763afeba0..9e2f13374a 100644
> > > --- a/hw/sd/ssi-sd.c
> > > +++ b/hw/sd/ssi-sd.c
> > > @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
> > >      ssi_sd_state *s = SSI_SD(dev);
> > >
> > >      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> > > -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> > > +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
> >
> > Patch is correct, but I wonder if we couldn't improve using instead:
> >
> >      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
> >
>
> (val & 0x3f == 12) isn't correct because it can catch values equal to
> 0x7c or 0xfc.

Sorry, wrong calculation. It should read:

It can catch values 0x0c, 0x8c, 0xcc.

Regards,
Bin

Re: [PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Bin Meng]

On Mon, Jan 25, 2021 at 8:33 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, Jan 25, 2021 at 1:33 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > This fixes the wrong command index for STOP_TRANSMISSION, the
> > > required command to interrupt the multiple block read command,
> > > in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).
> > >
> > > Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
> > > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > > - Make this fix a separate patch from the CMD18 support
> > >
> > >  hw/sd/ssi-sd.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> > > index 5763afeba0..9e2f13374a 100644
> > > --- a/hw/sd/ssi-sd.c
> > > +++ b/hw/sd/ssi-sd.c
> > > @@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
> > >      ssi_sd_state *s = SSI_SD(dev);
> > >
> > >      /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
> > > -    if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
> > > +    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
> >
> > Patch is correct, but I wonder if we couldn't improve using instead:
> >
> >      if (s->mode == SSI_SD_DATA_READ && ((val & 0x3f) == 12)) {
> >
>
> (val & 0x3f == 12) isn't correct because it can catch values equal to
> 0x7c or 0xfc.

Sorry, wrong calculation. It should read:

It can catch values 0x0c, 0x8c, 0xcc.

Regards,
Bin

[PATCH v2 10/25] hw/sd: ssi-sd: Support multiple block read

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

In the case of a multiple block read operation every transferred
block has its suffix of CRC16. Update the state machine logic to
handle multiple block read.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Fix 2 typos in the commit message
- Add a comment block to explain the CMD12 timing
- Catch CMD12 in all of the data read states per the timing requirement

 hw/sd/ssi-sd.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 9e2f13374a..c1532b004b 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -52,6 +52,7 @@ struct ssi_sd_state {
     uint8_t cmdarg[4];
     uint8_t response[5];
     uint16_t crc16;
+    int32_t read_bytes;
     int32_t arglen;
     int32_t response_pos;
     int32_t stopping;
@@ -82,11 +83,26 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
     ssi_sd_state *s = SSI_SD(dev);
 
-    /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
-    if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
-        s->mode = SSI_SD_CMD;
-        /* There must be at least one byte delay before the card responds.  */
-        s->stopping = 1;
+    /*
+     * Special case: allow CMD12 (STOP TRANSMISSION) while reading data.
+     *
+     * See "Physical Layer Specification Version 8.00" chapter 7.5.2.2,
+     * to avoid conflict between CMD12 response and next data block,
+     * timing of CMD12 should be controlled as follows:
+     *
+     * - CMD12 issued at the timing that end bit of CMD12 and end bit of
+     *   data block is overlapped
+     * - CMD12 issued after one clock cycle after host receives a token
+     *   (either Start Block token or Data Error token)
+     *
+     * We need to catch CMD12 in all of the data read states.
+     */
+    if (s->mode >= SSI_SD_PREP_DATA && s->mode <= SSI_SD_DATA_CRC16) {
+        if (val == 0x4c) {
+            s->mode = SSI_SD_CMD;
+            /* There must be at least one byte delay before the card responds */
+            s->stopping = 1;
+        }
     }
 
     switch (s->mode) {
@@ -206,8 +222,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
         return 0xfe;
     case SSI_SD_DATA_READ:
         val = sdbus_read_byte(&s->sdbus);
+        s->read_bytes++;
         s->crc16 = crc_ccitt_false(s->crc16, (uint8_t *)&val, 1);
-        if (!sdbus_data_ready(&s->sdbus)) {
+        if (!sdbus_data_ready(&s->sdbus) || s->read_bytes == 512) {
             DPRINTF("Data read end\n");
             s->mode = SSI_SD_DATA_CRC16;
         }
@@ -218,7 +235,12 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
         s->response_pos++;
         if (s->response_pos == 2) {
             DPRINTF("CRC16 read end\n");
-            s->mode = SSI_SD_CMD;
+            if (s->read_bytes == 512 && s->cmd != 17) {
+                s->mode = SSI_SD_PREP_DATA;
+            } else {
+                s->mode = SSI_SD_CMD;
+            }
+            s->read_bytes = 0;
             s->response_pos = 0;
         }
         return val;
@@ -258,6 +280,7 @@ static const VMStateDescription vmstate_ssi_sd = {
         VMSTATE_UINT8_ARRAY(cmdarg, ssi_sd_state, 4),
         VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
         VMSTATE_UINT16(crc16, ssi_sd_state),
+        VMSTATE_INT32(read_bytes, ssi_sd_state),
         VMSTATE_INT32(arglen, ssi_sd_state),
         VMSTATE_INT32(response_pos, ssi_sd_state),
         VMSTATE_INT32(stopping, ssi_sd_state),
@@ -310,6 +333,7 @@ static void ssi_sd_reset(DeviceState *dev)
     memset(s->cmdarg, 0, sizeof(s->cmdarg));
     memset(s->response, 0, sizeof(s->response));
     s->crc16 = 0;
+    s->read_bytes = 0;
     s->arglen = 0;
     s->response_pos = 0;
     s->stopping = 0;
-- 
2.25.1

[PATCH v2 11/25] hw/sd: ssi-sd: Use macros for the dummy value and tokens in the transfer

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

At present the codes use hardcoded numbers (0xff/0xfe) for the dummy
value and block start token. Replace them with macros.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Move multiple write token definitions out of this patch

 hw/sd/ssi-sd.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index c1532b004b..75e76cf87a 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -79,6 +79,12 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 #define SSI_SDR_ADDRESS_ERROR   0x2000
 #define SSI_SDR_PARAMETER_ERROR 0x4000
 
+/* single block read/write, multiple block read */
+#define SSI_TOKEN_SINGLE        0xfe
+
+/* dummy value - don't care */
+#define SSI_DUMMY               0xff
+
 static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
     ssi_sd_state *s = SSI_SD(dev);
@@ -107,14 +113,14 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 
     switch (s->mode) {
     case SSI_SD_CMD:
-        if (val == 0xff) {
+        if (val == SSI_DUMMY) {
             DPRINTF("NULL command\n");
-            return 0xff;
+            return SSI_DUMMY;
         }
         s->cmd = val & 0x3f;
         s->mode = SSI_SD_CMDARG;
         s->arglen = 0;
-        return 0xff;
+        return SSI_DUMMY;
     case SSI_SD_CMDARG:
         if (s->arglen == 4) {
             SDRequest request;
@@ -189,15 +195,15 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
         } else {
             s->cmdarg[s->arglen++] = val;
         }
-        return 0xff;
+        return SSI_DUMMY;
     case SSI_SD_PREP_RESP:
         DPRINTF("Prepare card response (Ncr)\n");
         s->mode = SSI_SD_RESPONSE;
-        return 0xff;
+        return SSI_DUMMY;
     case SSI_SD_RESPONSE:
         if (s->stopping) {
             s->stopping = 0;
-            return 0xff;
+            return SSI_DUMMY;
         }
         if (s->response_pos < s->arglen) {
             DPRINTF("Response 0x%02x\n", s->response[s->response_pos]);
@@ -210,16 +216,16 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
             DPRINTF("End of command\n");
             s->mode = SSI_SD_CMD;
         }
-        return 0xff;
+        return SSI_DUMMY;
     case SSI_SD_PREP_DATA:
         DPRINTF("Prepare data block (Nac)\n");
         s->mode = SSI_SD_DATA_START;
-        return 0xff;
+        return SSI_DUMMY;
     case SSI_SD_DATA_START:
         DPRINTF("Start read block\n");
         s->mode = SSI_SD_DATA_READ;
         s->response_pos = 0;
-        return 0xfe;
+        return SSI_TOKEN_SINGLE;
     case SSI_SD_DATA_READ:
         val = sdbus_read_byte(&s->sdbus);
         s->read_bytes++;
@@ -246,7 +252,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
         return val;
     }
     /* Should never happen.  */
-    return 0xff;
+    return SSI_DUMMY;
 }
 
 static int ssi_sd_post_load(void *opaque, int version_id)
-- 
2.25.1

Re: [PATCH v2 11/25] hw/sd: ssi-sd: Use macros for the dummy value and tokens in the transfer

[Philippe Mathieu-Daudé]

On 1/23/21 11:40 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> At present the codes use hardcoded numbers (0xff/0xfe) for the dummy
> value and block start token. Replace them with macros.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> ---
> 
> Changes in v2:
> - Move multiple write token definitions out of this patch
> 
>  hw/sd/ssi-sd.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)

Again:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
https://www.mail-archive.com/qemu-block@nongnu.org/msg78880.html

[PATCH v2 12/25] hw/sd: sd: Remove duplicated codes in single/multiple block read/write

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

The single block read (CMD17) codes are the same as the multiple
block read (CMD18). Merge them into one. The same applies to single
block write (CMD24) and multiple block write (CMD25).

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 hw/sd/sd.c | 47 -----------------------------------------------
 1 file changed, 47 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b3952514fe..09753359bb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1181,24 +1181,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 17:	/* CMD17:  READ_SINGLE_BLOCK */
-        switch (sd->state) {
-        case sd_transfer_state:
-
-            if (addr + sd->blk_len > sd->size) {
-                sd->card_status |= ADDRESS_ERROR;
-                return sd_r1;
-            }
-
-            sd->state = sd_sendingdata_state;
-            sd->data_start = addr;
-            sd->data_offset = 0;
-            return sd_r1;
-
-        default:
-            break;
-        }
-        break;
-
     case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
@@ -1245,35 +1227,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Block write commands (Class 4) */
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
-        switch (sd->state) {
-        case sd_transfer_state:
-            /* Writing in SPI mode not implemented.  */
-            if (sd->spi)
-                break;
-
-            if (addr + sd->blk_len > sd->size) {
-                sd->card_status |= ADDRESS_ERROR;
-                return sd_r1;
-            }
-
-            sd->state = sd_receivingdata_state;
-            sd->data_start = addr;
-            sd->data_offset = 0;
-            sd->blk_written = 0;
-
-            if (sd_wp_addr(sd, sd->data_start)) {
-                sd->card_status |= WP_VIOLATION;
-            }
-            if (sd->csd[14] & 0x30) {
-                sd->card_status |= WP_VIOLATION;
-            }
-            return sd_r1;
-
-        default:
-            break;
-        }
-        break;
-
     case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
-- 
2.25.1

[PATCH v2 13/25] hw/sd: sd: Allow single/multiple block write for SPI mode

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

At present the single/multiple block write in SPI mode is blocked
by sd_normal_command(). Remove the limitation.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 hw/sd/sd.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 09753359bb..946036d87c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1230,9 +1230,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
         switch (sd->state) {
         case sd_transfer_state:
-            /* Writing in SPI mode not implemented.  */
-            if (sd->spi)
-                break;
 
             if (addr + sd->blk_len > sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
-- 
2.25.1

[PATCH v2 14/25] hw/sd: sd.h: Cosmetic change of using spaces

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

QEMU coding convention prefers spaces over tabs.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Correct the "coding" typo in the commit message

 include/hw/sd/sd.h | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 59d108d453..05ef9b73e5 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -33,27 +33,27 @@
 #include "hw/qdev-core.h"
 #include "qom/object.h"
 
-#define OUT_OF_RANGE		(1 << 31)
-#define ADDRESS_ERROR		(1 << 30)
-#define BLOCK_LEN_ERROR		(1 << 29)
-#define ERASE_SEQ_ERROR		(1 << 28)
-#define ERASE_PARAM		(1 << 27)
-#define WP_VIOLATION		(1 << 26)
-#define CARD_IS_LOCKED		(1 << 25)
-#define LOCK_UNLOCK_FAILED	(1 << 24)
-#define COM_CRC_ERROR		(1 << 23)
-#define ILLEGAL_COMMAND		(1 << 22)
-#define CARD_ECC_FAILED		(1 << 21)
-#define CC_ERROR		(1 << 20)
-#define SD_ERROR		(1 << 19)
-#define CID_CSD_OVERWRITE	(1 << 16)
-#define WP_ERASE_SKIP		(1 << 15)
-#define CARD_ECC_DISABLED	(1 << 14)
-#define ERASE_RESET		(1 << 13)
-#define CURRENT_STATE		(7 << 9)
-#define READY_FOR_DATA		(1 << 8)
-#define APP_CMD			(1 << 5)
-#define AKE_SEQ_ERROR		(1 << 3)
+#define OUT_OF_RANGE            (1 << 31)
+#define ADDRESS_ERROR           (1 << 30)
+#define BLOCK_LEN_ERROR         (1 << 29)
+#define ERASE_SEQ_ERROR         (1 << 28)
+#define ERASE_PARAM             (1 << 27)
+#define WP_VIOLATION            (1 << 26)
+#define CARD_IS_LOCKED          (1 << 25)
+#define LOCK_UNLOCK_FAILED      (1 << 24)
+#define COM_CRC_ERROR           (1 << 23)
+#define ILLEGAL_COMMAND         (1 << 22)
+#define CARD_ECC_FAILED         (1 << 21)
+#define CC_ERROR                (1 << 20)
+#define SD_ERROR                (1 << 19)
+#define CID_CSD_OVERWRITE       (1 << 16)
+#define WP_ERASE_SKIP           (1 << 15)
+#define CARD_ECC_DISABLED       (1 << 14)
+#define ERASE_RESET             (1 << 13)
+#define CURRENT_STATE           (7 << 9)
+#define READY_FOR_DATA          (1 << 8)
+#define APP_CMD                 (1 << 5)
+#define AKE_SEQ_ERROR           (1 << 3)
 
 enum SDPhySpecificationVersion {
     SD_PHY_SPECv1_10_VERS     = 1,
-- 
2.25.1

Re: [PATCH v2 14/25] hw/sd: sd.h: Cosmetic change of using spaces

[Philippe Mathieu-Daudé]

On 1/23/21 11:40 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> QEMU coding convention prefers spaces over tabs.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> ---
> 
> Changes in v2:
> - Correct the "coding" typo in the commit message
> 
>  include/hw/sd/sd.h | 42 +++++++++++++++++++++---------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 59d108d453..05ef9b73e5 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -33,27 +33,27 @@
>  #include "hw/qdev-core.h"
>  #include "qom/object.h"
>  
> -#define OUT_OF_RANGE		(1 << 31)
> -#define ADDRESS_ERROR		(1 << 30)
> -#define BLOCK_LEN_ERROR		(1 << 29)
> -#define ERASE_SEQ_ERROR		(1 << 28)
> -#define ERASE_PARAM		(1 << 27)
> -#define WP_VIOLATION		(1 << 26)
> -#define CARD_IS_LOCKED		(1 << 25)
> -#define LOCK_UNLOCK_FAILED	(1 << 24)
> -#define COM_CRC_ERROR		(1 << 23)
> -#define ILLEGAL_COMMAND		(1 << 22)
> -#define CARD_ECC_FAILED		(1 << 21)
> -#define CC_ERROR		(1 << 20)
> -#define SD_ERROR		(1 << 19)
> -#define CID_CSD_OVERWRITE	(1 << 16)
> -#define WP_ERASE_SKIP		(1 << 15)
> -#define CARD_ECC_DISABLED	(1 << 14)
> -#define ERASE_RESET		(1 << 13)
> -#define CURRENT_STATE		(7 << 9)
> -#define READY_FOR_DATA		(1 << 8)
> -#define APP_CMD			(1 << 5)
> -#define AKE_SEQ_ERROR		(1 << 3)
> +#define OUT_OF_RANGE            (1 << 31)
> +#define ADDRESS_ERROR           (1 << 30)
> +#define BLOCK_LEN_ERROR         (1 << 29)
> +#define ERASE_SEQ_ERROR         (1 << 28)
> +#define ERASE_PARAM             (1 << 27)
> +#define WP_VIOLATION            (1 << 26)
> +#define CARD_IS_LOCKED          (1 << 25)
> +#define LOCK_UNLOCK_FAILED      (1 << 24)
> +#define COM_CRC_ERROR           (1 << 23)
> +#define ILLEGAL_COMMAND         (1 << 22)
> +#define CARD_ECC_FAILED         (1 << 21)
> +#define CC_ERROR                (1 << 20)
> +#define SD_ERROR                (1 << 19)
> +#define CID_CSD_OVERWRITE       (1 << 16)
> +#define WP_ERASE_SKIP           (1 << 15)
> +#define CARD_ECC_DISABLED       (1 << 14)
> +#define ERASE_RESET             (1 << 13)
> +#define CURRENT_STATE           (7 << 9)
> +#define READY_FOR_DATA          (1 << 8)
> +#define APP_CMD                 (1 << 5)
> +#define AKE_SEQ_ERROR           (1 << 3)

The plan was to use the REGISTERFIELD definitions
we already have in sd.c and simply remove these:

FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
FIELD(CSR, APP_CMD,                     5,  1)
FIELD(CSR, FX_EVENT,                    6,  1)
FIELD(CSR, READY_FOR_DATA,              8,  1)
FIELD(CSR, CURRENT_STATE,               9,  4)
FIELD(CSR, ERASE_RESET,                13,  1)
FIELD(CSR, CARD_ECC_DISABLED,          14,  1)
FIELD(CSR, WP_ERASE_SKIP,              15,  1)
FIELD(CSR, CSD_OVERWRITE,              16,  1)
FIELD(CSR, DEFERRED_RESPONSE,          17,  1)
FIELD(CSR, ERROR,                      19,  1)
FIELD(CSR, CC_ERROR,                   20,  1)
FIELD(CSR, CARD_ECC_FAILED,            21,  1)
FIELD(CSR, ILLEGAL_COMMAND,            22,  1)
FIELD(CSR, COM_CRC_ERROR,              23,  1)
FIELD(CSR, LOCK_UNLOCK_FAILED,         24,  1)
FIELD(CSR, CARD_IS_LOCKED,             25,  1)
FIELD(CSR, WP_VIOLATION,               26,  1)
FIELD(CSR, ERASE_PARAM,                27,  1)
FIELD(CSR, ERASE_SEQ_ERROR,            28,  1)
FIELD(CSR, BLOCK_LEN_ERROR,            29,  1)
FIELD(CSR, ADDRESS_ERROR,              30,  1)
FIELD(CSR, OUT_OF_RANGE,               31,  1)

Anyway, we never got there...
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

[PATCH v2 15/25] hw/sd: Introduce receive_ready() callback

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

At present there is a data_ready() callback for the SD data read
path. Let's add a receive_ready() for the SD data write path.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---

(no changes since v1)

 include/hw/sd/sd.h |  2 ++
 hw/sd/core.c       | 13 +++++++++++++
 hw/sd/sd.c         |  6 ++++++
 3 files changed, 21 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 05ef9b73e5..47360ba4ee 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -116,6 +116,7 @@ struct SDCardClass {
      * Return: byte value read
      */
     uint8_t (*read_byte)(SDState *sd);
+    bool (*receive_ready)(SDState *sd);
     bool (*data_ready)(SDState *sd);
     void (*set_voltage)(SDState *sd, uint16_t millivolts);
     uint8_t (*get_dat_lines)(SDState *sd);
@@ -187,6 +188,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length);
  * Read multiple bytes of data on the data lines of a SD bus.
  */
 void sdbus_read_data(SDBus *sdbus, void *buf, size_t length);
+bool sdbus_receive_ready(SDBus *sd);
 bool sdbus_data_ready(SDBus *sd);
 bool sdbus_get_inserted(SDBus *sd);
 bool sdbus_get_readonly(SDBus *sd);
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 08c93b5903..30ee62c510 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -160,6 +160,19 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
     }
 }
 
+bool sdbus_receive_ready(SDBus *sdbus)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        return sc->receive_ready(card);
+    }
+
+    return false;
+}
+
 bool sdbus_data_ready(SDBus *sdbus)
 {
     SDState *card = get_card(sdbus);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 946036d87c..c99c0e93bb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2037,6 +2037,11 @@ uint8_t sd_read_byte(SDState *sd)
     return ret;
 }
 
+static bool sd_receive_ready(SDState *sd)
+{
+    return sd->state == sd_receivingdata_state;
+}
+
 static bool sd_data_ready(SDState *sd)
 {
     return sd->state == sd_sendingdata_state;
@@ -2147,6 +2152,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
     sc->do_command = sd_do_command;
     sc->write_byte = sd_write_byte;
     sc->read_byte = sd_read_byte;
+    sc->receive_ready = sd_receive_ready;
     sc->data_ready = sd_data_ready;
     sc->enable = sd_enable;
     sc->get_inserted = sd_get_inserted;
-- 
2.25.1

[PATCH v2 16/25] hw/sd: ssi-sd: Support single block write

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Add 2 more states for the block write operation. The SPI host needs
to send a data start token to start the transfer, and the data block
written to the card will be acknowledged by a data response token.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Correct the "token" typo in the commit message
- Add 'write_bytes' in vmstate_ssi_sd

 hw/sd/ssi-sd.c | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 75e76cf87a..240cfd919c 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -43,6 +43,8 @@ typedef enum {
     SSI_SD_DATA_START,
     SSI_SD_DATA_READ,
     SSI_SD_DATA_CRC16,
+    SSI_SD_DATA_WRITE,
+    SSI_SD_SKIP_CRC16,
 } ssi_sd_mode;
 
 struct ssi_sd_state {
@@ -53,6 +55,7 @@ struct ssi_sd_state {
     uint8_t response[5];
     uint16_t crc16;
     int32_t read_bytes;
+    int32_t write_bytes;
     int32_t arglen;
     int32_t response_pos;
     int32_t stopping;
@@ -85,6 +88,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 /* dummy value - don't care */
 #define SSI_DUMMY               0xff
 
+/* data accepted */
+#define DATA_RESPONSE_ACCEPTED  0x05
+
 static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
     ssi_sd_state *s = SSI_SD(dev);
@@ -113,10 +119,17 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 
     switch (s->mode) {
     case SSI_SD_CMD:
-        if (val == SSI_DUMMY) {
+        switch (val) {
+        case SSI_DUMMY:
             DPRINTF("NULL command\n");
             return SSI_DUMMY;
+            break;
+        case SSI_TOKEN_SINGLE:
+            DPRINTF("Start write block\n");
+            s->mode = SSI_SD_DATA_WRITE;
+            return SSI_DUMMY;
         }
+
         s->cmd = val & 0x3f;
         s->mode = SSI_SD_CMDARG;
         s->arglen = 0;
@@ -250,6 +263,27 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
             s->response_pos = 0;
         }
         return val;
+    case SSI_SD_DATA_WRITE:
+        sdbus_write_byte(&s->sdbus, val);
+        s->write_bytes++;
+        if (!sdbus_receive_ready(&s->sdbus) || s->write_bytes == 512) {
+            DPRINTF("Data write end\n");
+            s->mode = SSI_SD_SKIP_CRC16;
+            s->response_pos = 0;
+        }
+        return val;
+    case SSI_SD_SKIP_CRC16:
+        /* we don't verify the crc16 */
+        s->response_pos++;
+        if (s->response_pos == 2) {
+            DPRINTF("CRC16 receive end\n");
+            s->mode = SSI_SD_RESPONSE;
+            s->write_bytes = 0;
+            s->arglen = 1;
+            s->response[0] = DATA_RESPONSE_ACCEPTED;
+            s->response_pos = 0;
+        }
+        return SSI_DUMMY;
     }
     /* Should never happen.  */
     return SSI_DUMMY;
@@ -287,6 +321,7 @@ static const VMStateDescription vmstate_ssi_sd = {
         VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
         VMSTATE_UINT16(crc16, ssi_sd_state),
         VMSTATE_INT32(read_bytes, ssi_sd_state),
+        VMSTATE_INT32(write_bytes, ssi_sd_state),
         VMSTATE_INT32(arglen, ssi_sd_state),
         VMSTATE_INT32(response_pos, ssi_sd_state),
         VMSTATE_INT32(stopping, ssi_sd_state),
@@ -340,6 +375,7 @@ static void ssi_sd_reset(DeviceState *dev)
     memset(s->response, 0, sizeof(s->response));
     s->crc16 = 0;
     s->read_bytes = 0;
+    s->write_bytes = 0;
     s->arglen = 0;
     s->response_pos = 0;
     s->stopping = 0;
-- 
2.25.1

[PATCH v2 17/25] hw/sd: ssi-sd: Support multiple block write

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

For a multiple block write operation, each block begins with a multi
write start token. Unlike the SD mode that the multiple block write
ends when receiving a STOP_TRAN command (CMD12), a special stop tran
token is used to signal the card.

Emulating this by manually sending a CMD12 to the SD card core, to
bring it out of the receiving data state.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Acked-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Correct the "token" typo in the commit message
- Introduce multiple write token definitions in this patch

 hw/sd/ssi-sd.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 240cfd919c..ee4fbc3dfe 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -82,6 +82,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 #define SSI_SDR_ADDRESS_ERROR   0x2000
 #define SSI_SDR_PARAMETER_ERROR 0x4000
 
+/* multiple block write */
+#define SSI_TOKEN_MULTI_WRITE   0xfc
+/* terminate multiple block write */
+#define SSI_TOKEN_STOP_TRAN     0xfd
 /* single block read/write, multiple block read */
 #define SSI_TOKEN_SINGLE        0xfe
 
@@ -94,6 +98,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
     ssi_sd_state *s = SSI_SD(dev);
+    SDRequest request;
+    uint8_t longresp[16];
 
     /*
      * Special case: allow CMD12 (STOP TRANSMISSION) while reading data.
@@ -125,9 +131,31 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
             return SSI_DUMMY;
             break;
         case SSI_TOKEN_SINGLE:
+        case SSI_TOKEN_MULTI_WRITE:
             DPRINTF("Start write block\n");
             s->mode = SSI_SD_DATA_WRITE;
             return SSI_DUMMY;
+        case SSI_TOKEN_STOP_TRAN:
+            DPRINTF("Stop multiple write\n");
+
+            /* manually issue cmd12 to stop the transfer */
+            request.cmd = 12;
+            request.arg = 0;
+            s->arglen = sdbus_do_command(&s->sdbus, &request, longresp);
+            if (s->arglen <= 0) {
+                s->arglen = 1;
+                /* a zero value indicates the card is busy */
+                s->response[0] = 0;
+                DPRINTF("SD card busy\n");
+            } else {
+                s->arglen = 1;
+                /* a non-zero value indicates the card is ready */
+                s->response[0] = SSI_DUMMY;
+            }
+
+            s->mode = SSI_SD_RESPONSE;
+            s->response_pos = 0;
+            return SSI_DUMMY;
         }
 
         s->cmd = val & 0x3f;
@@ -136,8 +164,6 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
         return SSI_DUMMY;
     case SSI_SD_CMDARG:
         if (s->arglen == 4) {
-            SDRequest request;
-            uint8_t longresp[16];
             /* FIXME: Check CRC.  */
             request.cmd = s->cmd;
             request.arg = ldl_be_p(s->cmdarg);
-- 
2.25.1

[PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

With all these fixes and improvements, there is no way for the
VMStateDescription to keep backward compatibility. We will have
to bump up version ids.

The s->mode check in the post_load() hook is also updated.

Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- new patch: bump up version ids of VMStateDescription

 hw/sd/ssi-sd.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index ee4fbc3dfe..0c507f3ec5 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -4,6 +4,11 @@
  * Copyright (c) 2007-2009 CodeSourcery.
  * Written by Paul Brook
  *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ * Improved by Bin Meng <bin.meng@windriver.com>
+ *
+ * Validated with U-Boot v2021.01 and Linux v5.10 mmc_spi driver
+ *
  * This code is licensed under the GNU GPL v2.
  *
  * Contributions after 2012-01-13 are licensed under the terms of the
@@ -319,7 +324,7 @@ static int ssi_sd_post_load(void *opaque, int version_id)
 {
     ssi_sd_state *s = (ssi_sd_state *)opaque;
 
-    if (s->mode > SSI_SD_DATA_READ) {
+    if (s->mode > SSI_SD_SKIP_CRC16) {
         return -EINVAL;
     }
     if (s->mode == SSI_SD_CMDARG &&
@@ -337,8 +342,8 @@ static int ssi_sd_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_ssi_sd = {
     .name = "ssi_sd",
-    .version_id = 2,
-    .minimum_version_id = 2,
+    .version_id = 3,
+    .minimum_version_id = 3,
     .post_load = ssi_sd_post_load,
     .fields = (VMStateField []) {
         VMSTATE_UINT32(mode, ssi_sd_state),
-- 
2.25.1

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Philippe Mathieu-Daudé]

On 1/23/21 11:40 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> With all these fixes and improvements, there is no way for the
> VMStateDescription to keep backward compatibility. We will have
> to bump up version ids.

Unfortunately this breaks bisectability (think about downstream
distributions cherry-picking patches individually).

I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
(Cc'ed David in case). Could you respin increasing the version
on each VMState change?

> 
> The s->mode check in the post_load() hook is also updated.
> 
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> 
> ---
> 
> Changes in v2:
> - new patch: bump up version ids of VMStateDescription
> 
>  hw/sd/ssi-sd.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index ee4fbc3dfe..0c507f3ec5 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -4,6 +4,11 @@
>   * Copyright (c) 2007-2009 CodeSourcery.
>   * Written by Paul Brook
>   *
> + * Copyright (c) 2021 Wind River Systems, Inc.
> + * Improved by Bin Meng <bin.meng@windriver.com>
> + *
> + * Validated with U-Boot v2021.01 and Linux v5.10 mmc_spi driver
> + *
>   * This code is licensed under the GNU GPL v2.
>   *
>   * Contributions after 2012-01-13 are licensed under the terms of the
> @@ -319,7 +324,7 @@ static int ssi_sd_post_load(void *opaque, int version_id)
>  {
>      ssi_sd_state *s = (ssi_sd_state *)opaque;
>  
> -    if (s->mode > SSI_SD_DATA_READ) {
> +    if (s->mode > SSI_SD_SKIP_CRC16) {
>          return -EINVAL;
>      }
>      if (s->mode == SSI_SD_CMDARG &&
> @@ -337,8 +342,8 @@ static int ssi_sd_post_load(void *opaque, int version_id)
>  
>  static const VMStateDescription vmstate_ssi_sd = {
>      .name = "ssi_sd",
> -    .version_id = 2,
> -    .minimum_version_id = 2,
> +    .version_id = 3,
> +    .minimum_version_id = 3,
>      .post_load = ssi_sd_post_load,
>      .fields = (VMStateField []) {
>          VMSTATE_UINT32(mode, ssi_sd_state),
> 

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Philippe Mathieu-Daudé]

On 1/24/21 5:59 PM, Philippe Mathieu-Daudé wrote:
> On 1/23/21 11:40 AM, Bin Meng wrote:
>> From: Bin Meng <bin.meng@windriver.com>
>>
>> With all these fixes and improvements, there is no way for the
>> VMStateDescription to keep backward compatibility. We will have
>> to bump up version ids.
> 
> Unfortunately this breaks bisectability (think about downstream
> distributions cherry-picking patches individually).
> 
> I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> (Cc'ed David in case). Could you respin increasing the version
> on each VMState change?
> 
>>
>> The s->mode check in the post_load() hook is also updated.
>>
>> Signed-off-by: Bin Meng <bin.meng@windriver.com>
>>
>> ---
>>
>> Changes in v2:
>> - new patch: bump up version ids of VMStateDescription
>>
>>  hw/sd/ssi-sd.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
>> index ee4fbc3dfe..0c507f3ec5 100644
>> --- a/hw/sd/ssi-sd.c
>> +++ b/hw/sd/ssi-sd.c
>> @@ -4,6 +4,11 @@
>>   * Copyright (c) 2007-2009 CodeSourcery.
>>   * Written by Paul Brook
>>   *
>> + * Copyright (c) 2021 Wind River Systems, Inc.
>> + * Improved by Bin Meng <bin.meng@windriver.com>
>> + *
>> + * Validated with U-Boot v2021.01 and Linux v5.10 mmc_spi driver
>> + *
>>   * This code is licensed under the GNU GPL v2.
>>   *
>>   * Contributions after 2012-01-13 are licensed under the terms of the
>> @@ -319,7 +324,7 @@ static int ssi_sd_post_load(void *opaque, int version_id)
>>  {
>>      ssi_sd_state *s = (ssi_sd_state *)opaque;
>>  
>> -    if (s->mode > SSI_SD_DATA_READ) {
>> +    if (s->mode > SSI_SD_SKIP_CRC16) {

Doesn't this belong to patch #16 "Support single block write"?

>>          return -EINVAL;
>>      }
>>      if (s->mode == SSI_SD_CMDARG &&
>> @@ -337,8 +342,8 @@ static int ssi_sd_post_load(void *opaque, int version_id)
>>  
>>  static const VMStateDescription vmstate_ssi_sd = {
>>      .name = "ssi_sd",
>> -    .version_id = 2,
>> -    .minimum_version_id = 2,
>> +    .version_id = 3,
>> +    .minimum_version_id = 3,
>>      .post_load = ssi_sd_post_load,
>>      .fields = (VMStateField []) {
>>          VMSTATE_UINT32(mode, ssi_sd_state),
>>
> 

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Bin Meng]

On Mon, Jan 25, 2021 at 12:59 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/23/21 11:40 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > With all these fixes and improvements, there is no way for the
> > VMStateDescription to keep backward compatibility. We will have
> > to bump up version ids.
>
> Unfortunately this breaks bisectability (think about downstream
> distributions cherry-picking patches individually).
>
> I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> (Cc'ed David in case). Could you respin increasing the version
> on each VMState change?
>

I definitely could be wrong, the reason I posted a single patch to
upreve the version is that, I was under an impression that in each big
release (like here 5.2.0 -> 6.0.0), the incompatibility version id
should be bumped up once.
It does not look correct to me that in a big release we bump up the
version id for 10 times.

Since this is a series to fix issues in the ssi-sd, I don't think it's
practical for downstream to just cherry-pick some commits while
leaving some other commits there.

Regards,
Bin

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Bin Meng]

On Mon, Jan 25, 2021 at 12:59 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 1/23/21 11:40 AM, Bin Meng wrote:
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > With all these fixes and improvements, there is no way for the
> > VMStateDescription to keep backward compatibility. We will have
> > to bump up version ids.
>
> Unfortunately this breaks bisectability (think about downstream
> distributions cherry-picking patches individually).
>
> I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> (Cc'ed David in case). Could you respin increasing the version
> on each VMState change?
>

I definitely could be wrong, the reason I posted a single patch to
upreve the version is that, I was under an impression that in each big
release (like here 5.2.0 -> 6.0.0), the incompatibility version id
should be bumped up once.
It does not look correct to me that in a big release we bump up the
version id for 10 times.

Since this is a series to fix issues in the ssi-sd, I don't think it's
practical for downstream to just cherry-pick some commits while
leaving some other commits there.

Regards,
Bin

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Dr. David Alan Gilbert]

* Bin Meng (bmeng.cn@gmail.com) wrote:
> On Mon, Jan 25, 2021 at 12:59 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > With all these fixes and improvements, there is no way for the
> > > VMStateDescription to keep backward compatibility. We will have
> > > to bump up version ids.
> >
> > Unfortunately this breaks bisectability (think about downstream
> > distributions cherry-picking patches individually).
> >
> > I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> > (Cc'ed David in case). Could you respin increasing the version
> > on each VMState change?
> >
> 
> I definitely could be wrong, the reason I posted a single patch to
> upreve the version is that, I was under an impression that in each big
> release (like here 5.2.0 -> 6.0.0), the incompatibility version id
> should be bumped up once.
> It does not look correct to me that in a big release we bump up the
> version id for 10 times.

I think I agree; I don't think we've ever done it incrementally like
that before.

It would only break bisectability if you were cross-version migrating
during the bisect which is rare.

> Since this is a series to fix issues in the ssi-sd, I don't think it's
> practical for downstream to just cherry-pick some commits while
> leaving some other commits there.

Never underestimate downstream :-)
However, please add a comment when you're doing incrimentals like this -
e.g. a TODO or something showing that it's unfinished and you need the
remaining patches so people don't do it accidentally.

Dave

> Regards,
> Bin
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Dr. David Alan Gilbert]

* Bin Meng (bmeng.cn@gmail.com) wrote:
> On Mon, Jan 25, 2021 at 12:59 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > From: Bin Meng <bin.meng@windriver.com>
> > >
> > > With all these fixes and improvements, there is no way for the
> > > VMStateDescription to keep backward compatibility. We will have
> > > to bump up version ids.
> >
> > Unfortunately this breaks bisectability (think about downstream
> > distributions cherry-picking patches individually).
> >
> > I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> > (Cc'ed David in case). Could you respin increasing the version
> > on each VMState change?
> >
> 
> I definitely could be wrong, the reason I posted a single patch to
> upreve the version is that, I was under an impression that in each big
> release (like here 5.2.0 -> 6.0.0), the incompatibility version id
> should be bumped up once.
> It does not look correct to me that in a big release we bump up the
> version id for 10 times.

I think I agree; I don't think we've ever done it incrementally like
that before.

It would only break bisectability if you were cross-version migrating
during the bisect which is rare.

> Since this is a series to fix issues in the ssi-sd, I don't think it's
> practical for downstream to just cherry-pick some commits while
> leaving some other commits there.

Never underestimate downstream :-)
However, please add a comment when you're doing incrimentals like this -
e.g. a TODO or something showing that it's unfinished and you need the
remaining patches so people don't do it accidentally.

Dave

> Regards,
> Bin
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Bin Meng]

On Mon, Jan 25, 2021 at 6:41 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Bin Meng (bmeng.cn@gmail.com) wrote:
> > On Mon, Jan 25, 2021 at 12:59 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >
> > > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > With all these fixes and improvements, there is no way for the
> > > > VMStateDescription to keep backward compatibility. We will have
> > > > to bump up version ids.
> > >
> > > Unfortunately this breaks bisectability (think about downstream
> > > distributions cherry-picking patches individually).
> > >
> > > I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> > > (Cc'ed David in case). Could you respin increasing the version
> > > on each VMState change?
> > >
> >
> > I definitely could be wrong, the reason I posted a single patch to
> > upreve the version is that, I was under an impression that in each big
> > release (like here 5.2.0 -> 6.0.0), the incompatibility version id
> > should be bumped up once.
> > It does not look correct to me that in a big release we bump up the
> > version id for 10 times.
>
> I think I agree; I don't think we've ever done it incrementally like
> that before.
>

Thanks David.

> It would only break bisectability if you were cross-version migrating
> during the bisect which is rare.
>
> > Since this is a series to fix issues in the ssi-sd, I don't think it's
> > practical for downstream to just cherry-pick some commits while
> > leaving some other commits there.
>
> Never underestimate downstream :-)
> However, please add a comment when you're doing incrimentals like this -
> e.g. a TODO or something showing that it's unfinished and you need the
> remaining patches so people don't do it accidentally.
>

Sure, next time :)

Philippe, I guess we will need to hold on your PR?
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=226135

Regards,
Bin

Re: [PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Bin Meng]

On Mon, Jan 25, 2021 at 6:41 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Bin Meng (bmeng.cn@gmail.com) wrote:
> > On Mon, Jan 25, 2021 at 12:59 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >
> > > On 1/23/21 11:40 AM, Bin Meng wrote:
> > > > From: Bin Meng <bin.meng@windriver.com>
> > > >
> > > > With all these fixes and improvements, there is no way for the
> > > > VMStateDescription to keep backward compatibility. We will have
> > > > to bump up version ids.
> > >
> > > Unfortunately this breaks bisectability (think about downstream
> > > distributions cherry-picking patches individually).
> > >
> > > I don't think there is a problem increasing 2 -> 3 -> 4 -> 5
> > > (Cc'ed David in case). Could you respin increasing the version
> > > on each VMState change?
> > >
> >
> > I definitely could be wrong, the reason I posted a single patch to
> > upreve the version is that, I was under an impression that in each big
> > release (like here 5.2.0 -> 6.0.0), the incompatibility version id
> > should be bumped up once.
> > It does not look correct to me that in a big release we bump up the
> > version id for 10 times.
>
> I think I agree; I don't think we've ever done it incrementally like
> that before.
>

Thanks David.

> It would only break bisectability if you were cross-version migrating
> during the bisect which is rare.
>
> > Since this is a series to fix issues in the ssi-sd, I don't think it's
> > practical for downstream to just cherry-pick some commits while
> > leaving some other commits there.
>
> Never underestimate downstream :-)
> However, please add a comment when you're doing incrimentals like this -
> e.g. a TODO or something showing that it's unfinished and you need the
> remaining patches so people don't do it accidentally.
>

Sure, next time :)

Philippe, I guess we will need to hold on your PR?
http://patchwork.ozlabs.org/project/qemu-devel/list/?series=226135

Regards,
Bin

[PATCH v2 19/25] hw/ssi: Add SiFive SPI controller support

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds the SiFive SPI controller model for the FU540 SoC.
The direct memory-mapped SPI flash mode is unsupported.

Signed-off-by: Bin Meng <bin.meng@windriver.com>

---

Changes in v2:
- Log guest error when trying to write reserved registers
- Log guest error when trying to access out-of-bounds registers
- log guest error when writing to reserved bits for chip select
  registers and watermark registers
- Log unimplemented warning when trying to write direct-map flash
  interface registers
- Add test tx fifo full logic in sifive_spi_read(), hence remove
  setting the tx fifo full flag in sifive_spi_write().
- Populate register with their default value

 include/hw/ssi/sifive_spi.h |  47 +++++
 hw/ssi/sifive_spi.c         | 367 ++++++++++++++++++++++++++++++++++++
 hw/ssi/Kconfig              |   4 +
 hw/ssi/meson.build          |   1 +
 4 files changed, 419 insertions(+)
 create mode 100644 include/hw/ssi/sifive_spi.h
 create mode 100644 hw/ssi/sifive_spi.c

diff --git a/include/hw/ssi/sifive_spi.h b/include/hw/ssi/sifive_spi.h
new file mode 100644
index 0000000000..47d0d6a47c
--- /dev/null
+++ b/include/hw/ssi/sifive_spi.h
@@ -0,0 +1,47 @@
+/*
+ * QEMU model of the SiFive SPI Controller
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng <bin.meng@windriver.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_SIFIVE_SPI_H
+#define HW_SIFIVE_SPI_H
+
+#define SIFIVE_SPI_REG_NUM  (0x78 / 4)
+
+#define TYPE_SIFIVE_SPI "sifive.spi"
+#define SIFIVE_SPI(obj) OBJECT_CHECK(SiFiveSPIState, (obj), TYPE_SIFIVE_SPI)
+
+typedef struct SiFiveSPIState {
+    SysBusDevice parent_obj;
+
+    MemoryRegion mmio;
+    qemu_irq irq;
+
+    uint32_t num_cs;
+    qemu_irq *cs_lines;
+
+    SSIBus *spi;
+
+    Fifo8 tx_fifo;
+    Fifo8 rx_fifo;
+
+    uint32_t regs[SIFIVE_SPI_REG_NUM];
+} SiFiveSPIState;
+
+#endif /* HW_SIFIVE_SPI_H */
diff --git a/hw/ssi/sifive_spi.c b/hw/ssi/sifive_spi.c
new file mode 100644
index 0000000000..61504336ad
--- /dev/null
+++ b/hw/ssi/sifive_spi.c
@@ -0,0 +1,367 @@
+/*
+ * QEMU model of the SiFive SPI Controller
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng <bin.meng@windriver.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "hw/ssi/ssi.h"
+#include "sysemu/sysemu.h"
+#include "qemu/fifo8.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/ssi/sifive_spi.h"
+
+#define R_SCKDIV        (0x00 / 4)
+#define R_SCKMODE       (0x04 / 4)
+#define R_CSID          (0x10 / 4)
+#define R_CSDEF         (0x14 / 4)
+#define R_CSMODE        (0x18 / 4)
+#define R_DELAY0        (0x28 / 4)
+#define R_DELAY1        (0x2C / 4)
+#define R_FMT           (0x40 / 4)
+#define R_TXDATA        (0x48 / 4)
+#define R_RXDATA        (0x4C / 4)
+#define R_TXMARK        (0x50 / 4)
+#define R_RXMARK        (0x54 / 4)
+#define R_FCTRL         (0x60 / 4)
+#define R_FFMT          (0x64 / 4)
+#define R_IE            (0x70 / 4)
+#define R_IP            (0x74 / 4)
+
+#define FMT_DIR         (1 << 3)
+
+#define TXDATA_FULL     (1 << 31)
+#define RXDATA_EMPTY    (1 << 31)
+
+#define IE_TXWM         (1 << 0)
+#define IE_RXWM         (1 << 1)
+
+#define IP_TXWM         (1 << 0)
+#define IP_RXWM         (1 << 1)
+
+#define FIFO_CAPACITY   8
+
+static void sifive_spi_txfifo_reset(SiFiveSPIState *s)
+{
+    fifo8_reset(&s->tx_fifo);
+
+    s->regs[R_TXDATA] &= ~TXDATA_FULL;
+    s->regs[R_IP] &= ~IP_TXWM;
+}
+
+static void sifive_spi_rxfifo_reset(SiFiveSPIState *s)
+{
+    fifo8_reset(&s->rx_fifo);
+
+    s->regs[R_RXDATA] |= RXDATA_EMPTY;
+    s->regs[R_IP] &= ~IP_RXWM;
+}
+
+static void sifive_spi_update_cs(SiFiveSPIState *s)
+{
+    int i;
+
+    for (i = 0; i < s->num_cs; i++) {
+        if (s->regs[R_CSDEF] & (1 << i)) {
+            qemu_set_irq(s->cs_lines[i], !(s->regs[R_CSMODE]));
+        }
+    }
+}
+
+static void sifive_spi_update_irq(SiFiveSPIState *s)
+{
+    int level;
+
+    if (fifo8_num_used(&s->tx_fifo) < s->regs[R_TXMARK]) {
+        s->regs[R_IP] |= IP_TXWM;
+    } else {
+        s->regs[R_IP] &= ~IP_TXWM;
+    }
+
+    if (fifo8_num_used(&s->rx_fifo) > s->regs[R_RXMARK]) {
+        s->regs[R_IP] |= IP_RXWM;
+    } else {
+        s->regs[R_IP] &= ~IP_RXWM;
+    }
+
+    level = s->regs[R_IP] & s->regs[R_IE] ? 1 : 0;
+    qemu_set_irq(s->irq, level);
+}
+
+static void sifive_spi_reset(DeviceState *d)
+{
+    SiFiveSPIState *s = SIFIVE_SPI(d);
+
+    memset(s->regs, 0, sizeof(s->regs));
+
+    /* The reset value is high for all implemented CS pins */
+    s->regs[R_CSDEF] = (1 << s->num_cs) - 1;
+
+    /* Populate register with their default value */
+    s->regs[R_SCKDIV] = 0x03;
+    s->regs[R_DELAY0] = 0x1001;
+    s->regs[R_DELAY1] = 0x01;
+
+    sifive_spi_txfifo_reset(s);
+    sifive_spi_rxfifo_reset(s);
+
+    sifive_spi_update_cs(s);
+    sifive_spi_update_irq(s);
+}
+
+static void sifive_spi_flush_txfifo(SiFiveSPIState *s)
+{
+    uint8_t tx;
+    uint8_t rx;
+
+    while (!fifo8_is_empty(&s->tx_fifo)) {
+        tx = fifo8_pop(&s->tx_fifo);
+        s->regs[R_TXDATA] &= ~TXDATA_FULL;
+
+        rx = ssi_transfer(s->spi, tx);
+
+        if (fifo8_is_full(&s->rx_fifo)) {
+            s->regs[R_IP] |= IP_RXWM;
+        } else {
+            if (!(s->regs[R_FMT] & FMT_DIR)) {
+                fifo8_push(&s->rx_fifo, rx);
+                s->regs[R_RXDATA] &= ~RXDATA_EMPTY;
+
+                if (fifo8_is_full(&s->rx_fifo)) {
+                    s->regs[R_IP] |= IP_RXWM;
+                }
+            }
+        }
+    }
+}
+
+static bool sifive_spi_is_bad_reg(hwaddr addr, bool allow_reserved)
+{
+    bool bad;
+
+    switch (addr) {
+    /* reserved offsets */
+    case 0x08:
+    case 0x0C:
+    case 0x1C:
+    case 0x20:
+    case 0x24:
+    case 0x30:
+    case 0x34:
+    case 0x38:
+    case 0x3C:
+    case 0x44:
+    case 0x58:
+    case 0x5C:
+    case 0x68:
+    case 0x6C:
+        bad = allow_reserved ? false : true;
+        break;
+    default:
+        bad = false;
+    }
+
+    if (addr >= (SIFIVE_SPI_REG_NUM << 2)) {
+        bad = true;
+    }
+
+    return bad;
+}
+
+static uint64_t sifive_spi_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    SiFiveSPIState *s = opaque;
+    uint32_t r;
+
+    if (sifive_spi_is_bad_reg(addr, true)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad read at address 0x%"
+                      HWADDR_PRIx "\n", __func__, addr);
+        return 0;
+    }
+
+    addr >>= 2;
+    switch (addr) {
+    case R_TXDATA:
+        if (fifo8_is_full(&s->tx_fifo)) {
+            return TXDATA_FULL;
+        }
+        r = 0;
+        break;
+
+    case R_RXDATA:
+        if (fifo8_is_empty(&s->rx_fifo)) {
+            return RXDATA_EMPTY;
+        }
+        r = fifo8_pop(&s->rx_fifo);
+        break;
+
+    default:
+        r = s->regs[addr];
+        break;
+    }
+
+    sifive_spi_update_irq(s);
+
+    return r;
+}
+
+static void sifive_spi_write(void *opaque, hwaddr addr,
+                             uint64_t val64, unsigned int size)
+{
+    SiFiveSPIState *s = opaque;
+    uint32_t value = val64;
+
+    if (sifive_spi_is_bad_reg(addr, false)) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write at addr=0x%"
+                      HWADDR_PRIx " value=0x%x\n", __func__, addr, value);
+        return;
+    }
+
+    addr >>= 2;
+    switch (addr) {
+    case R_CSID:
+        if (value >= s->num_cs) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid csid %d\n",
+                          __func__, value);
+        } else {
+            s->regs[R_CSID] = value;
+            sifive_spi_update_cs(s);
+        }
+        break;
+
+    case R_CSDEF:
+        if (value >= (1 << s->num_cs)) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid csdef %x\n",
+                          __func__, value);
+        } else {
+            s->regs[R_CSDEF] = value;
+        }
+        break;
+
+    case R_CSMODE:
+        if (value > 3) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid csmode %x\n",
+                          __func__, value);
+        } else {
+            s->regs[R_CSMODE] = value;
+            sifive_spi_update_cs(s);
+        }
+        break;
+
+    case R_TXDATA:
+        if (!fifo8_is_full(&s->tx_fifo)) {
+            fifo8_push(&s->tx_fifo, (uint8_t)value);
+            sifive_spi_flush_txfifo(s);
+        }
+        break;
+
+    case R_RXDATA:
+    case R_IP:
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: invalid write to read-only reigster 0x%"
+                      HWADDR_PRIx " with 0x%x\n", __func__, addr << 2, value);
+        break;
+
+    case R_TXMARK:
+    case R_RXMARK:
+        if (value >= FIFO_CAPACITY) {
+            qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid watermark %d\n",
+                          __func__, value);
+        } else {
+            s->regs[addr] = value;
+        }
+        break;
+
+    case R_FCTRL:
+    case R_FFMT:
+        qemu_log_mask(LOG_UNIMP,
+                      "%s: direct-map flash interface unimplemented\n",
+                      __func__);
+        break;
+
+    default:
+        s->regs[addr] = value;
+        break;
+    }
+
+    sifive_spi_update_irq(s);
+}
+
+static const MemoryRegionOps sifive_spi_ops = {
+    .read = sifive_spi_read,
+    .write = sifive_spi_write,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4
+    }
+};
+
+static void sifive_spi_realize(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    SiFiveSPIState *s = SIFIVE_SPI(dev);
+    int i;
+
+    s->spi = ssi_create_bus(dev, "spi");
+    sysbus_init_irq(sbd, &s->irq);
+
+    s->cs_lines = g_new0(qemu_irq, s->num_cs);
+    for (i = 0; i < s->num_cs; i++) {
+        sysbus_init_irq(sbd, &s->cs_lines[i]);
+    }
+
+    memory_region_init_io(&s->mmio, OBJECT(s), &sifive_spi_ops, s,
+                          TYPE_SIFIVE_SPI, 0x1000);
+    sysbus_init_mmio(sbd, &s->mmio);
+
+    fifo8_create(&s->tx_fifo, FIFO_CAPACITY);
+    fifo8_create(&s->rx_fifo, FIFO_CAPACITY);
+}
+
+static Property sifive_spi_properties[] = {
+    DEFINE_PROP_UINT32("num-cs", SiFiveSPIState, num_cs, 1),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void sifive_spi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    device_class_set_props(dc, sifive_spi_properties);
+    dc->reset = sifive_spi_reset;
+    dc->realize = sifive_spi_realize;
+}
+
+static const TypeInfo sifive_spi_info = {
+    .name           = TYPE_SIFIVE_SPI,
+    .parent         = TYPE_SYS_BUS_DEVICE,
+    .instance_size  = sizeof(SiFiveSPIState),
+    .class_init     = sifive_spi_class_init,
+};
+
+static void sifive_spi_register_types(void)
+{
+    type_register_static(&sifive_spi_info);
+}
+
+type_init(sifive_spi_register_types)
diff --git a/hw/ssi/Kconfig b/hw/ssi/Kconfig
index 9e54a0c8dd..7d90a02181 100644
--- a/hw/ssi/Kconfig
+++ b/hw/ssi/Kconfig
@@ -2,6 +2,10 @@ config PL022
     bool
     select SSI
 
+config SIFIVE_SPI
+    bool
+    select SSI
+
 config SSI
     bool
 
diff --git a/hw/ssi/meson.build b/hw/ssi/meson.build
index dee00c0da6..3d6bc82ab1 100644
--- a/hw/ssi/meson.build
+++ b/hw/ssi/meson.build
@@ -2,6 +2,7 @@ softmmu_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files('aspeed_smc.c'))
 softmmu_ss.add(when: 'CONFIG_MSF2', if_true: files('mss-spi.c'))
 softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_fiu.c'))
 softmmu_ss.add(when: 'CONFIG_PL022', if_true: files('pl022.c'))
+softmmu_ss.add(when: 'CONFIG_SIFIVE_SPI', if_true: files('sifive_spi.c'))
 softmmu_ss.add(when: 'CONFIG_SSI', if_true: files('ssi.c'))
 softmmu_ss.add(when: 'CONFIG_STM32F2XX_SPI', if_true: files('stm32f2xx_spi.c'))
 softmmu_ss.add(when: 'CONFIG_XILINX_SPI', if_true: files('xilinx_spi.c'))
-- 
2.25.1

[PATCH v2 20/25] hw/riscv: sifive_u: Add QSPI0 controller and connect a flash

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds the QSPI0 controller to the SoC, and connects an ISSI
25WP256 flash to it. The generation of corresponding device tree
source fragment is also added.

Since the direct memory-mapped mode is not supported by the SiFive
SPI model, the <reg> property does not populate the second group
which represents the memory mapped address of the SPI flash.

With this commit, upstream U-Boot for the SiFive HiFive Unleashed
board can boot on QEMU 'sifive_u' out of the box. This allows users
to develop and test the recommended RISC-V boot flow with a real
world use case: ZSBL (in QEMU) loads U-Boot SPL from SPI flash to
L2LIM, then U-Boot SPL loads the payload from SPI flash that is
combined with OpenSBI fw_dynamic firmware and U-Boot proper.

Specify machine property `msel` to 6 to allow booting from the SPI
flash. U-Boot spl is directly loaded via `-bios`, and subsequent
payload is stored in the SPI flash image. Example command line:

$ qemu-system-riscv64 -nographic -M sifive_u,msel=6 -smp 5 -m 8G \
    -bios u-boot-spl.bin -drive file=spi-nor.img,if=mtd

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Correct the "connects" typo in the commit message
- Mention in the commit message that <reg> property does not populate
  the second group which represents the memory mapped address of the
  SPI flash

 include/hw/riscv/sifive_u.h |  4 +++
 hw/riscv/sifive_u.c         | 52 +++++++++++++++++++++++++++++++++++++
 hw/riscv/Kconfig            |  2 ++
 3 files changed, 58 insertions(+)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index a9f7b4a084..8824b7c031 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -26,6 +26,7 @@
 #include "hw/gpio/sifive_gpio.h"
 #include "hw/misc/sifive_u_otp.h"
 #include "hw/misc/sifive_u_prci.h"
+#include "hw/ssi/sifive_spi.h"
 
 #define TYPE_RISCV_U_SOC "riscv.sifive.u.soc"
 #define RISCV_U_SOC(obj) \
@@ -45,6 +46,7 @@ typedef struct SiFiveUSoCState {
     SIFIVEGPIOState gpio;
     SiFiveUOTPState otp;
     SiFivePDMAState dma;
+    SiFiveSPIState spi0;
     CadenceGEMState gem;
 
     uint32_t serial;
@@ -82,6 +84,7 @@ enum {
     SIFIVE_U_DEV_UART0,
     SIFIVE_U_DEV_UART1,
     SIFIVE_U_DEV_GPIO,
+    SIFIVE_U_DEV_QSPI0,
     SIFIVE_U_DEV_OTP,
     SIFIVE_U_DEV_DMC,
     SIFIVE_U_DEV_FLASH0,
@@ -120,6 +123,7 @@ enum {
     SIFIVE_U_PDMA_IRQ5 = 28,
     SIFIVE_U_PDMA_IRQ6 = 29,
     SIFIVE_U_PDMA_IRQ7 = 30,
+    SIFIVE_U_QSPI0_IRQ = 51,
     SIFIVE_U_GEM_IRQ = 0x35
 };
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 59b61cea01..43a0e983d2 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -15,6 +15,7 @@
  * 5) OTP (One-Time Programmable) memory with stored serial number
  * 6) GEM (Gigabit Ethernet Controller) and management block
  * 7) DMA (Direct Memory Access Controller)
+ * 8) SPI0 connected to an SPI flash
  *
  * This board currently generates devicetree dynamically that indicates at least
  * two harts and up to five harts.
@@ -44,6 +45,7 @@
 #include "hw/char/serial.h"
 #include "hw/cpu/cluster.h"
 #include "hw/misc/unimp.h"
+#include "hw/ssi/ssi.h"
 #include "target/riscv/cpu.h"
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/sifive_u.h"
@@ -74,6 +76,7 @@ static const struct MemmapEntry {
     [SIFIVE_U_DEV_PRCI] =     { 0x10000000,     0x1000 },
     [SIFIVE_U_DEV_UART0] =    { 0x10010000,     0x1000 },
     [SIFIVE_U_DEV_UART1] =    { 0x10011000,     0x1000 },
+    [SIFIVE_U_DEV_QSPI0] =    { 0x10040000,     0x1000 },
     [SIFIVE_U_DEV_GPIO] =     { 0x10060000,     0x1000 },
     [SIFIVE_U_DEV_OTP] =      { 0x10070000,     0x1000 },
     [SIFIVE_U_DEV_GEM] =      { 0x10090000,     0x2000 },
@@ -342,6 +345,32 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
                             "sifive,fu540-c000-ccache");
     g_free(nodename);
 
+    nodename = g_strdup_printf("/soc/spi@%lx",
+        (long)memmap[SIFIVE_U_DEV_QSPI0].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
+    qemu_fdt_setprop_cells(fdt, nodename, "clocks",
+        prci_phandle, PRCI_CLK_TLCLK);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI0_IRQ);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+        0x0, memmap[SIFIVE_U_DEV_QSPI0].base,
+        0x0, memmap[SIFIVE_U_DEV_QSPI0].size);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,spi0");
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/spi@%lx/flash@0",
+        (long)memmap[SIFIVE_U_DEV_QSPI0].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_cell(fdt, nodename, "spi-rx-bus-width", 4);
+    qemu_fdt_setprop_cell(fdt, nodename, "spi-tx-bus-width", 4);
+    qemu_fdt_setprop(fdt, nodename, "m25p,fast-read", NULL, 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "spi-max-frequency", 50000000);
+    qemu_fdt_setprop_cell(fdt, nodename, "reg", 0);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "jedec,spi-nor");
+    g_free(nodename);
+
     phy_phandle = phandle++;
     nodename = g_strdup_printf("/soc/ethernet@%lx",
         (long)memmap[SIFIVE_U_DEV_GEM].base);
@@ -439,6 +468,9 @@ static void sifive_u_machine_init(MachineState *machine)
     int i;
     uint32_t fdt_load_addr;
     uint64_t kernel_entry;
+    DriveInfo *dinfo;
+    DeviceState *flash_dev;
+    qemu_irq flash_cs;
 
     /* Initialize SoC */
     object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RISCV_U_SOC);
@@ -571,6 +603,19 @@ static void sifive_u_machine_init(MachineState *machine)
     riscv_rom_copy_firmware_info(machine, memmap[SIFIVE_U_DEV_MROM].base,
                                  memmap[SIFIVE_U_DEV_MROM].size,
                                  sizeof(reset_vec), kernel_entry);
+
+    /* Connect an SPI flash to SPI0 */
+    flash_dev = qdev_new("is25wp256");
+    dinfo = drive_get_next(IF_MTD);
+    if (dinfo) {
+        qdev_prop_set_drive_err(flash_dev, "drive",
+                                blk_by_legacy_dinfo(dinfo),
+                                &error_fatal);
+    }
+    qdev_realize_and_unref(flash_dev, BUS(s->soc.spi0.spi), &error_fatal);
+
+    flash_cs = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi0), 1, flash_cs);
 }
 
 static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
@@ -680,6 +725,7 @@ static void sifive_u_soc_instance_init(Object *obj)
     object_initialize_child(obj, "gem", &s->gem, TYPE_CADENCE_GEM);
     object_initialize_child(obj, "gpio", &s->gpio, TYPE_SIFIVE_GPIO);
     object_initialize_child(obj, "pdma", &s->dma, TYPE_SIFIVE_PDMA);
+    object_initialize_child(obj, "spi0", &s->spi0, TYPE_SIFIVE_SPI);
 }
 
 static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
@@ -827,6 +873,12 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
 
     create_unimplemented_device("riscv.sifive.u.l2cc",
         memmap[SIFIVE_U_DEV_L2CC].base, memmap[SIFIVE_U_DEV_L2CC].size);
+
+    sysbus_realize(SYS_BUS_DEVICE(&s->spi0), errp);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi0), 0,
+                    memmap[SIFIVE_U_DEV_QSPI0].base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi0), 0,
+                       qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_QSPI0_IRQ));
 }
 
 static Property sifive_u_soc_props[] = {
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index facb0cbacc..6330297b4e 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -52,9 +52,11 @@ config SIFIVE_U
     select SIFIVE_GPIO
     select SIFIVE_PDMA
     select SIFIVE_PLIC
+    select SIFIVE_SPI
     select SIFIVE_UART
     select SIFIVE_U_OTP
     select SIFIVE_U_PRCI
+    select SSI_M25P80
     select UNIMP
 
 config SPIKE
-- 
2.25.1

[PATCH v2 21/25] hw/riscv: sifive_u: Add QSPI2 controller and connect an SD card

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds the QSPI2 controller to the SoC, and connects an SD
card to it. The generation of corresponding device tree source
fragment is also added.

Specify machine property `msel` to 11 to boot the same upstream
U-Boot SPL and payload image for the SiFive HiFive Unleashed board.
Note subsequent payload is stored in the SD card image.

$ qemu-system-riscv64 -nographic -M sifive_u,msel=11 -smp 5 -m 8G \
    -bios u-boot-spl.bin -drive file=sdcard.img,if=sd

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Correct the "connects" typo in the commit message

 include/hw/riscv/sifive_u.h |  3 +++
 hw/riscv/sifive_u.c         | 43 +++++++++++++++++++++++++++++++++++--
 hw/riscv/Kconfig            |  1 +
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 8824b7c031..de1464a2ce 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -47,6 +47,7 @@ typedef struct SiFiveUSoCState {
     SiFiveUOTPState otp;
     SiFivePDMAState dma;
     SiFiveSPIState spi0;
+    SiFiveSPIState spi2;
     CadenceGEMState gem;
 
     uint32_t serial;
@@ -85,6 +86,7 @@ enum {
     SIFIVE_U_DEV_UART1,
     SIFIVE_U_DEV_GPIO,
     SIFIVE_U_DEV_QSPI0,
+    SIFIVE_U_DEV_QSPI2,
     SIFIVE_U_DEV_OTP,
     SIFIVE_U_DEV_DMC,
     SIFIVE_U_DEV_FLASH0,
@@ -99,6 +101,7 @@ enum {
     SIFIVE_U_L2CC_IRQ2 = 3,
     SIFIVE_U_UART0_IRQ = 4,
     SIFIVE_U_UART1_IRQ = 5,
+    SIFIVE_U_QSPI2_IRQ = 6,
     SIFIVE_U_GPIO_IRQ0 = 7,
     SIFIVE_U_GPIO_IRQ1 = 8,
     SIFIVE_U_GPIO_IRQ2 = 9,
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 43a0e983d2..6c1158a848 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -16,6 +16,7 @@
  * 6) GEM (Gigabit Ethernet Controller) and management block
  * 7) DMA (Direct Memory Access Controller)
  * 8) SPI0 connected to an SPI flash
+ * 9) SPI2 connected to an SD card
  *
  * This board currently generates devicetree dynamically that indicates at least
  * two harts and up to five harts.
@@ -77,6 +78,7 @@ static const struct MemmapEntry {
     [SIFIVE_U_DEV_UART0] =    { 0x10010000,     0x1000 },
     [SIFIVE_U_DEV_UART1] =    { 0x10011000,     0x1000 },
     [SIFIVE_U_DEV_QSPI0] =    { 0x10040000,     0x1000 },
+    [SIFIVE_U_DEV_QSPI2] =    { 0x10050000,     0x1000 },
     [SIFIVE_U_DEV_GPIO] =     { 0x10060000,     0x1000 },
     [SIFIVE_U_DEV_OTP] =      { 0x10070000,     0x1000 },
     [SIFIVE_U_DEV_GEM] =      { 0x10090000,     0x2000 },
@@ -345,6 +347,31 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
                             "sifive,fu540-c000-ccache");
     g_free(nodename);
 
+    nodename = g_strdup_printf("/soc/spi@%lx",
+        (long)memmap[SIFIVE_U_DEV_QSPI2].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0);
+    qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
+    qemu_fdt_setprop_cells(fdt, nodename, "clocks",
+        prci_phandle, PRCI_CLK_TLCLK);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI2_IRQ);
+    qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+    qemu_fdt_setprop_cells(fdt, nodename, "reg",
+        0x0, memmap[SIFIVE_U_DEV_QSPI2].base,
+        0x0, memmap[SIFIVE_U_DEV_QSPI2].size);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,spi0");
+    g_free(nodename);
+
+    nodename = g_strdup_printf("/soc/spi@%lx/mmc@0",
+        (long)memmap[SIFIVE_U_DEV_QSPI2].base);
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop(fdt, nodename, "disable-wp", NULL, 0);
+    qemu_fdt_setprop_cells(fdt, nodename, "voltage-ranges", 3300, 3300);
+    qemu_fdt_setprop_cell(fdt, nodename, "spi-max-frequency", 20000000);
+    qemu_fdt_setprop_cell(fdt, nodename, "reg", 0);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "mmc-spi-slot");
+    g_free(nodename);
+
     nodename = g_strdup_printf("/soc/spi@%lx",
         (long)memmap[SIFIVE_U_DEV_QSPI0].base);
     qemu_fdt_add_subnode(fdt, nodename);
@@ -469,8 +496,8 @@ static void sifive_u_machine_init(MachineState *machine)
     uint32_t fdt_load_addr;
     uint64_t kernel_entry;
     DriveInfo *dinfo;
-    DeviceState *flash_dev;
-    qemu_irq flash_cs;
+    DeviceState *flash_dev, *sd_dev;
+    qemu_irq flash_cs, sd_cs;
 
     /* Initialize SoC */
     object_initialize_child(OBJECT(machine), "soc", &s->soc, TYPE_RISCV_U_SOC);
@@ -616,6 +643,12 @@ static void sifive_u_machine_init(MachineState *machine)
 
     flash_cs = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi0), 1, flash_cs);
+
+    /* Connect an SD card to SPI2 */
+    sd_dev = ssi_create_peripheral(s->soc.spi2.spi, "ssi-sd");
+
+    sd_cs = qdev_get_gpio_in_named(sd_dev, SSI_GPIO_CS, 0);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi2), 1, sd_cs);
 }
 
 static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp)
@@ -726,6 +759,7 @@ static void sifive_u_soc_instance_init(Object *obj)
     object_initialize_child(obj, "gpio", &s->gpio, TYPE_SIFIVE_GPIO);
     object_initialize_child(obj, "pdma", &s->dma, TYPE_SIFIVE_PDMA);
     object_initialize_child(obj, "spi0", &s->spi0, TYPE_SIFIVE_SPI);
+    object_initialize_child(obj, "spi2", &s->spi2, TYPE_SIFIVE_SPI);
 }
 
 static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
@@ -879,6 +913,11 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
                     memmap[SIFIVE_U_DEV_QSPI0].base);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi0), 0,
                        qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_QSPI0_IRQ));
+    sysbus_realize(SYS_BUS_DEVICE(&s->spi2), errp);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi2), 0,
+                    memmap[SIFIVE_U_DEV_QSPI2].base);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->spi2), 0,
+                       qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_QSPI2_IRQ));
 }
 
 static Property sifive_u_soc_props[] = {
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 6330297b4e..d139074b02 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -57,6 +57,7 @@ config SIFIVE_U
     select SIFIVE_U_OTP
     select SIFIVE_U_PRCI
     select SSI_M25P80
+    select SSI_SD
     select UNIMP
 
 config SPIKE
-- 
2.25.1

[PATCH v2 22/25] hw/riscv: sifive_u: Change SIFIVE_U_GEM_IRQ to decimal value

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

All other peripherals' IRQs are in the format of decimal value.
Change SIFIVE_U_GEM_IRQ to be consistent.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 include/hw/riscv/sifive_u.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index de1464a2ce..2656b39808 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -127,7 +127,7 @@ enum {
     SIFIVE_U_PDMA_IRQ6 = 29,
     SIFIVE_U_PDMA_IRQ7 = 30,
     SIFIVE_U_QSPI0_IRQ = 51,
-    SIFIVE_U_GEM_IRQ = 0x35
+    SIFIVE_U_GEM_IRQ = 53
 };
 
 enum {
-- 
2.25.1

[PATCH v2 23/25] docs/system: Sort targets in alphabetical order

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 docs/system/targets.rst | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index 560783644d..564cea9a9b 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -7,16 +7,21 @@ various targets are mentioned in the following sections.
 
 Contents:
 
+..
+   This table of contents should be kept sorted alphabetically
+   by the title text of each file, which isn't the same ordering
+   as an alphabetical sort by filename.
+
 .. toctree::
 
-   target-i386
+   target-arm
+   target-avr
+   target-m68k
+   target-mips
    target-ppc
+   target-rx
+   target-s390x
    target-sparc
    target-sparc64
-   target-mips
-   target-arm
-   target-m68k
+   target-i386
    target-xtensa
-   target-s390x
-   target-rx
-   target-avr
-- 
2.25.1

[PATCH v2 24/25] docs/system: Add RISC-V documentation

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

Add RISC-V system emulator documentation for generic information.
`Board-specific documentation` and `RISC-V CPU features` are only
a placeholder and will be added in the future.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 docs/system/target-riscv.rst | 62 ++++++++++++++++++++++++++++++++++++
 docs/system/targets.rst      |  1 +
 2 files changed, 63 insertions(+)
 create mode 100644 docs/system/target-riscv.rst

diff --git a/docs/system/target-riscv.rst b/docs/system/target-riscv.rst
new file mode 100644
index 0000000000..9f4b7586e5
--- /dev/null
+++ b/docs/system/target-riscv.rst
@@ -0,0 +1,62 @@
+.. _RISC-V-System-emulator:
+
+RISC-V System emulator
+======================
+
+QEMU can emulate both 32-bit and 64-bit RISC-V CPUs. Use the
+``qemu-system-riscv64`` executable to simulate a 64-bit RISC-V machine,
+``qemu-system-riscv32`` executable to simulate a 32-bit RISC-V machine.
+
+QEMU has generally good support for RISC-V guests. It has support for
+several different machines. The reason we support so many is that
+RISC-V hardware is much more widely varying than x86 hardware. RISC-V
+CPUs are generally built into "system-on-chip" (SoC) designs created by
+many different companies with different devices, and these SoCs are
+then built into machines which can vary still further even if they use
+the same SoC.
+
+For most boards the CPU type is fixed (matching what the hardware has),
+so typically you don't need to specify the CPU type by hand, except for
+special cases like the ``virt`` board.
+
+Choosing a board model
+----------------------
+
+For QEMU's RISC-V system emulation, you must specify which board
+model you want to use with the ``-M`` or ``--machine`` option;
+there is no default.
+
+Because RISC-V systems differ so much and in fundamental ways, typically
+operating system or firmware images intended to run on one machine
+will not run at all on any other. This is often surprising for new
+users who are used to the x86 world where every system looks like a
+standard PC. (Once the kernel has booted, most user space software
+cares much less about the detail of the hardware.)
+
+If you already have a system image or a kernel that works on hardware
+and you want to boot with QEMU, check whether QEMU lists that machine
+in its ``-machine help`` output. If it is listed, then you can probably
+use that board model. If it is not listed, then unfortunately your image
+will almost certainly not boot on QEMU. (You might be able to
+extract the file system and use that with a different kernel which
+boots on a system that QEMU does emulate.)
+
+If you don't care about reproducing the idiosyncrasies of a particular
+bit of hardware, such as small amount of RAM, no PCI or other hard
+disk, etc., and just want to run Linux, the best option is to use the
+``virt`` board. This is a platform which doesn't correspond to any
+real hardware and is designed for use in virtual machines. You'll
+need to compile Linux with a suitable configuration for running on
+the ``virt`` board. ``virt`` supports PCI, virtio, recent CPUs and
+large amounts of RAM. It also supports 64-bit CPUs.
+
+Board-specific documentation
+----------------------------
+
+Unfortunately many of the RISC-V boards QEMU supports are currently
+undocumented; you can get a complete list by running
+``qemu-system-riscv64 --machine help``, or
+``qemu-system-riscv32 --machine help``.
+
+RISC-V CPU features
+-------------------
diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index 564cea9a9b..75ed1087fd 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -19,6 +19,7 @@ Contents:
    target-m68k
    target-mips
    target-ppc
+   target-riscv
    target-rx
    target-s390x
    target-sparc
-- 
2.25.1

[PATCH v2 25/25] docs/system: riscv: Add documentation for sifive_u machine

[Bin Meng]

From: Bin Meng <bin.meng@windriver.com>

This adds detailed documentation for RISC-V `sifive_u` machine,
including the following information:

- Supported devices
- Hardware configuration information
- Boot options
- Machine-specific options
- Running Linux kernel
- Running VxWorks kernel
- Running U-Boot, and with an alternate configuration

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

---

Changes in v2:
- Correct several typos in sifive_u.rst
- Update doc to mention U-Boot v2021.01

 docs/system/riscv/sifive_u.rst | 336 +++++++++++++++++++++++++++++++++
 docs/system/target-riscv.rst   |  10 +
 2 files changed, 346 insertions(+)
 create mode 100644 docs/system/riscv/sifive_u.rst

diff --git a/docs/system/riscv/sifive_u.rst b/docs/system/riscv/sifive_u.rst
new file mode 100644
index 0000000000..3ad2426413
--- /dev/null
+++ b/docs/system/riscv/sifive_u.rst
@@ -0,0 +1,336 @@
+SiFive HiFive Unleashed (``sifive_u``)
+======================================
+
+SiFive HiFive Unleashed Development Board is the ultimate RISC‑V development
+board featuring the Freedom U540 multi-core RISC‑V processor.
+
+Supported devices
+-----------------
+
+The ``sifive_u`` machine supports the following devices:
+
+ * 1 E51 / E31 core
+ * Up to 4 U54 / U34 cores
+ * Core Level Interruptor (CLINT)
+ * Platform-Level Interrupt Controller (PLIC)
+ * Power, Reset, Clock, Interrupt (PRCI)
+ * L2 Loosely Integrated Memory (L2-LIM)
+ * DDR memory controller
+ * 2 UARTs
+ * 1 GEM Ethernet controller
+ * 1 GPIO controller
+ * 1 One-Time Programmable (OTP) memory with stored serial number
+ * 1 DMA controller
+ * 2 QSPI controllers
+ * 1 ISSI 25WP256 flash
+ * 1 SD card in SPI mode
+
+Please note the real world HiFive Unleashed board has a fixed configuration of
+1 E51 core and 4 U54 core combination and the RISC-V core boots in 64-bit mode.
+With QEMU, one can create a machine with 1 E51 core and up to 4 U54 cores. It
+is also possible to create a 32-bit variant with the same peripherals except
+that the RISC-V cores are replaced by the 32-bit ones (E31 and U34), to help
+testing of 32-bit guest software.
+
+Hardware configuration information
+----------------------------------
+
+The ``sifive_u`` machine automatically generates a device tree blob ("dtb")
+which it passes to the guest. This provides information about the addresses,
+interrupt lines and other configuration of the various devices in the system.
+Guest software should discover the devices that are present in the generated
+DTB instead of using a DTB for the real hardware, as some of the devices are
+not modeled by QEMU and trying to access these devices may cause unexpected
+behavior.
+
+Boot options
+------------
+
+The ``sifive_u`` machine can start using the standard -kernel functionality
+for loading a Linux kernel, a VxWorks kernel, a modified U-Boot bootloader
+(S-mode) or ELF executable with the default OpenSBI firmware image as the
+-bios. It also supports booting the unmodified U-Boot bootloader using the
+standard -bios functionality.
+
+Machine-specific options
+------------------------
+
+The following machine-specific options are supported:
+
+- serial=nnn
+
+  The board serial number. When not given, the default serial number 1 is used.
+
+  SiFive reserves the first 1 KiB of the 16 KiB OTP memory for internal use.
+  The current usage is only used to store the serial number of the board at
+  offset 0xfc. U-Boot reads the serial number from the OTP memory, and uses
+  it to generate a unique MAC address to be programmed to the on-chip GEM
+  Ethernet controller. When multiple QEMU ``sifive_u`` machines are created
+  and connected to the same subnet, they all have the same MAC address hence
+  it creates an unusable network. In such scenario, user should give different
+  values to serial= when creating different ``sifive_u`` machines.
+
+- start-in-flash
+
+  When given, QEMU's ROM codes jump to QSPI memory-mapped flash directly.
+  Otherwise QEMU will jump to DRAM or L2LIM depending on the msel= value.
+  When not given, it defaults to direct DRAM booting.
+
+- msel=[6|11]
+
+  Mode Select (MSEL[3:0]) pins value, used to control where to boot from.
+
+  The FU540 SoC supports booting from several sources, which are controlled
+  using the Mode Select pins on the chip. Typically, the boot process runs
+  through several stages before it begins execution of user-provided programs.
+  These stages typically include the following:
+
+  1. Zeroth Stage Boot Loader (ZSBL), which is contained in an on-chip mask
+     ROM and provided by QEMU. Note QEMU implemented ROM codes are not the
+     same as what is programmed in the hardware. The QEMU one is a simplified
+     version, but it provides the same functionality as the hardware.
+  2. First Stage Boot Loader (FSBL), which brings up PLLs and DDR memory.
+     This is U-Boot SPL.
+  3. Second Stage Boot Loader (SSBL), which further initializes additional
+     peripherals as needed. This is U-Boot proper combined with an OpenSBI
+     fw_dynamic firmware image.
+
+  msel=6 means FSBL and SSBL are both on the QSPI flash. msel=11 means FSBL
+  and SSBL are both on the SD card.
+
+Running Linux kernel
+--------------------
+
+Linux mainline v5.10 release is tested at the time of writing. To build a
+Linux mainline kernel that can be booted by the ``sifive_u`` machine in
+64-bit mode, simply configure the kernel using the defconfig configuration:
+
+.. code-block:: bash
+
+  $ export ARCH=riscv
+  $ export CROSS_COMPILE=riscv64-linux-
+  $ make defconfig
+  $ make
+
+To boot the newly built Linux kernel in QEMU with the ``sifive_u`` machine:
+
+.. code-block:: bash
+
+  $ qemu-system-riscv64 -M sifive_u -smp 5 -m 2G \
+      -display none -serial stdio \
+      -kernel arch/riscv/boot/Image \
+      -initrd /path/to/rootfs.ext4 \
+      -append "root=/dev/ram"
+
+To build a Linux mainline kernel that can be booted by the ``sifive_u`` machine
+in 32-bit mode, use the rv32_defconfig configuration. A patch is required to
+fix the 32-bit boot issue for Linux kernel v5.10.
+
+.. code-block:: bash
+
+  $ export ARCH=riscv
+  $ export CROSS_COMPILE=riscv64-linux-
+  $ curl https://patchwork.kernel.org/project/linux-riscv/patch/20201219001356.2887782-1-atish.patra@wdc.com/mbox/ > riscv.patch
+  $ git am riscv.patch
+  $ make rv32_defconfig
+  $ make
+
+Replace ``qemu-system-riscv64`` with ``qemu-system-riscv32`` in the command
+line above to boot the 32-bit Linux kernel. A rootfs image containing 32-bit
+applications shall be used in order for kernel to boot to user space.
+
+Running VxWorks kernel
+----------------------
+
+VxWorks 7 SR0650 release is tested at the time of writing. To build a 64-bit
+VxWorks mainline kernel that can be booted by the ``sifive_u`` machine, simply
+create a VxWorks source build project based on the sifive_generic BSP, and a
+VxWorks image project to generate the bootable VxWorks image, by following the
+BSP documentation instructions.
+
+A pre-built 64-bit VxWorks 7 image for HiFive Unleashed board is available as
+part of the VxWorks SDK for testing as well. Instructions to download the SDK:
+
+.. code-block:: bash
+
+  $ wget https://labs.windriver.com/downloads/wrsdk-vxworks7-sifive-hifive-1.01.tar.bz2
+  $ tar xvf wrsdk-vxworks7-sifive-hifive-1.01.tar.bz2
+  $ ls bsps/sifive_generic_1_0_0_0/uboot/uVxWorks
+
+To boot the VxWorks kernel in QEMU with the ``sifive_u`` machine, use:
+
+.. code-block:: bash
+
+  $ qemu-system-riscv64 -M sifive_u -smp 5 -m 2G \
+      -display none -serial stdio \
+      -nic tap,ifname=tap0,script=no,downscript=no \
+      -kernel /path/to/vxWorks \
+      -append "gem(0,0)host:vxWorks h=192.168.200.1 e=192.168.200.2:ffffff00 u=target pw=vxTarget f=0x01"
+
+It is also possible to test 32-bit VxWorks on the ``sifive_u`` machine. Create
+a 32-bit project to build the 32-bit VxWorks image, and use exact the same
+command line options with ``qemu-system-riscv32``.
+
+Running U-Boot
+--------------
+
+U-Boot mainline v2021.01 release is tested at the time of writing. To build a
+U-Boot mainline bootloader that can be booted by the ``sifive_u`` machine, use
+the sifive_fu540_defconfig with similar commands as described above for Linux:
+
+.. code-block:: bash
+
+  $ export CROSS_COMPILE=riscv64-linux-
+  $ export OPENSBI=/path/to/opensbi-riscv64-generic-fw_dynamic.bin
+  $ make sifive_fu540_defconfig
+
+You will get spl/u-boot-spl.bin and u-boot.itb file in the build tree.
+
+To start U-Boot using the ``sifive_u`` machine, prepare an SPI flash image, or
+SD card image that is properly partitioned and populated with correct contents.
+genimage_ can be used to generate these images.
+
+A sample configuration file for a 128 MiB SD card image is:
+
+.. code-block:: bash
+
+  $ cat genimage_sdcard.cfg
+  image sdcard.img {
+          size = 128M
+
+          hdimage {
+                  gpt = true
+          }
+
+          partition u-boot-spl {
+                  image = "u-boot-spl.bin"
+                  offset = 17K
+                  partition-type-uuid = 5B193300-FC78-40CD-8002-E86C45580B47
+          }
+
+          partition u-boot {
+                  image = "u-boot.itb"
+                  offset = 1041K
+                  partition-type-uuid = 2E54B353-1271-4842-806F-E436D6AF6985
+          }
+  }
+
+SPI flash image has slightly different partition offsets, and the size has to
+be 32 MiB to match the ISSI 25WP256 flash on the real board:
+
+.. code-block:: bash
+
+  $ cat genimage_spi-nor.cfg
+  image spi-nor.img {
+          size = 32M
+
+          hdimage {
+                  gpt = true
+          }
+
+          partition u-boot-spl {
+                  image = "u-boot-spl.bin"
+                  offset = 20K
+                  partition-type-uuid = 5B193300-FC78-40CD-8002-E86C45580B47
+          }
+
+          partition u-boot {
+                  image = "u-boot.itb"
+                  offset = 1044K
+                  partition-type-uuid = 2E54B353-1271-4842-806F-E436D6AF6985
+          }
+  }
+
+Assume U-Boot binaries are put in the same directory as the config file,
+we can generate the image by:
+
+.. code-block:: bash
+
+  $ genimage --config genimage_<boot_src>.cfg --inputpath .
+
+Boot U-Boot from SD card, by specifying msel=11 and pass the SD card image
+to QEMU ``sifive_u`` machine:
+
+.. code-block:: bash
+
+  $ qemu-system-riscv64 -M sifive_u,msel=11 -smp 5 -m 8G \
+      -display none -serial stdio \
+      -bios /path/to/u-boot-spl.bin \
+      -drive file=/path/to/sdcard.img,if=sd
+
+Changing msel= value to 6, allows booting U-Boot from the SPI flash:
+
+.. code-block:: bash
+
+  $ qemu-system-riscv64 -M sifive_u,msel=6 -smp 5 -m 8G \
+      -display none -serial stdio \
+      -bios /path/to/u-boot-spl.bin \
+      -drive file=/path/to/spi-nor.img,if=mtd
+
+Note when testing U-Boot, QEMU automatically generated device tree blob is
+not used because U-Boot itself embeds device tree blobs for U-Boot SPL and
+U-Boot proper. Hence the number of cores and size of memory have to match
+the real hardware, ie: 5 cores (-smp 5) and 8 GiB memory (-m 8G).
+
+Above use case is to run upstream U-Boot for the SiFive HiFive Unleashed
+board on QEMU ``sifive_u`` machine out of the box. This allows users to
+develop and test the recommended RISC-V boot flow with a real world use
+case: ZSBL (in QEMU) loads U-Boot SPL from SD card or SPI flash to L2LIM,
+then U-Boot SPL loads the combined payload image of OpenSBI fw_dynamic
+firmware and U-Boot proper. However sometimes we want to have a quick test
+of booting U-Boot on QEMU without the needs of preparing the SPI flash or
+SD card images, an alternate way can be used, which is to create a U-Boot
+S-mode image by modifying the configuration of U-Boot:
+
+.. code-block:: bash
+
+  $ make menuconfig
+
+then manually select the following configuration in U-Boot:
+
+  Device Tree Control > Provider of DTB for DT Control > Prior Stage bootloader DTB
+
+This lets U-Boot to use the QEMU generated device tree blob. During the build,
+a build error will be seen below:
+
+.. code-block:: none
+
+  MKIMAGE u-boot.img
+  ./tools/mkimage: Can't open arch/riscv/dts/hifive-unleashed-a00.dtb: No such file or directory
+  ./tools/mkimage: failed to build FIT
+  make: *** [Makefile:1440: u-boot.img] Error 1
+
+The above errors can be safely ignored as we don't run U-Boot SPL under QEMU
+in this alternate configuration.
+
+Boot the 64-bit U-Boot S-mode image directly:
+
+.. code-block:: bash
+
+  $ qemu-system-riscv64 -M sifive_u -smp 5 -m 2G \
+      -display none -serial stdio \
+      -kernel /path/to/u-boot.bin
+
+It's possible to create a 32-bit U-Boot S-mode image as well.
+
+.. code-block:: bash
+
+  $ export CROSS_COMPILE=riscv64-linux-
+  $ make sifive_fu540_defconfig
+  $ make menuconfig
+
+then manually update the following configuration in U-Boot:
+
+  Device Tree Control > Provider of DTB for DT Control > Prior Stage bootloader DTB
+  RISC-V architecture > Base ISA > RV32I
+  Boot images > Text Base > 0x80400000
+
+Use the same command line options to boot the 32-bit U-Boot S-mode image:
+
+.. code-block:: bash
+
+  $ qemu-system-riscv32 -M sifive_u -smp 5 -m 2G \
+      -display none -serial stdio \
+      -kernel /path/to/u-boot.bin
+
+.. _genimage: https://github.com/pengutronix/genimage
diff --git a/docs/system/target-riscv.rst b/docs/system/target-riscv.rst
index 9f4b7586e5..94d99c4c82 100644
--- a/docs/system/target-riscv.rst
+++ b/docs/system/target-riscv.rst
@@ -58,5 +58,15 @@ undocumented; you can get a complete list by running
 ``qemu-system-riscv64 --machine help``, or
 ``qemu-system-riscv32 --machine help``.
 
+..
+   This table of contents should be kept sorted alphabetically
+   by the title text of each file, which isn't the same ordering
+   as an alphabetical sort by filename.
+
+.. toctree::
+   :maxdepth: 1
+
+   riscv/sifive_u
+
 RISC-V CPU features
 -------------------
-- 
2.25.1

Re: [PATCH v2 00/25] hw/riscv: sifive_u: Add missing SPI support

[Philippe Mathieu-Daudé]

Hi Bin,

On 1/23/21 11:39 AM, Bin Meng wrote:
> From: Bin Meng <bin.meng@windriver.com>
> 
> This adds the missing SPI support to the `sifive_u` machine in the QEMU
> mainline. With this series, upstream U-Boot for the SiFive HiFive Unleashed
> board can boot on QEMU `sifive_u` out of the box. This allows users to
> develop and test the recommended RISC-V boot flow with a real world use
> case: ZSBL (in QEMU) loads U-Boot SPL from SD card or SPI flash to L2LIM,
> then U-Boot SPL loads the payload from SD card or SPI flash that is a
> combination of OpenSBI fw_dynamic firmware and U-Boot proper.
> 
> The m25p80 model is updated to support ISSI flash series. A bunch of
> ssi-sd issues are fixed, and writing to SD card in SPI mode is supported.
> 
> reST documentation for RISC-V is added. Currently only `sifive_u`
> machine is documented, but more to come.
> 
> Changes in v2:
> - Mention QPI (Quad Peripheral Interface) mode is not supported
> - Add a debug printf in the state handling codes
> - Fix several typos in the commit message
> - new patch: add a state representing Nac
> - Make this fix a separate patch from the CMD18 support
> - Fix 2 typos in the commit message
> - Add a comment block to explain the CMD12 timing
> - Catch CMD12 in all of the data read states per the timing requirement
> - Move multiple write token definitions out of this patch
> - Correct the "coding" typo in the commit message
> - Correct the "token" typo in the commit message
> - Add 'write_bytes' in vmstate_ssi_sd
> - Correct the "token" typo in the commit message
> - Introduce multiple write token definitions in this patch
> - new patch: bump up version ids of VMStateDescription
> - Log guest error when trying to write reserved registers
> - Log guest error when trying to access out-of-bounds registers
> - log guest error when writing to reserved bits for chip select
>   registers and watermark registers
> - Log unimplemented warning when trying to write direct-map flash
>   interface registers
> - Add test tx fifo full logic in sifive_spi_read(), hence remove
>   setting the tx fifo full flag in sifive_spi_write().
> - Populate register with their default value
> - Correct the "connects" typo in the commit message
> - Mention in the commit message that <reg> property does not populate
>   the second group which represents the memory mapped address of the
>   SPI flash
> - Correct the "connects" typo in the commit message
> - Correct several typos in sifive_u.rst
> - Update doc to mention U-Boot v2021.01
> 
> Bin Meng (25):
>   hw/block: m25p80: Add ISSI SPI flash support
>   hw/block: m25p80: Add various ISSI flash information
>   hw/sd: ssi-sd: Fix incorrect card response sequence
>   hw/sd: sd: Support CMD59 for SPI mode
>   hw/sd: sd: Drop sd_crc16()
>   util: Add CRC16 (CCITT) calculation routines
>   hw/sd: ssi-sd: Suffix a data block with CRC16
>   hw/sd: ssi-sd: Add a state representing Nac
>   hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
>   hw/sd: ssi-sd: Support multiple block read
>   hw/sd: ssi-sd: Use macros for the dummy value and tokens in the
>     transfer
>   hw/sd: sd: Remove duplicated codes in single/multiple block read/write
>   hw/sd: sd: Allow single/multiple block write for SPI mode
>   hw/sd: sd.h: Cosmetic change of using spaces
>   hw/sd: Introduce receive_ready() callback
>   hw/sd: ssi-sd: Support single block write
>   hw/sd: ssi-sd: Support multiple block write
>   hw/sd: ssi-sd: Bump up version ids of VMStateDescription
>   hw/ssi: Add SiFive SPI controller support
>   hw/riscv: sifive_u: Add QSPI0 controller and connect a flash
>   hw/riscv: sifive_u: Add QSPI2 controller and connect an SD card
>   hw/riscv: sifive_u: Change SIFIVE_U_GEM_IRQ to decimal value
>   docs/system: Sort targets in alphabetical order
>   docs/system: Add RISC-V documentation
>   docs/system: riscv: Add documentation for sifive_u machine

I'm queuing patches 3-9,11,14 to sd-next, and will repost
your 10,12,13,15-18 based on it.

Regards,

Phil.