[Qemu-devel] [PATCH 0/4] aspeed: add MMIO exec support to the FMC controller

[Qemu-devel] [PATCH 0/4] aspeed: add MMIO exec support to the FMC controller

[Cédric Le Goater]

Hello,

When MMIO execution support is active, these changes let the Aspeed
SoC machine boot directly from CE0. As there is still some
issues, the feature is disabled by default and should be activated
with :

	-global driver=aspeed.smc,property=mmio-exec,value=true

Thanks,

C.

Cédric Le Goater (4):
  aspeed/smc: fix HW strapping
  aspeed/smc: rename aspeed_smc_flash_send_addr() to
    aspeed_smc_flash_setup()
  aspeed/smc: add a new memory region dedicated to MMIO execution
  hw/arm/aspeed: boot from the FMC CE0 flash module

 include/hw/ssi/aspeed_smc.h |   7 ++
 hw/arm/aspeed.c             |  41 +++++++----
 hw/ssi/aspeed_smc.c         | 161 +++++++++++++++++++++++++++++++++++++-------
 3 files changed, 172 insertions(+), 37 deletions(-)

-- 
2.13.6

[Qemu-devel] [PATCH 1/4] aspeed/smc: fix HW strapping

[Cédric Le Goater]

Only the flash type is strapped by HW. The 4BYTE mode is set by
firmware when the flash device is detected.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 5059396bc623..f33ec87fcb74 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -632,23 +632,17 @@ static void aspeed_smc_reset(DeviceState *d)
             aspeed_smc_segment_to_reg(&s->ctrl->segments[i]);
     }
 
-    /* HW strapping for AST2500 FMC controllers  */
+    /* HW strapping flash type for FMC controllers  */
     if (s->ctrl->segments == aspeed_segments_ast2500_fmc) {
         /* flash type is fixed to SPI for CE0 and CE1 */
         s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0);
         s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE1);
-
-        /* 4BYTE mode is autodetected for CE0. Let's force it to 1 for
-         * now */
-        s->regs[s->r_ce_ctrl] |= (1 << (CTRL_EXTENDED0));
     }
 
     /* HW strapping for AST2400 FMC controllers (SCU70). Let's use the
      * configuration of the palmetto-bmc machine */
     if (s->ctrl->segments == aspeed_segments_fmc) {
         s->regs[s->r_conf] |= (CONF_FLASH_TYPE_SPI << CONF_FLASH_TYPE0);
-
-        s->regs[s->r_ce_ctrl] |= (1 << (CTRL_EXTENDED0));
     }
 }
 
-- 
2.13.6

[Qemu-devel] [PATCH 2/4] aspeed/smc: rename aspeed_smc_flash_send_addr() to aspeed_smc_flash_setup()

[Cédric Le Goater]

Also handle the fake transfers for dummy bytes in this setup
routine. It will be useful when we activate MMIO execution.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ssi/aspeed_smc.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index f33ec87fcb74..5808181568c4 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -496,10 +496,11 @@ static int aspeed_smc_flash_dummies(const AspeedSMCFlash *fl)
     return ((dummy_high << 2) | dummy_low) * 8;
 }
 
-static void aspeed_smc_flash_send_addr(AspeedSMCFlash *fl, uint32_t addr)
+static void aspeed_smc_flash_setup(AspeedSMCFlash *fl, uint32_t addr)
 {
     const AspeedSMCState *s = fl->controller;
     uint8_t cmd = aspeed_smc_flash_cmd(fl);
+    int i;
 
     /* Flash access can not exceed CS segment */
     addr = aspeed_smc_check_segment_addr(fl, addr);
@@ -512,6 +513,18 @@ static void aspeed_smc_flash_send_addr(AspeedSMCFlash *fl, uint32_t addr)
     ssi_transfer(s->spi, (addr >> 16) & 0xff);
     ssi_transfer(s->spi, (addr >> 8) & 0xff);
     ssi_transfer(s->spi, (addr & 0xff));
+
+    /*
+     * Use fake transfers to model dummy bytes. The value should
+     * be configured to some non-zero value in fast read mode and
+     * zero in read mode. But, as the HW allows inconsistent
+     * settings, let's check for fast read mode.
+     */
+    if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) {
+        for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
+                ssi_transfer(fl->controller->spi, 0xFF);
+        }
+    }
 }
 
 static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
@@ -530,19 +543,7 @@ static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned size)
     case CTRL_READMODE:
     case CTRL_FREADMODE:
         aspeed_smc_flash_select(fl);
-        aspeed_smc_flash_send_addr(fl, addr);
-
-        /*
-         * Use fake transfers to model dummy bytes. The value should
-         * be configured to some non-zero value in fast read mode and
-         * zero in read mode. But, as the HW allows inconsistent
-         * settings, let's check for fast read mode.
-         */
-        if (aspeed_smc_flash_mode(fl) == CTRL_FREADMODE) {
-            for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
-                ssi_transfer(fl->controller->spi, 0xFF);
-            }
-        }
+        aspeed_smc_flash_setup(fl, addr);
 
         for (i = 0; i < size; i++) {
             ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
@@ -579,7 +580,7 @@ static void aspeed_smc_flash_write(void *opaque, hwaddr addr, uint64_t data,
         break;
     case CTRL_WRITEMODE:
         aspeed_smc_flash_select(fl);
-        aspeed_smc_flash_send_addr(fl, addr);
+        aspeed_smc_flash_setup(fl, addr);
 
         for (i = 0; i < size; i++) {
             ssi_transfer(s->spi, (data >> (8 * i)) & 0xff);
-- 
2.13.6

[Qemu-devel] [PATCH 3/4] aspeed/smc: add a new memory region dedicated to MMIO execution

[Cédric Le Goater]

The Aspeed SoC are generally booted from one of the flash modules
behind the FMC controller. The FMC CS0 flash module is mapped at a
specific address depending on the SoC revision and also at 0x0, the
default boot-up address.

To support this second mapping, we add a new 'ROM' like memory region
under the FMC flash module model and activate support for MMIO
execution with a 'request_ptr' handler. The latter fills up a cache of
flash content to be executed or read by the boot up process.

Also add a 'mmio_exec' bool to activate the feature which still has
some issues.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/ssi/aspeed_smc.h |   7 +++
 hw/ssi/aspeed_smc.c         | 122 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 128 insertions(+), 1 deletion(-)

diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
index 1f557313fa93..5e853afe725d 100644
--- a/include/hw/ssi/aspeed_smc.h
+++ b/include/hw/ssi/aspeed_smc.h
@@ -48,6 +48,8 @@ typedef struct AspeedSMCController {
     uint32_t nregs;
 } AspeedSMCController;
 
+#define ASPEED_SMC_CACHE_SIZE    1024  /* 1K is the minimum */
+
 typedef struct AspeedSMCFlash {
     struct AspeedSMCState *controller;
 
@@ -56,6 +58,10 @@ typedef struct AspeedSMCFlash {
 
     MemoryRegion mmio;
     DeviceState *flash;
+
+    MemoryRegion mmio_rom;
+    uint8_t cache[ASPEED_SMC_CACHE_SIZE];
+    hwaddr cache_addr;
 } AspeedSMCFlash;
 
 #define TYPE_ASPEED_SMC "aspeed.smc"
@@ -79,6 +85,7 @@ typedef struct AspeedSMCState {
 
     MemoryRegion mmio;
     MemoryRegion mmio_flash;
+    bool mmio_exec;
 
     qemu_irq irq;
     int irqline;
diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 5808181568c4..d599eebc7d21 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -604,6 +604,96 @@ static const MemoryRegionOps aspeed_smc_flash_ops = {
     },
 };
 
+static bool aspeed_smc_flash_rom_is_cached(AspeedSMCFlash *fl, hwaddr addr)
+{
+    return (addr >= fl->cache_addr &&
+            addr <= fl->cache_addr + ASPEED_SMC_CACHE_SIZE - 4);
+}
+
+static void aspeed_smc_flash_rom_load_cache(AspeedSMCFlash *fl, hwaddr addr)
+{
+    AspeedSMCState *s = fl->controller;
+    hwaddr cache_addr = addr & ~(ASPEED_SMC_CACHE_SIZE - 1);
+    int i;
+
+    if (fl->cache_addr != ~0ULL) {
+        memory_region_invalidate_mmio_ptr(&fl->mmio_rom, fl->cache_addr,
+                                          ASPEED_SMC_CACHE_SIZE);
+    }
+
+    aspeed_smc_flash_select(fl);
+    aspeed_smc_flash_setup(fl, cache_addr);
+
+    for (i = 0; i < ASPEED_SMC_CACHE_SIZE; i++) {
+        fl->cache[i] = ssi_transfer(s->spi, 0x0);
+    }
+
+    aspeed_smc_flash_unselect(fl);
+
+    fl->cache_addr = cache_addr;
+}
+
+static void *aspeed_smc_flash_rom_request_ptr(void *opaque, hwaddr addr,
+                                              unsigned *size, unsigned *offset)
+{
+    AspeedSMCFlash *fl = opaque;
+
+    if (!aspeed_smc_flash_rom_is_cached(fl, addr)) {
+        aspeed_smc_flash_rom_load_cache(fl, addr);
+    }
+
+    *size = ASPEED_SMC_CACHE_SIZE;
+    *offset = fl->cache_addr;
+    return fl->cache;
+}
+
+static uint64_t aspeed_smc_flash_rom_read(void *opaque, hwaddr addr,
+                                          unsigned size)
+{
+    AspeedSMCFlash *fl = opaque;
+    AspeedSMCState *s = fl->controller;
+    uint64_t ret = 0;
+    int i;
+
+    /*
+     * Transfer or use the cache if possible. Reloading the cache
+     * while loading from the flash can break the TCG execution flow.
+     */
+    if (!aspeed_smc_flash_rom_is_cached(fl, addr)) {
+        aspeed_smc_flash_select(fl);
+        aspeed_smc_flash_setup(fl, addr);
+
+        for (i = 0; i < size; i++) {
+            ret |= (uint64_t) ssi_transfer(s->spi, 0x0) << (8 * i);
+        }
+
+        aspeed_smc_flash_unselect(fl);
+    } else {
+        for (i = 0; i < size; i++) {
+            ret |= (uint64_t) fl->cache[addr - fl->cache_addr + i] << (8 * i);
+        }
+    }
+    return ret;
+}
+
+static void aspeed_smc_flash_rom_write(void *opaque, hwaddr addr, uint64_t data,
+                                       unsigned size)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: flash is not writable at 0x%"
+                  HWADDR_PRIx "\n", __func__, addr);
+}
+
+static const MemoryRegionOps aspeed_smc_flash_rom_ops = {
+    .read = aspeed_smc_flash_rom_read,
+    .write = aspeed_smc_flash_rom_write,
+    .request_ptr = aspeed_smc_flash_rom_request_ptr,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 4,
+    },
+};
+
 static void aspeed_smc_flash_update_cs(AspeedSMCFlash *fl)
 {
     const AspeedSMCState *s = fl->controller;
@@ -778,21 +868,51 @@ static void aspeed_smc_realize(DeviceState *dev, Error **errp)
                               fl, name, fl->size);
         memory_region_add_subregion(&s->mmio_flash, offset, &fl->mmio);
         offset += fl->size;
+
+        /*
+         * The system is generally booted from one of the flash
+         * modules behind the FMC controller. Initialize the 'ROM'
+         * region which supports MMIO execution and let the board
+         * decide how to use them.
+         */
+        if ((s->ctrl->segments == aspeed_segments_ast2500_fmc ||
+             s->ctrl->segments == aspeed_segments_fmc) && s->mmio_exec) {
+            snprintf(name, sizeof(name), "%s.%d-rom", s->ctrl->name, i);
+
+            memory_region_init_io(&fl->mmio_rom, OBJECT(s),
+                                  &aspeed_smc_flash_rom_ops,
+                                  fl, name, fl->size);
+            fl->cache_addr = ~0ULL;
+        }
     }
 }
 
+static const VMStateDescription vmstate_aspeed_smc_flash = {
+    .name = TYPE_ASPEED_SMC "/flash",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField []) {
+        VMSTATE_UINT8_ARRAY(cache, AspeedSMCFlash, ASPEED_SMC_CACHE_SIZE),
+        VMSTATE_UINT64(cache_addr, AspeedSMCFlash),
+        VMSTATE_END_OF_LIST()
+    },
+};
+
 static const VMStateDescription vmstate_aspeed_smc = {
-    .name = "aspeed.smc",
+    .name = TYPE_ASPEED_SMC,
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (VMStateField[]) {
         VMSTATE_UINT32_ARRAY(regs, AspeedSMCState, ASPEED_SMC_R_MAX),
+        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(flashes, AspeedSMCState, num_cs,
+                                     vmstate_aspeed_smc_flash, AspeedSMCFlash),
         VMSTATE_END_OF_LIST()
     }
 };
 
 static Property aspeed_smc_properties[] = {
     DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
+    DEFINE_PROP_BOOL("mmio-exec", AspeedSMCState, mmio_exec, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.13.6

[Qemu-devel] [PATCH 4/4] hw/arm/aspeed: boot from the FMC CE0 flash module

[Cédric Le Goater]

When MMIO execution is activated on the FMC controller, remove the
copy of the flash module contents in a ROM memory region and boot
directly from CE0.

Booting from an alternate module (FMC CE1) should be possible. There
is still some work to support the module swap when the watchdog resets
the system.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/aspeed.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index e28170b7e1d8..e87a5899845f 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -160,6 +160,32 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
     g_free(storage);
 }
 
+static void install_boot_rom(AspeedBoardState *bmc, DriveInfo *drive,
+                             hwaddr addr)
+{
+    AspeedSMCState *fmc = &bmc->soc.fmc;
+    AspeedSMCFlash *fl = &fmc->flashes[0];
+    bool mmio_exec = object_property_get_bool(OBJECT(fmc), "mmio-exec",
+                                              &error_abort);
+    if (mmio_exec) {
+        memory_region_add_subregion(get_system_memory(), addr,
+                                    &fl->mmio_rom);
+    } else {
+        MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
+
+        /*
+         * create a ROM region using the default mapping window size of
+         * the flash module. The window size is 64MB for the AST2400
+         * SoC and 128MB for the AST2500 SoC, which is twice as big as
+         * needed by the flash modules of the Aspeed machines.
+         */
+        memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+                               fl->size, &error_abort);
+        memory_region_add_subregion(get_system_memory(), addr, boot_rom);
+        write_boot_rom(drive, addr, fl->size, &error_abort);
+    }
+}
+
 static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                       Error **errp)
 {
@@ -232,20 +258,7 @@ static void aspeed_board_init(MachineState *machine,
 
     /* Install first FMC flash content as a boot rom. */
     if (drive0) {
-        AspeedSMCFlash *fl = &bmc->soc.fmc.flashes[0];
-        MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
-
-        /*
-         * create a ROM region using the default mapping window size of
-         * the flash module. The window size is 64MB for the AST2400
-         * SoC and 128MB for the AST2500 SoC, which is twice as big as
-         * needed by the flash modules of the Aspeed machines.
-         */
-        memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
-                               fl->size, &error_abort);
-        memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
-                                    boot_rom);
-        write_boot_rom(drive0, FIRMWARE_ADDR, fl->size, &error_abort);
+        install_boot_rom(bmc, drive0, FIRMWARE_ADDR);
     }
 
     aspeed_board_binfo.kernel_filename = machine->kernel_filename;
-- 
2.13.6

Re: [Qemu-devel] [PATCH 0/4] aspeed: add MMIO exec support to the FMC controller

[Peter Maydell]

On 30 May 2018 at 08:49, Cédric Le Goater <clg@kaod.org> wrote:
> Hello,
>
> When MMIO execution support is active, these changes let the Aspeed
> SoC machine boot directly from CE0. As there is still some
> issues, the feature is disabled by default and should be activated
> with :
>
>         -global driver=aspeed.smc,property=mmio-exec,value=true

I'd really rather not add another mmio-exec device until
we've sorted out making it actually work properly...

thanks
-- PMM

Re: [Qemu-devel] [PATCH 0/4] aspeed: add MMIO exec support to the FMC controller

[Cédric Le Goater]

On 05/30/2018 02:40 PM, Peter Maydell wrote:
> On 30 May 2018 at 08:49, Cédric Le Goater <clg@kaod.org> wrote:
>> Hello,
>>
>> When MMIO execution support is active, these changes let the Aspeed
>> SoC machine boot directly from CE0. As there is still some
>> issues, the feature is disabled by default and should be activated
>> with :
>>
>>         -global driver=aspeed.smc,property=mmio-exec,value=true
> 
> I'd really rather not add another mmio-exec device until
> we've sorted out making it actually work properly...

OK. I will keep it for later. The FW of the witherspoon machine is 
a very good mmio-exec torture test. 

Thanks,

C. 

Re: [Qemu-devel] [PATCH 0/4] aspeed: add MMIO exec support to the FMC controller

[Cédric Le Goater]

On 05/30/2018 02:40 PM, Peter Maydell wrote:
> On 30 May 2018 at 08:49, Cédric Le Goater <clg@kaod.org> wrote:
>> Hello,
>>
>> When MMIO execution support is active, these changes let the Aspeed
>> SoC machine boot directly from CE0. As there is still some
>> issues, the feature is disabled by default and should be activated
>> with :
>>
>>         -global driver=aspeed.smc,property=mmio-exec,value=true
> 
> I'd really rather not add another mmio-exec device until
> we've sorted out making it actually work properly...

Could we still take patch 1-2 which are good model cleanups
for the flash controller ? 

Thanks,

C.