qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles
@ 2019-01-21 15:49 Cédric Le Goater
  2019-01-22 23:52 ` Alistair Francis
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Cédric Le Goater @ 2019-01-21 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-arm, Peter Maydell, Joel Stanley, Andrew Jeffery,
	Alistair Francis, Peter Crosthwaite, Francisco Iglesias,
	Cédric Le Goater

The m25p80 models dummy cycles using byte transfers. This works well
when the transfers are initiated by the QEMU model of a SPI controller
but when these are initiated by the OS, it breaks emulation.

Snoop the SPI transfer to catch commands requiring dummy cycles and
replace them with byte transfers compatible with the m25p80 model.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---

 I have had few hacks to work around this problem, Andrew also, but I
 think that the model for the Xilinx Zynq SPI controller has an
 elegant solution.

 Tested with on different Linux kernels (OpenBMC, 5.0.0-rcX) and
 different Linux drivers doing fast read SPI transfers in User
 mode. It would be good to give the patch a try on other proprietary
 Firmwares using the USER command mode to access the flash.

 include/hw/ssi/aspeed_smc.h |   3 +
 hw/ssi/aspeed_smc.c         | 115 +++++++++++++++++++++++++++++++++++-
 2 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index b8b2dfbec280..f3909440d53c 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -104,6 +104,9 @@ typedef struct AspeedSMCState {
     AddressSpace dma_as;
 
     AspeedSMCFlash *flashes;
+
+    uint8_t snoop_index;
+    uint8_t snoop_dummies;
 } AspeedSMCState;
 
 #endif /* ASPEED_SMC_H */
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 0dc22e44cfd4..69683aca61b7 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -166,6 +166,9 @@
 /* Flash opcodes. */
 #define SPI_OP_READ       0x03    /* Read data bytes (low frequency) */
 
+#define SNOOP_OFF         0xFF
+#define SNOOP_START       0x0
+
 /*
  * Default segments mapping addresses and size for each slave per
  * controller. These can be changed when board is initialized with the
@@ -587,6 +590,101 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
     return ret;
 }
 
+/*
+ * TODO (clg@kaod.org): stolen from xilinx_spips.c. Should move to a
+ * common include header.
+ */
+typedef enum {
+    READ = 0x3,         READ_4 = 0x13,
+    FAST_READ = 0xb,    FAST_READ_4 = 0x0c,
+    DOR = 0x3b,         DOR_4 = 0x3c,
+    QOR = 0x6b,         QOR_4 = 0x6c,
+    DIOR = 0xbb,        DIOR_4 = 0xbc,
+    QIOR = 0xeb,        QIOR_4 = 0xec,
+
+    PP = 0x2,           PP_4 = 0x12,
+    DPP = 0xa2,
+    QPP = 0x32,         QPP_4 = 0x34,
+} FlashCMD;
+
+static int aspeed_smc_num_dummies(uint8_t command)
+{
+    switch (command) { /* check for dummies */
+    case READ: /* no dummy bytes/cycles */
+    case PP:
+    case DPP:
+    case QPP:
+    case READ_4:
+    case PP_4:
+    case QPP_4:
+        return 0;
+    case FAST_READ:
+    case DOR:
+    case QOR:
+    case DOR_4:
+    case QOR_4:
+        return 1;
+    case DIOR:
+    case FAST_READ_4:
+    case DIOR_4:
+        return 2;
+    case QIOR:
+    case QIOR_4:
+        return 4;
+    default:
+        return -1;
+    }
+}
+
+static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl,  uint64_t data,
+                                unsigned size)
+{
+    AspeedSMCState *s = fl->controller;
+    uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3;
+
+    if (s->snoop_index == SNOOP_OFF) {
+        return false; /* Do nothing */
+
+    } else if (s->snoop_index == SNOOP_START) {
+        uint8_t cmd = data & 0xff;
+        int ndummies = aspeed_smc_num_dummies(cmd);
+
+        /*
+         * No dummy cycles are expected with the current command. Turn
+         * off snooping and let the transfer proceed normally.
+         */
+        if (ndummies <= 0) {
+            s->snoop_index = SNOOP_OFF;
+            return false;
+        }
+
+        s->snoop_dummies = ndummies * 8;
+
+    } else if (s->snoop_index >= addr_width + 1) {
+
+        /* The SPI transfer has reached the dummy cycles sequence */
+        for (; s->snoop_dummies; s->snoop_dummies--) {
+            ssi_transfer(s->spi, s->regs[R_DUMMY_DATA] & 0xff);
+        }
+
+        /* If no more dummy cycles are expected, turn off snooping */
+        if (!s->snoop_dummies) {
+            s->snoop_index = SNOOP_OFF;
+        } else {
+            s->snoop_index += size;
+        }
+
+        /*
+         * Dummy cycles have been faked already. Ignore the current
+         * SPI transfer
+         */
+        return true;
+    }
+
+    s->snoop_index += size;
+    return false;
+}
+
 static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
                                    unsigned size)
 {
@@ -602,6 +700,10 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
 
     switch (aspeed_smc_flash_mode(fl)) {
     case CTRL_USERMODE:
+        if (aspeed_smc_do_snoop(fl, data, size)) {
+            break;
+        }
+
         for (i = 0; i < size; i++) {
             ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
         }
@@ -634,7 +736,9 @@ static const MemoryRegionOps aspeed_smc_flash_ops = {
 
 static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl)
 {
-    const AspeedSMCState *s = fl->controller;
+    AspeedSMCState *s = fl->controller;
+
+    s->snoop_index = aspeed_smc_is_ce_stop_active(fl) ? SNOOP_OFF : SNOOP_START;
 
     qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
 }
@@ -670,6 +774,9 @@ static void aspeed_smc_reset(DeviceState *d)
     if (s->ctrl->segments == aspeed_segments_fmc) {
         s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0);
     }
+
+    s->snoop_index = SNOOP_OFF;
+    s->snoop_dummies = 0;
 }
 
 static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
@@ -1100,10 +1207,12 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
 
 static const VMStateDescription vmstate_aspeed_smc = {
     .name = "aspeed.smc",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
+        VMSTATE_UINT8(snoop_index, AspeedSMCState),
+        VMSTATE_UINT8(snoop_dummies, AspeedSMCState),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles
  2019-01-21 15:49 [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles Cédric Le Goater
@ 2019-01-22 23:52 ` Alistair Francis
  2019-01-23  8:33 ` Francisco Iglesias
  2019-01-24 13:24 ` Peter Maydell
  2 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2019-01-22 23:52 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: qemu-devel@nongnu.org Developers, Peter Maydell,
	Peter Crosthwaite, Andrew Jeffery, Francisco Iglesias,
	Alistair Francis, qemu-arm, Joel Stanley

On Mon, Jan 21, 2019 at 7:50 AM Cédric Le Goater <clg@kaod.org> wrote:
>
> The m25p80 models dummy cycles using byte transfers. This works well
> when the transfers are initiated by the QEMU model of a SPI controller
> but when these are initiated by the OS, it breaks emulation.
>
> Snoop the SPI transfer to catch commands requiring dummy cycles and
> replace them with byte transfers compatible with the m25p80 model.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
>  I have had few hacks to work around this problem, Andrew also, but I
>  think that the model for the Xilinx Zynq SPI controller has an
>  elegant solution.
>
>  Tested with on different Linux kernels (OpenBMC, 5.0.0-rcX) and
>  different Linux drivers doing fast read SPI transfers in User
>  mode. It would be good to give the patch a try on other proprietary
>  Firmwares using the USER command mode to access the flash.
>
>  include/hw/ssi/aspeed_smc.h |   3 +
>  hw/ssi/aspeed_smc.c         | 115 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 115 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index b8b2dfbec280..f3909440d53c 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -104,6 +104,9 @@ typedef struct AspeedSMCState {
>      AddressSpace dma_as;
>
>      AspeedSMCFlash *flashes;
> +
> +    uint8_t snoop_index;
> +    uint8_t snoop_dummies;
>  } AspeedSMCState;
>
>  #endif /* ASPEED_SMC_H */
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 0dc22e44cfd4..69683aca61b7 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -166,6 +166,9 @@
>  /* Flash opcodes. */
>  #define SPI_OP_READ       0x03    /* Read data bytes (low frequency) */
>
> +#define SNOOP_OFF         0xFF
> +#define SNOOP_START       0x0
> +
>  /*
>   * Default segments mapping addresses and size for each slave per
>   * controller. These can be changed when board is initialized with the
> @@ -587,6 +590,101 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
>      return ret;
>  }
>
> +/*
> + * TODO (clg@kaod.org): stolen from xilinx_spips.c. Should move to a
> + * common include header.
> + */
> +typedef enum {
> +    READ = 0x3,         READ_4 = 0x13,
> +    FAST_READ = 0xb,    FAST_READ_4 = 0x0c,
> +    DOR = 0x3b,         DOR_4 = 0x3c,
> +    QOR = 0x6b,         QOR_4 = 0x6c,
> +    DIOR = 0xbb,        DIOR_4 = 0xbc,
> +    QIOR = 0xeb,        QIOR_4 = 0xec,
> +
> +    PP = 0x2,           PP_4 = 0x12,
> +    DPP = 0xa2,
> +    QPP = 0x32,         QPP_4 = 0x34,
> +} FlashCMD;
> +
> +static int aspeed_smc_num_dummies(uint8_t command)
> +{
> +    switch (command) { /* check for dummies */
> +    case READ: /* no dummy bytes/cycles */
> +    case PP:
> +    case DPP:
> +    case QPP:
> +    case READ_4:
> +    case PP_4:
> +    case QPP_4:
> +        return 0;
> +    case FAST_READ:
> +    case DOR:
> +    case QOR:
> +    case DOR_4:
> +    case QOR_4:
> +        return 1;
> +    case DIOR:
> +    case FAST_READ_4:
> +    case DIOR_4:
> +        return 2;
> +    case QIOR:
> +    case QIOR_4:
> +        return 4;
> +    default:
> +        return -1;
> +    }
> +}
> +
> +static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl,  uint64_t data,
> +                                unsigned size)
> +{
> +    AspeedSMCState *s = fl->controller;
> +    uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3;
> +
> +    if (s->snoop_index == SNOOP_OFF) {
> +        return false; /* Do nothing */
> +
> +    } else if (s->snoop_index == SNOOP_START) {
> +        uint8_t cmd = data & 0xff;
> +        int ndummies = aspeed_smc_num_dummies(cmd);
> +
> +        /*
> +         * No dummy cycles are expected with the current command. Turn
> +         * off snooping and let the transfer proceed normally.
> +         */
> +        if (ndummies <= 0) {
> +            s->snoop_index = SNOOP_OFF;
> +            return false;
> +        }
> +
> +        s->snoop_dummies = ndummies * 8;
> +
> +    } else if (s->snoop_index >= addr_width + 1) {
> +
> +        /* The SPI transfer has reached the dummy cycles sequence */
> +        for (; s->snoop_dummies; s->snoop_dummies--) {
> +            ssi_transfer(s->spi, s->regs[R_DUMMY_DATA] & 0xff);
> +        }
> +
> +        /* If no more dummy cycles are expected, turn off snooping */
> +        if (!s->snoop_dummies) {
> +            s->snoop_index = SNOOP_OFF;
> +        } else {
> +            s->snoop_index += size;
> +        }
> +
> +        /*
> +         * Dummy cycles have been faked already. Ignore the current
> +         * SPI transfer
> +         */
> +        return true;
> +    }
> +
> +    s->snoop_index += size;
> +    return false;
> +}
> +
>  static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
>                                     unsigned size)
>  {
> @@ -602,6 +700,10 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
>
>      switch (aspeed_smc_flash_mode(fl)) {
>      case CTRL_USERMODE:
> +        if (aspeed_smc_do_snoop(fl, data, size)) {
> +            break;
> +        }
> +
>          for (i = 0; i < size; i++) {
>              ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
>          }
> @@ -634,7 +736,9 @@ static const MemoryRegionOps aspeed_smc_flash_ops = {
>
>  static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl)
>  {
> -    const AspeedSMCState *s = fl->controller;
> +    AspeedSMCState *s = fl->controller;
> +
> +    s->snoop_index = aspeed_smc_is_ce_stop_active(fl) ? SNOOP_OFF : SNOOP_START;
>
>      qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
>  }
> @@ -670,6 +774,9 @@ static void aspeed_smc_reset(DeviceState *d)
>      if (s->ctrl->segments == aspeed_segments_fmc) {
>          s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0);
>      }
> +
> +    s->snoop_index = SNOOP_OFF;
> +    s->snoop_dummies = 0;
>  }
>
>  static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
> @@ -1100,10 +1207,12 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>
>  static const VMStateDescription vmstate_aspeed_smc = {
>      .name = "aspeed.smc",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
> +        VMSTATE_UINT8(snoop_index, AspeedSMCState),
> +        VMSTATE_UINT8(snoop_dummies, AspeedSMCState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> --
> 2.20.1
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles
  2019-01-21 15:49 [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles Cédric Le Goater
  2019-01-22 23:52 ` Alistair Francis
@ 2019-01-23  8:33 ` Francisco Iglesias
  2019-01-24 13:24 ` Peter Maydell
  2 siblings, 0 replies; 5+ messages in thread
From: Francisco Iglesias @ 2019-01-23  8:33 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: qemu-devel, qemu-arm, Peter Maydell, Joel Stanley, Andrew Jeffery,
	Alistair Francis, Peter Crosthwaite

On [2019 Jan 21] Mon 16:49:28, Cédric Le Goater wrote:
> The m25p80 models dummy cycles using byte transfers. This works well
> when the transfers are initiated by the QEMU model of a SPI controller
> but when these are initiated by the OS, it breaks emulation.
> 
> Snoop the SPI transfer to catch commands requiring dummy cycles and
> replace them with byte transfers compatible with the m25p80 model.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
> 
>  I have had few hacks to work around this problem, Andrew also, but I
>  think that the model for the Xilinx Zynq SPI controller has an
>  elegant solution.
> 
>  Tested with on different Linux kernels (OpenBMC, 5.0.0-rcX) and
>  different Linux drivers doing fast read SPI transfers in User
>  mode. It would be good to give the patch a try on other proprietary
>  Firmwares using the USER command mode to access the flash.
> 
>  include/hw/ssi/aspeed_smc.h |   3 +
>  hw/ssi/aspeed_smc.c         | 115 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 115 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
> index b8b2dfbec280..f3909440d53c 100644
> --- a/include/hw/ssi/aspeed_smc.h
> +++ b/include/hw/ssi/aspeed_smc.h
> @@ -104,6 +104,9 @@ typedef struct AspeedSMCState {
>      AddressSpace dma_as;
>  
>      AspeedSMCFlash *flashes;
> +
> +    uint8_t snoop_index;
> +    uint8_t snoop_dummies;
>  } AspeedSMCState;
>  
>  #endif /* ASPEED_SMC_H */
> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
> index 0dc22e44cfd4..69683aca61b7 100644
> --- a/hw/ssi/aspeed_smc.c
> +++ b/hw/ssi/aspeed_smc.c
> @@ -166,6 +166,9 @@
>  /* Flash opcodes. */
>  #define SPI_OP_READ       0x03    /* Read data bytes (low frequency) */
>  
> +#define SNOOP_OFF         0xFF
> +#define SNOOP_START       0x0
> +
>  /*
>   * Default segments mapping addresses and size for each slave per
>   * controller. These can be changed when board is initialized with the
> @@ -587,6 +590,101 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
>      return ret;
>  }
>  
> +/*
> + * TODO (clg@kaod.org): stolen from xilinx_spips.c. Should move to a
> + * common include header.
> + */
> +typedef enum {
> +    READ = 0x3,         READ_4 = 0x13,
> +    FAST_READ = 0xb,    FAST_READ_4 = 0x0c,
> +    DOR = 0x3b,         DOR_4 = 0x3c,
> +    QOR = 0x6b,         QOR_4 = 0x6c,
> +    DIOR = 0xbb,        DIOR_4 = 0xbc,
> +    QIOR = 0xeb,        QIOR_4 = 0xec,
> +
> +    PP = 0x2,           PP_4 = 0x12,
> +    DPP = 0xa2,
> +    QPP = 0x32,         QPP_4 = 0x34,
> +} FlashCMD;
> +
> +static int aspeed_smc_num_dummies(uint8_t command)
> +{
> +    switch (command) { /* check for dummies */
> +    case READ: /* no dummy bytes/cycles */
> +    case PP:
> +    case DPP:
> +    case QPP:
> +    case READ_4:
> +    case PP_4:
> +    case QPP_4:
> +        return 0;
> +    case FAST_READ:
> +    case DOR:
> +    case QOR:
> +    case DOR_4:
> +    case QOR_4:
> +        return 1;
> +    case DIOR:
> +    case FAST_READ_4:
> +    case DIOR_4:
> +        return 2;
> +    case QIOR:
> +    case QIOR_4:
> +        return 4;
> +    default:
> +        return -1;
> +    }
> +}
> +
> +static bool aspeed_smc_do_snoop(AspeedSMCFlash *fl,  uint64_t data,
> +                                unsigned size)
> +{
> +    AspeedSMCState *s = fl->controller;
> +    uint8_t addr_width = aspeed_smc_flash_is_4byte(fl) ? 4 : 3;
> +
> +    if (s->snoop_index == SNOOP_OFF) {
> +        return false; /* Do nothing */
> +
> +    } else if (s->snoop_index == SNOOP_START) {
> +        uint8_t cmd = data & 0xff;
> +        int ndummies = aspeed_smc_num_dummies(cmd);
> +
> +        /*
> +         * No dummy cycles are expected with the current command. Turn
> +         * off snooping and let the transfer proceed normally.
> +         */
> +        if (ndummies <= 0) {
> +            s->snoop_index = SNOOP_OFF;
> +            return false;
> +        }
> +
> +        s->snoop_dummies = ndummies * 8;
> +
> +    } else if (s->snoop_index >= addr_width + 1) {
> +
> +        /* The SPI transfer has reached the dummy cycles sequence */
> +        for (; s->snoop_dummies; s->snoop_dummies--) {
> +            ssi_transfer(s->spi, s->regs[R_DUMMY_DATA] & 0xff);
> +        }
> +
> +        /* If no more dummy cycles are expected, turn off snooping */
> +        if (!s->snoop_dummies) {
> +            s->snoop_index = SNOOP_OFF;
> +        } else {
> +            s->snoop_index += size;
> +        }
> +
> +        /*
> +         * Dummy cycles have been faked already. Ignore the current
> +         * SPI transfer
> +         */
> +        return true;
> +    }
> +
> +    s->snoop_index += size;
> +    return false;
> +}
> +
>  static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
>                                     unsigned size)
>  {
> @@ -602,6 +700,10 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
>  
>      switch (aspeed_smc_flash_mode(fl)) {
>      case CTRL_USERMODE:
> +        if (aspeed_smc_do_snoop(fl, data, size)) {
> +            break;
> +        }
> +
>          for (i = 0; i < size; i++) {
>              ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
>          }
> @@ -634,7 +736,9 @@ static const MemoryRegionOps aspeed_smc_flash_ops = {
>  
>  static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl)
>  {
> -    const AspeedSMCState *s = fl->controller;
> +    AspeedSMCState *s = fl->controller;
> +
> +    s->snoop_index = aspeed_smc_is_ce_stop_active(fl) ? SNOOP_OFF : SNOOP_START;
>  
>      qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
>  }
> @@ -670,6 +774,9 @@ static void aspeed_smc_reset(DeviceState *d)
>      if (s->ctrl->segments == aspeed_segments_fmc) {
>          s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0);
>      }
> +
> +    s->snoop_index = SNOOP_OFF;
> +    s->snoop_dummies = 0;
>  }
>  
>  static uint64_t aspeed_smc_read(void *opaque, hwaddr addr, unsigned int size)
> @@ -1100,10 +1207,12 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
>  
>  static const VMStateDescription vmstate_aspeed_smc = {
>      .name = "aspeed.smc",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
> +        VMSTATE_UINT8(snoop_index, AspeedSMCState),
> +        VMSTATE_UINT8(snoop_dummies, AspeedSMCState),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> -- 
> 2.20.1
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles
  2019-01-21 15:49 [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles Cédric Le Goater
  2019-01-22 23:52 ` Alistair Francis
  2019-01-23  8:33 ` Francisco Iglesias
@ 2019-01-24 13:24 ` Peter Maydell
  2019-01-24 13:33   ` Cédric Le Goater
  2 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2019-01-24 13:24 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: QEMU Developers, qemu-arm, Joel Stanley, Andrew Jeffery,
	Alistair Francis, Peter Crosthwaite, Francisco Iglesias

On Mon, 21 Jan 2019 at 15:49, Cédric Le Goater <clg@kaod.org> wrote:
>
> The m25p80 models dummy cycles using byte transfers. This works well
> when the transfers are initiated by the QEMU model of a SPI controller
> but when these are initiated by the OS, it breaks emulation.
>
> Snoop the SPI transfer to catch commands requiring dummy cycles and
> replace them with byte transfers compatible with the m25p80 model.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

This fails to compile:

/home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c: In
function ‘aspeed_smc_do_snoop’:
/home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:646:42:
error: ‘R_DUMMY_DATA’ undeclared (first use in this function)
             ssi_transfer(s->spi, s->regs[R_DUMMY_DATA] & 0xff);
                                          ^
/home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:646:42:
note: each undeclared identifier is reported only once for each
function it appears in
/home/petmay01/linaro/qemu-from-laptop/qemu/rules.mak:69: recipe for
target 'hw/ssi/aspeed_smc.o' failed

Is it dependent on some other patch ?

thanks
-- PMM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles
  2019-01-24 13:24 ` Peter Maydell
@ 2019-01-24 13:33   ` Cédric Le Goater
  0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2019-01-24 13:33 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, qemu-arm, Joel Stanley, Andrew Jeffery,
	Alistair Francis, Peter Crosthwaite, Francisco Iglesias

On 1/24/19 2:24 PM, Peter Maydell wrote:
> On Mon, 21 Jan 2019 at 15:49, Cédric Le Goater <clg@kaod.org> wrote:
>>
>> The m25p80 models dummy cycles using byte transfers. This works well
>> when the transfers are initiated by the QEMU model of a SPI controller
>> but when these are initiated by the OS, it breaks emulation.
>>
>> Snoop the SPI transfer to catch commands requiring dummy cycles and
>> replace them with byte transfers compatible with the m25p80 model.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
> 
> This fails to compile:
> 
> /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c: In
> function ‘aspeed_smc_do_snoop’:
> /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:646:42:
> error: ‘R_DUMMY_DATA’ undeclared (first use in this function)
>              ssi_transfer(s->spi, s->regs[R_DUMMY_DATA] & 0xff);
>                                           ^
> /home/petmay01/linaro/qemu-from-laptop/qemu/hw/ssi/aspeed_smc.c:646:42:
> note: each undeclared identifier is reported only once for each
> function it appears in
> /home/petmay01/linaro/qemu-from-laptop/qemu/rules.mak:69: recipe for
> target 'hw/ssi/aspeed_smc.o' failed
> 
> Is it dependent on some other patch ?

oups. sorry :/ it depends on another patch adding this register. 

I will build a patchset for the whole. It shouldn't be controversial.
I haven't add time to rework the DMA support so I will leave that out.

Thanks,

C.
 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-01-24 13:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-21 15:49 [Qemu-devel] [PATCH] aspeed/smc: snoop SPI transfers to fake dummy cycles Cédric Le Goater
2019-01-22 23:52 ` Alistair Francis
2019-01-23  8:33 ` Francisco Iglesias
2019-01-24 13:24 ` Peter Maydell
2019-01-24 13:33   ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).