[Qemu-devel] [PATCH 0/3] target-xtensa: xtfpga: support cores without MMU

[Qemu-devel] [PATCH 0/3] target-xtensa: xtfpga: support cores without MMU

[Max Filippov]

Hello,

this series adds noMMU memory map to Xtensa XTFPGA board allowing to run
uClinux on cores without MMU.

Max Filippov (3):
  pflash_cfi01: add pflash_cfi01_init
  target-xtensa: xtfpga: attach FLASH to system IO
  target-xtensa: xtfpga: support noMMU cores

 hw/block/pflash_cfi01.c  | 29 +++++++++++++------
 hw/xtensa/xtfpga.c       | 73 ++++++++++++++++++++++++++++++++++++------------
 include/hw/block/flash.h |  7 +++++
 3 files changed, 83 insertions(+), 26 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init

[Max Filippov]

pflash_cfi01_register always registers FLASH to the default
system_memory region, which may not always be the right thing.

Provide pflash_cfi01_init function that only creates FLASH device and
sets its properties, leaving MMIO region registration to its caller.

Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 hw/block/pflash_cfi01.c  | 29 +++++++++++++++++++++--------
 include/hw/block/flash.h |  7 +++++++
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 2ba6c77..dfa7085 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -909,13 +909,12 @@ static void pflash_cfi01_register_types(void)
 
 type_init(pflash_cfi01_register_types)
 
-pflash_t *pflash_cfi01_register(hwaddr base,
-                                DeviceState *qdev, const char *name,
-                                hwaddr size,
-                                BlockBackend *blk,
-                                uint32_t sector_len, int nb_blocs,
-                                int bank_width, uint16_t id0, uint16_t id1,
-                                uint16_t id2, uint16_t id3, int be)
+pflash_t *pflash_cfi01_init(DeviceState *qdev, const char *name,
+                            hwaddr size,
+                            BlockBackend *blk,
+                            uint32_t sector_len, int nb_blocs,
+                            int bank_width, uint16_t id0, uint16_t id1,
+                            uint16_t id2, uint16_t id3, int be)
 {
     DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
 
@@ -932,9 +931,23 @@ pflash_t *pflash_cfi01_register(hwaddr base,
     qdev_prop_set_uint16(dev, "id3", id3);
     qdev_prop_set_string(dev, "name", name);
     qdev_init_nofail(dev);
+    return CFI_PFLASH01(dev);
+}
+
+pflash_t *pflash_cfi01_register(hwaddr base,
+                                DeviceState *qdev, const char *name,
+                                hwaddr size,
+                                BlockBackend *blk,
+                                uint32_t sector_len, int nb_blocs,
+                                int bank_width, uint16_t id0, uint16_t id1,
+                                uint16_t id2, uint16_t id3, int be)
+{
+    pflash_t *dev = pflash_cfi01_init(qdev, name, size, blk,
+                                      sector_len, nb_blocs, bank_width,
+                                      id0, id1, id2, id3, be);
 
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-    return CFI_PFLASH01(dev);
+    return dev;
 }
 
 MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 50ccbbc..a57d6e9 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -16,6 +16,13 @@ pflash_t *pflash_cfi01_register(hwaddr base,
                                 uint16_t id0, uint16_t id1,
                                 uint16_t id2, uint16_t id3, int be);
 
+pflash_t *pflash_cfi01_init(DeviceState *qdev, const char *name,
+                            hwaddr size,
+                            BlockBackend *blk,
+                            uint32_t sector_len, int nb_blocs, int width,
+                            uint16_t id0, uint16_t id1,
+                            uint16_t id2, uint16_t id3, int be);
+
 /* pflash_cfi02.c */
 pflash_t *pflash_cfi02_register(hwaddr base,
                                 DeviceState *qdev, const char *name,
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/3] target-xtensa: xtfpga: attach FLASH to system IO

[Max Filippov]

XTFPGA FLASH is tied to XTFPGA system IO block. It's not very important
for systems with MMU where system IO block is visible at single
location, but it's important for noMMU systems, where system IO block is
accessible through two separate physical address ranges.

Map XTFPGA FLASH to system IO block and fix offsets used for mapping.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 hw/xtensa/xtfpga.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 72350f1..d4b9afb 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -247,16 +247,20 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
     if (dinfo) {
-        flash = pflash_cfi01_register(board->flash_base,
-                NULL, "lx60.io.flash", board->flash_size,
-                blk_by_legacy_dinfo(dinfo),
-                board->flash_sector_size,
-                board->flash_size / board->flash_sector_size,
-                4, 0x0000, 0x0000, 0x0000, 0x0000, be);
+        SysBusDevice *s;
+
+        flash = pflash_cfi01_init(NULL, "lx60.io.flash", board->flash_size,
+                                  blk_by_legacy_dinfo(dinfo),
+                                  board->flash_sector_size,
+                                  board->flash_size / board->flash_sector_size,
+                                  4, 0x0000, 0x0000, 0x0000, 0x0000, be);
         if (flash == NULL) {
             error_report("unable to mount pflash");
             exit(EXIT_FAILURE);
         }
+        s = SYS_BUS_DEVICE(flash);
+        memory_region_add_subregion(system_io, board->flash_base,
+                                    sysbus_mmio_get_region(s, 0));
     }
 
     /* Use presence of kernel file name as 'boot from SRAM' switch. */
@@ -386,7 +390,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
 static void xtensa_lx60_init(MachineState *machine)
 {
     static const LxBoardDesc lx60_board = {
-        .flash_base = 0xf8000000,
+        .flash_base = 0x08000000,
         .flash_size = 0x00400000,
         .flash_sector_size = 0x10000,
         .sram_size = 0x20000,
@@ -397,7 +401,7 @@ static void xtensa_lx60_init(MachineState *machine)
 static void xtensa_lx200_init(MachineState *machine)
 {
     static const LxBoardDesc lx200_board = {
-        .flash_base = 0xf8000000,
+        .flash_base = 0x08000000,
         .flash_size = 0x01000000,
         .flash_sector_size = 0x20000,
         .sram_size = 0x2000000,
@@ -408,7 +412,7 @@ static void xtensa_lx200_init(MachineState *machine)
 static void xtensa_ml605_init(MachineState *machine)
 {
     static const LxBoardDesc ml605_board = {
-        .flash_base = 0xf8000000,
+        .flash_base = 0x08000000,
         .flash_size = 0x01000000,
         .flash_sector_size = 0x20000,
         .sram_size = 0x2000000,
@@ -419,7 +423,7 @@ static void xtensa_ml605_init(MachineState *machine)
 static void xtensa_kc705_init(MachineState *machine)
 {
     static const LxBoardDesc kc705_board = {
-        .flash_base = 0xf0000000,
+        .flash_base = 0x00000000,
         .flash_size = 0x08000000,
         .flash_boot_base = 0x06000000,
         .flash_sector_size = 0x20000,
-- 
1.8.1.4

[Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

Cores with and without MMU have system RAM and ROM at different locations.
Also with noMMU cores system IO region is accessible through two physical
address ranges.

Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 hw/xtensa/xtfpga.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index d4b9afb..b53f40d 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -199,7 +199,29 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
     const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
     const char *dtb_filename = qemu_opt_get(machine_opts, "dtb");
     const char *initrd_filename = qemu_opt_get(machine_opts, "initrd");
+    const unsigned system_io_size = 224 * 1024 * 1024;
+    bool mmu;
     int n;
+    static const struct {
+        hwaddr ram;
+        hwaddr rom;
+        hwaddr io[2];
+    } base[2] = {
+        {
+            .ram = 0x60000000,
+            .rom = 0x50000000,
+            .io = {
+                0x70000000,
+                0x90000000,
+            },
+        }, {
+            .ram = 0,
+            .rom = 0xfe000000,
+            .io = {
+                0xf0000000,
+            },
+        }
+    };
 
     if (!cpu_model) {
         cpu_model = XTENSA_DEFAULT_CPU_MODEL;
@@ -222,16 +244,24 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
         cpu_reset(CPU(cpu));
     }
 
+    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
     ram = g_malloc(sizeof(*ram));
     memory_region_init_ram(ram, NULL, "lx60.dram", machine->ram_size,
                            &error_fatal);
     vmstate_register_ram_global(ram);
-    memory_region_add_subregion(system_memory, 0, ram);
+    memory_region_add_subregion(system_memory, base[mmu].ram, ram);
 
     system_io = g_malloc(sizeof(*system_io));
     memory_region_init_io(system_io, NULL, &lx60_io_ops, NULL, "lx60.io",
-                          224 * 1024 * 1024);
-    memory_region_add_subregion(system_memory, 0xf0000000, system_io);
+                          system_io_size);
+    memory_region_add_subregion(system_memory, base[mmu].io[0], system_io);
+    if (!mmu) {
+        MemoryRegion *io = g_malloc(sizeof(*io));
+
+        memory_region_init_alias(io, NULL, "lx60.io.cached",
+                                 system_io, 0, system_io_size);
+        memory_region_add_subregion(system_memory, base[mmu].io[1], io);
+    }
     lx60_fpga_init(system_io, 0x0d020000);
     if (nd_table[0].used) {
         lx60_net_init(system_io, 0x0d030000, 0x0d030400, 0x0d800000,
@@ -267,22 +297,25 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
     if (kernel_filename) {
         uint32_t entry_point = env->pc;
         size_t bp_size = 3 * get_tag_size(0); /* first/last and memory tags */
-        uint32_t tagptr = 0xfe000000 + board->sram_size;
+        uint32_t tagptr = base[mmu].rom + board->sram_size;
         uint32_t cur_tagptr;
         BpMemInfo memory_location = {
             .type = tswap32(MEMORY_TYPE_CONVENTIONAL),
-            .start = tswap32(0),
-            .end = tswap32(machine->ram_size),
+            .start = tswap32(base[mmu].ram),
+            .end = tswap32(base[mmu].ram + machine->ram_size),
         };
         uint32_t lowmem_end = machine->ram_size < 0x08000000 ?
             machine->ram_size : 0x08000000;
         uint32_t cur_lowmem = QEMU_ALIGN_UP(lowmem_end / 2, 4096);
 
+        lowmem_end += base[mmu].ram;
+        cur_lowmem += base[mmu].ram;
+
         rom = g_malloc(sizeof(*rom));
         memory_region_init_ram(rom, NULL, "lx60.sram", board->sram_size,
                                &error_fatal);
         vmstate_register_ram_global(rom);
-        memory_region_add_subregion(system_memory, 0xfe000000, rom);
+        memory_region_add_subregion(system_memory, base[mmu].rom, rom);
 
         if (kernel_cmdline) {
             bp_size += get_tag_size(strlen(kernel_cmdline) + 1);
@@ -381,7 +414,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
                     flash_mr, board->flash_boot_base,
                     board->flash_size - board->flash_boot_base < 0x02000000 ?
                     board->flash_size - board->flash_boot_base : 0x02000000);
-            memory_region_add_subregion(system_memory, 0xfe000000,
+            memory_region_add_subregion(system_memory, base[mmu].rom,
                     flash_io);
         }
     }
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> pflash_cfi01_register always registers FLASH to the default
> system_memory region, which may not always be the right thing.
>
> Provide pflash_cfi01_init function that only creates FLASH device and
> sets its properties, leaving MMIO region registration to its caller.
>

You should just use QOM to create your device inline without need for
a construction helper. See Vexpress's ve_pflash_cfi01_register for an
example.

Regards,
Peter

> Cc: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  hw/block/pflash_cfi01.c  | 29 +++++++++++++++++++++--------
>  include/hw/block/flash.h |  7 +++++++
>  2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 2ba6c77..dfa7085 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -909,13 +909,12 @@ static void pflash_cfi01_register_types(void)
>
>  type_init(pflash_cfi01_register_types)
>
> -pflash_t *pflash_cfi01_register(hwaddr base,
> -                                DeviceState *qdev, const char *name,
> -                                hwaddr size,
> -                                BlockBackend *blk,
> -                                uint32_t sector_len, int nb_blocs,
> -                                int bank_width, uint16_t id0, uint16_t id1,
> -                                uint16_t id2, uint16_t id3, int be)
> +pflash_t *pflash_cfi01_init(DeviceState *qdev, const char *name,
> +                            hwaddr size,
> +                            BlockBackend *blk,
> +                            uint32_t sector_len, int nb_blocs,
> +                            int bank_width, uint16_t id0, uint16_t id1,
> +                            uint16_t id2, uint16_t id3, int be)
>  {
>      DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
>
> @@ -932,9 +931,23 @@ pflash_t *pflash_cfi01_register(hwaddr base,
>      qdev_prop_set_uint16(dev, "id3", id3);
>      qdev_prop_set_string(dev, "name", name);
>      qdev_init_nofail(dev);
> +    return CFI_PFLASH01(dev);
> +}
> +
> +pflash_t *pflash_cfi01_register(hwaddr base,
> +                                DeviceState *qdev, const char *name,
> +                                hwaddr size,
> +                                BlockBackend *blk,
> +                                uint32_t sector_len, int nb_blocs,
> +                                int bank_width, uint16_t id0, uint16_t id1,
> +                                uint16_t id2, uint16_t id3, int be)
> +{
> +    pflash_t *dev = pflash_cfi01_init(qdev, name, size, blk,
> +                                      sector_len, nb_blocs, bank_width,
> +                                      id0, id1, id2, id3, be);
>
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
> -    return CFI_PFLASH01(dev);
> +    return dev;
>  }
>
>  MemoryRegion *pflash_cfi01_get_memory(pflash_t *fl)
> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
> index 50ccbbc..a57d6e9 100644
> --- a/include/hw/block/flash.h
> +++ b/include/hw/block/flash.h
> @@ -16,6 +16,13 @@ pflash_t *pflash_cfi01_register(hwaddr base,
>                                  uint16_t id0, uint16_t id1,
>                                  uint16_t id2, uint16_t id3, int be);
>
> +pflash_t *pflash_cfi01_init(DeviceState *qdev, const char *name,
> +                            hwaddr size,
> +                            BlockBackend *blk,
> +                            uint32_t sector_len, int nb_blocs, int width,
> +                            uint16_t id0, uint16_t id1,
> +                            uint16_t id2, uint16_t id3, int be);
> +
>  /* pflash_cfi02.c */
>  pflash_t *pflash_cfi02_register(hwaddr base,
>                                  DeviceState *qdev, const char *name,
> --
> 1.8.1.4
>
>

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Cores with and without MMU have system RAM and ROM at different locations.
> Also with noMMU cores system IO region is accessible through two physical
> address ranges.
>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  hw/xtensa/xtfpga.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 41 insertions(+), 8 deletions(-)
>
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index d4b9afb..b53f40d 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -199,7 +199,29 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>      const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
>      const char *dtb_filename = qemu_opt_get(machine_opts, "dtb");
>      const char *initrd_filename = qemu_opt_get(machine_opts, "initrd");
> +    const unsigned system_io_size = 224 * 1024 * 1024;
> +    bool mmu;

You are indexing into an array of configs so it's really an int (or
better, an enum).

>      int n;

Blank line.

> +    static const struct {
> +        hwaddr ram;
> +        hwaddr rom;
> +        hwaddr io[2];
> +    } base[2] = {
> +        {
> +            .ram = 0x60000000,
> +            .rom = 0x50000000,
> +            .io = {
> +                0x70000000,
> +                0x90000000,
> +            },
> +        }, {
> +            .ram = 0,
> +            .rom = 0xfe000000,
> +            .io = {
> +                0xf0000000,
> +            },
> +        }
> +    };
>
>      if (!cpu_model) {
>          cpu_model = XTENSA_DEFAULT_CPU_MODEL;
> @@ -222,16 +244,24 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>          cpu_reset(CPU(cpu));
>      }
>
> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);

This looks backwards, the board should be in charge of itself and the
CPU config, rather than spying on the CPU setup to rewire the board.

>      ram = g_malloc(sizeof(*ram));
>      memory_region_init_ram(ram, NULL, "lx60.dram", machine->ram_size,
>                             &error_fatal);
>      vmstate_register_ram_global(ram);
> -    memory_region_add_subregion(system_memory, 0, ram);
> +    memory_region_add_subregion(system_memory, base[mmu].ram, ram);
>
>      system_io = g_malloc(sizeof(*system_io));
>      memory_region_init_io(system_io, NULL, &lx60_io_ops, NULL, "lx60.io",
> -                          224 * 1024 * 1024);
> -    memory_region_add_subregion(system_memory, 0xf0000000, system_io);
> +                          system_io_size);
> +    memory_region_add_subregion(system_memory, base[mmu].io[0], system_io);
> +    if (!mmu) {

The boolean switch for whether the alias exists could go in the
struct. That makes it more robust to add yet more configs in the
future rather than iffery on the config index.

Regards,
Peter

> +        MemoryRegion *io = g_malloc(sizeof(*io));
> +
> +        memory_region_init_alias(io, NULL, "lx60.io.cached",
> +                                 system_io, 0, system_io_size);
> +        memory_region_add_subregion(system_memory, base[mmu].io[1], io);
> +    }
>      lx60_fpga_init(system_io, 0x0d020000);
>      if (nd_table[0].used) {
>          lx60_net_init(system_io, 0x0d030000, 0x0d030400, 0x0d800000,
> @@ -267,22 +297,25 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>      if (kernel_filename) {
>          uint32_t entry_point = env->pc;
>          size_t bp_size = 3 * get_tag_size(0); /* first/last and memory tags */
> -        uint32_t tagptr = 0xfe000000 + board->sram_size;
> +        uint32_t tagptr = base[mmu].rom + board->sram_size;
>          uint32_t cur_tagptr;
>          BpMemInfo memory_location = {
>              .type = tswap32(MEMORY_TYPE_CONVENTIONAL),
> -            .start = tswap32(0),
> -            .end = tswap32(machine->ram_size),
> +            .start = tswap32(base[mmu].ram),
> +            .end = tswap32(base[mmu].ram + machine->ram_size),
>          };
>          uint32_t lowmem_end = machine->ram_size < 0x08000000 ?
>              machine->ram_size : 0x08000000;
>          uint32_t cur_lowmem = QEMU_ALIGN_UP(lowmem_end / 2, 4096);
>
> +        lowmem_end += base[mmu].ram;
> +        cur_lowmem += base[mmu].ram;
> +
>          rom = g_malloc(sizeof(*rom));
>          memory_region_init_ram(rom, NULL, "lx60.sram", board->sram_size,
>                                 &error_fatal);
>          vmstate_register_ram_global(rom);
> -        memory_region_add_subregion(system_memory, 0xfe000000, rom);
> +        memory_region_add_subregion(system_memory, base[mmu].rom, rom);
>
>          if (kernel_cmdline) {
>              bp_size += get_tag_size(strlen(kernel_cmdline) + 1);
> @@ -381,7 +414,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>                      flash_mr, board->flash_boot_base,
>                      board->flash_size - board->flash_boot_base < 0x02000000 ?
>                      board->flash_size - board->flash_boot_base : 0x02000000);
> -            memory_region_add_subregion(system_memory, 0xfe000000,
> +            memory_region_add_subregion(system_memory, base[mmu].rom,
>                      flash_io);
>          }
>      }
> --
> 1.8.1.4
>
>

Re: [Qemu-devel] [PATCH 2/3] target-xtensa: xtfpga: attach FLASH to system IO

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> XTFPGA FLASH is tied to XTFPGA system IO block. It's not very important
> for systems with MMU where system IO block is visible at single
> location,

Are your relying on a matching change in the guest for MMU setup here?
Can you explain a little more how the MMU allows a physical address
remap of the underlying hardware as this looks like a non
backwards-compatible board design rev.

Regards,
Peter

but it's important for noMMU systems, where system IO block is
> accessible through two separate physical address ranges.
>
> Map XTFPGA FLASH to system IO block and fix offsets used for mapping.
>
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>  hw/xtensa/xtfpga.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
> index 72350f1..d4b9afb 100644
> --- a/hw/xtensa/xtfpga.c
> +++ b/hw/xtensa/xtfpga.c
> @@ -247,16 +247,20 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>
>      dinfo = drive_get(IF_PFLASH, 0, 0);
>      if (dinfo) {
> -        flash = pflash_cfi01_register(board->flash_base,
> -                NULL, "lx60.io.flash", board->flash_size,
> -                blk_by_legacy_dinfo(dinfo),
> -                board->flash_sector_size,
> -                board->flash_size / board->flash_sector_size,
> -                4, 0x0000, 0x0000, 0x0000, 0x0000, be);
> +        SysBusDevice *s;
> +
> +        flash = pflash_cfi01_init(NULL, "lx60.io.flash", board->flash_size,
> +                                  blk_by_legacy_dinfo(dinfo),
> +                                  board->flash_sector_size,
> +                                  board->flash_size / board->flash_sector_size,
> +                                  4, 0x0000, 0x0000, 0x0000, 0x0000, be);
>          if (flash == NULL) {
>              error_report("unable to mount pflash");
>              exit(EXIT_FAILURE);
>          }
> +        s = SYS_BUS_DEVICE(flash);
> +        memory_region_add_subregion(system_io, board->flash_base,
> +                                    sysbus_mmio_get_region(s, 0));
>      }
>
>      /* Use presence of kernel file name as 'boot from SRAM' switch. */
> @@ -386,7 +390,7 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>  static void xtensa_lx60_init(MachineState *machine)
>  {
>      static const LxBoardDesc lx60_board = {
> -        .flash_base = 0xf8000000,
> +        .flash_base = 0x08000000,
>          .flash_size = 0x00400000,
>          .flash_sector_size = 0x10000,
>          .sram_size = 0x20000,
> @@ -397,7 +401,7 @@ static void xtensa_lx60_init(MachineState *machine)
>  static void xtensa_lx200_init(MachineState *machine)
>  {
>      static const LxBoardDesc lx200_board = {
> -        .flash_base = 0xf8000000,
> +        .flash_base = 0x08000000,
>          .flash_size = 0x01000000,
>          .flash_sector_size = 0x20000,
>          .sram_size = 0x2000000,
> @@ -408,7 +412,7 @@ static void xtensa_lx200_init(MachineState *machine)
>  static void xtensa_ml605_init(MachineState *machine)
>  {
>      static const LxBoardDesc ml605_board = {
> -        .flash_base = 0xf8000000,
> +        .flash_base = 0x08000000,
>          .flash_size = 0x01000000,
>          .flash_sector_size = 0x20000,
>          .sram_size = 0x2000000,
> @@ -419,7 +423,7 @@ static void xtensa_ml605_init(MachineState *machine)
>  static void xtensa_kc705_init(MachineState *machine)
>  {
>      static const LxBoardDesc kc705_board = {
> -        .flash_base = 0xf0000000,
> +        .flash_base = 0x00000000,
>          .flash_size = 0x08000000,
>          .flash_boot_base = 0x06000000,
>          .flash_sector_size = 0x20000,
> --
> 1.8.1.4
>
>

Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init

[Max Filippov]

On Sun, Sep 27, 2015 at 8:25 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> pflash_cfi01_register always registers FLASH to the default
>> system_memory region, which may not always be the right thing.
>>
>> Provide pflash_cfi01_init function that only creates FLASH device and
>> sets its properties, leaving MMIO region registration to its caller.
>>
>
> You should just use QOM to create your device inline without need for
> a construction helper. See Vexpress's ve_pflash_cfi01_register for an
> example.

I thought about it and it looks like a lot of boilerplate code for nothing.
Maybe it'd be better to remove qdev_init_nofail call from that init function
and let ve_pflash_cfi01_register reuse it too?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 2/3] target-xtensa: xtfpga: attach FLASH to system IO

[Max Filippov]

On Sun, Sep 27, 2015 at 8:42 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> XTFPGA FLASH is tied to XTFPGA system IO block. It's not very important
>> for systems with MMU where system IO block is visible at single
>> location,
>
> Are your relying on a matching change in the guest for MMU setup here?
>
> Can you explain a little more how the MMU allows a physical address
> remap of the underlying hardware as this looks like a non
> backwards-compatible board design rev.

No change is needed as this change doesn't affect memory map.
It only detaches FLASH from the system memory and attaches it
to the system IO region, at the same physical address as before.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> Cores with and without MMU have system RAM and ROM at different locations.
>> Also with noMMU cores system IO region is accessible through two physical
>> address ranges.
>>
>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> ---
>>  hw/xtensa/xtfpga.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 41 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>> index d4b9afb..b53f40d 100644
>> --- a/hw/xtensa/xtfpga.c
>> +++ b/hw/xtensa/xtfpga.c
>> @@ -199,7 +199,29 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>>      const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
>>      const char *dtb_filename = qemu_opt_get(machine_opts, "dtb");
>>      const char *initrd_filename = qemu_opt_get(machine_opts, "initrd");
>> +    const unsigned system_io_size = 224 * 1024 * 1024;
>> +    bool mmu;
>
> You are indexing into an array of configs so it's really an int (or
> better, an enum).

Ok.

>>      int n;
>
> Blank line.

Why?

>> +    static const struct {
>> +        hwaddr ram;
>> +        hwaddr rom;
>> +        hwaddr io[2];
>> +    } base[2] = {
>> +        {
>> +            .ram = 0x60000000,
>> +            .rom = 0x50000000,
>> +            .io = {
>> +                0x70000000,
>> +                0x90000000,
>> +            },
>> +        }, {
>> +            .ram = 0,
>> +            .rom = 0xfe000000,
>> +            .io = {
>> +                0xf0000000,
>> +            },
>> +        }
>> +    };
>>
>>      if (!cpu_model) {
>>          cpu_model = XTENSA_DEFAULT_CPU_MODEL;
>> @@ -222,16 +244,24 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>>          cpu_reset(CPU(cpu));
>>      }
>>
>> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>
> This looks backwards, the board should be in charge of itself and the
> CPU config, rather than spying on the CPU setup to rewire the board.

Well, it's an FPGA board and all connections are a part of bitstream.
It's generated that way, I'm just following the specification here.

>>      ram = g_malloc(sizeof(*ram));
>>      memory_region_init_ram(ram, NULL, "lx60.dram", machine->ram_size,
>>                             &error_fatal);
>>      vmstate_register_ram_global(ram);
>> -    memory_region_add_subregion(system_memory, 0, ram);
>> +    memory_region_add_subregion(system_memory, base[mmu].ram, ram);
>>
>>      system_io = g_malloc(sizeof(*system_io));
>>      memory_region_init_io(system_io, NULL, &lx60_io_ops, NULL, "lx60.io",
>> -                          224 * 1024 * 1024);
>> -    memory_region_add_subregion(system_memory, 0xf0000000, system_io);
>> +                          system_io_size);
>> +    memory_region_add_subregion(system_memory, base[mmu].io[0], system_io);
>> +    if (!mmu) {
>
> The boolean switch for whether the alias exists could go in the
> struct. That makes it more robust to add yet more configs in the
> future rather than iffery on the config index.

Ok.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 10:51 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 8:25 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> pflash_cfi01_register always registers FLASH to the default
>>> system_memory region, which may not always be the right thing.
>>>
>>> Provide pflash_cfi01_init function that only creates FLASH device and
>>> sets its properties, leaving MMIO region registration to its caller.
>>>
>>
>> You should just use QOM to create your device inline without need for
>> a construction helper. See Vexpress's ve_pflash_cfi01_register for an
>> example.
>
> I thought about it and it looks like a lot of boilerplate code for nothing.

Not really. It self documents the code. This is instantly understandable:

    qdev_prop_set_uint32(dev, "num-blocks",
                         VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE);
    qdev_prop_set_uint64(dev, "sector-length", VEXPRESS_FLASH_SECT_SIZE);
    qdev_prop_set_uint8(dev, "width", 4);
    qdev_prop_set_uint8(dev, "device-width", 2);
    qdev_prop_set_bit(dev, "big-endian", false);
    qdev_prop_set_uint16(dev, "id0", 0x89);
    qdev_prop_set_uint16(dev, "id1", 0x18);
    qdev_prop_set_uint16(dev, "id2", 0x00);
    qdev_prop_set_uint16(dev, "id3", 0x00);
    qdev_prop_set_string(dev, "name", name);

Whereas a call to pflash_cfi_register is a jumble of numbers without
any labelling of what is what. In this scheme you don't have to open
headers to figure out what args are.

> Maybe it'd be better to remove qdev_init_nofail call from that init function
> and let ve_pflash_cfi01_register reuse it too?
>

There was a conscious decision some time ago to get rid of qdev init
helper functions as provided by devs themselves. The interface between
dev and board should be QOM.

Regards,
Peter

> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 2/3] target-xtensa: xtfpga: attach FLASH to system IO

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 11:01 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 8:42 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> XTFPGA FLASH is tied to XTFPGA system IO block. It's not very important
>>> for systems with MMU where system IO block is visible at single
>>> location,
>>
>> Are your relying on a matching change in the guest for MMU setup here?
>>
>> Can you explain a little more how the MMU allows a physical address
>> remap of the underlying hardware as this looks like a non
>> backwards-compatible board design rev.
>
> No change is needed as this change doesn't affect memory map.
> It only detaches FLASH from the system memory and attaches it
> to the system IO region, at the same physical address as before.
>

Ok I get it now :). Commit message confused me a little with the MMU
stuff but reading it again it makes sense. Change looks good. I'll
wait for resolution on P1 before putting an RB.

Regards,
Peter

> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 1/3] pflash_cfi01: add pflash_cfi01_init

[Max Filippov]

On Sun, Sep 27, 2015 at 9:32 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 10:51 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 8:25 PM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>> pflash_cfi01_register always registers FLASH to the default
>>>> system_memory region, which may not always be the right thing.
>>>>
>>>> Provide pflash_cfi01_init function that only creates FLASH device and
>>>> sets its properties, leaving MMIO region registration to its caller.
>>>>
>>>
>>> You should just use QOM to create your device inline without need for
>>> a construction helper. See Vexpress's ve_pflash_cfi01_register for an
>>> example.
>>
>> I thought about it and it looks like a lot of boilerplate code for nothing.
>
> Not really. It self documents the code. This is instantly understandable:
>
>     qdev_prop_set_uint32(dev, "num-blocks",
>                          VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE);
>     qdev_prop_set_uint64(dev, "sector-length", VEXPRESS_FLASH_SECT_SIZE);
>     qdev_prop_set_uint8(dev, "width", 4);
>     qdev_prop_set_uint8(dev, "device-width", 2);
>     qdev_prop_set_bit(dev, "big-endian", false);
>     qdev_prop_set_uint16(dev, "id0", 0x89);
>     qdev_prop_set_uint16(dev, "id1", 0x18);
>     qdev_prop_set_uint16(dev, "id2", 0x00);
>     qdev_prop_set_uint16(dev, "id3", 0x00);
>     qdev_prop_set_string(dev, "name", name);
>
> Whereas a call to pflash_cfi_register is a jumble of numbers without
> any labelling of what is what. In this scheme you don't have to open
> headers to figure out what args are.
>
>> Maybe it'd be better to remove qdev_init_nofail call from that init function
>> and let ve_pflash_cfi01_register reuse it too?
>>
>
> There was a conscious decision some time ago to get rid of qdev init
> helper functions as provided by devs themselves. The interface between
> dev and board should be QOM.

Ok.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 11:13 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> Cores with and without MMU have system RAM and ROM at different locations.
>>> Also with noMMU cores system IO region is accessible through two physical
>>> address ranges.
>>>
>>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>>> ---
>>>  hw/xtensa/xtfpga.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
>>>  1 file changed, 41 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
>>> index d4b9afb..b53f40d 100644
>>> --- a/hw/xtensa/xtfpga.c
>>> +++ b/hw/xtensa/xtfpga.c
>>> @@ -199,7 +199,29 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>>>      const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
>>>      const char *dtb_filename = qemu_opt_get(machine_opts, "dtb");
>>>      const char *initrd_filename = qemu_opt_get(machine_opts, "initrd");
>>> +    const unsigned system_io_size = 224 * 1024 * 1024;
>>> +    bool mmu;
>>
>> You are indexing into an array of configs so it's really an int (or
>> better, an enum).
>
> Ok.
>
>>>      int n;
>>
>> Blank line.
>
> Why?
>

Just a readability suggestion. You have a collection of short defs
that then runs straight into a lengthy self-contained table.

>>> +    static const struct {
>>> +        hwaddr ram;
>>> +        hwaddr rom;
>>> +        hwaddr io[2];
>>> +    } base[2] = {
>>> +        {
>>> +            .ram = 0x60000000,
>>> +            .rom = 0x50000000,
>>> +            .io = {
>>> +                0x70000000,
>>> +                0x90000000,
>>> +            },
>>> +        }, {
>>> +            .ram = 0,
>>> +            .rom = 0xfe000000,
>>> +            .io = {
>>> +                0xf0000000,
>>> +            },
>>> +        }
>>> +    };
>>>
>>>      if (!cpu_model) {
>>>          cpu_model = XTENSA_DEFAULT_CPU_MODEL;
>>> @@ -222,16 +244,24 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
>>>          cpu_reset(CPU(cpu));
>>>      }
>>>
>>> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>>
>> This looks backwards, the board should be in charge of itself and the
>> CPU config, rather than spying on the CPU setup to rewire the board.
>
> Well, it's an FPGA board and all connections are a part of bitstream.
> It's generated that way, I'm just following the specification here.
>

OK, but the xtensa-CPU is not the bitstream, this board is. What
exactly is the user interface for switching between MMU and no-MMU?

With the major changes of address layout, the no-MMU variation should
be a set of new boards or a machine level parameterisation (i.e. QOM
property of the machine). It needs to be user-visible as different on
the machine level.

Regards,
Peter

>>>      ram = g_malloc(sizeof(*ram));
>>>      memory_region_init_ram(ram, NULL, "lx60.dram", machine->ram_size,
>>>                             &error_fatal);
>>>      vmstate_register_ram_global(ram);
>>> -    memory_region_add_subregion(system_memory, 0, ram);
>>> +    memory_region_add_subregion(system_memory, base[mmu].ram, ram);
>>>
>>>      system_io = g_malloc(sizeof(*system_io));
>>>      memory_region_init_io(system_io, NULL, &lx60_io_ops, NULL, "lx60.io",
>>> -                          224 * 1024 * 1024);
>>> -    memory_region_add_subregion(system_memory, 0xf0000000, system_io);
>>> +                          system_io_size);
>>> +    memory_region_add_subregion(system_memory, base[mmu].io[0], system_io);
>>> +    if (!mmu) {
>>
>> The boolean switch for whether the alias exists could go in the
>> struct. That makes it more robust to add yet more configs in the
>> future rather than iffery on the config index.
>
> Ok.
>
> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Sun, Sep 27, 2015 at 9:43 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 11:13 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>>      int n;
>>>
>>> Blank line.
>>
>> Why?
>>
> Just a readability suggestion. You have a collection of short defs
> that then runs straight into a lengthy self-contained table.

Ok

>>>> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>>>
>>> This looks backwards, the board should be in charge of itself and the
>>> CPU config, rather than spying on the CPU setup to rewire the board.
>>
>> Well, it's an FPGA board and all connections are a part of bitstream.
>> It's generated that way, I'm just following the specification here.
>>
>
> OK, but the xtensa-CPU is not the bitstream, this board is. What
> exactly is the user interface for switching between MMU and no-MMU?

Actually they both are. The user interface is a dropbox in the processor
generator software where user chooses memory management option.
Once it (and a bunch of other parameters) is chosen the bitstream with
CPU and peripherals can be generated.

> With the major changes of address layout, the no-MMU variation should
> be a set of new boards or a machine level parameterisation (i.e. QOM
> property of the machine). It needs to be user-visible as different on
> the machine level.

Why? The layouts are hard-coded based on MMU presence anyway.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 12:01 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 9:43 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 11:13 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> On Sun, Sep 27, 2015 at 10:16 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>>>      int n;
>>>>
>>>> Blank line.
>>>
>>> Why?
>>>
>> Just a readability suggestion. You have a collection of short defs
>> that then runs straight into a lengthy self-contained table.
>
> Ok
>
>>>>> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>>>>
>>>> This looks backwards, the board should be in charge of itself and the
>>>> CPU config, rather than spying on the CPU setup to rewire the board.
>>>
>>> Well, it's an FPGA board and all connections are a part of bitstream.
>>> It's generated that way, I'm just following the specification here.
>>>
>>
>> OK, but the xtensa-CPU is not the bitstream, this board is. What
>> exactly is the user interface for switching between MMU and no-MMU?
>
> Actually they both are. The user interface is a dropbox in the processor
> generator software where user chooses memory management option.
> Once it (and a bunch of other parameters) is chosen the bitstream with
> CPU and peripherals can be generated.
>
>> With the major changes of address layout, the no-MMU variation should
>> be a set of new boards or a machine level parameterisation (i.e. QOM
>> property of the machine). It needs to be user-visible as different on
>> the machine level.
>
> Why? The layouts are hard-coded based on MMU presence anyway.
>

So this really means that MMUness is board level property, not a CPU
level property.

To clarify, can you tell me the QEMU command line difference between
MMU and noMMU?

Regards,
Peter

> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Mon, Sep 28, 2015 at 12:28 AM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 12:01 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 9:43 PM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Sun, Sep 27, 2015 at 11:13 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>> On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
>>>>>> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>>>>>
>>>>> This looks backwards, the board should be in charge of itself and the
>>>>> CPU config, rather than spying on the CPU setup to rewire the board.
>>>>
>>>> Well, it's an FPGA board and all connections are a part of bitstream.
>>>> It's generated that way, I'm just following the specification here.
>>>>
>>>
>>> OK, but the xtensa-CPU is not the bitstream, this board is. What
>>> exactly is the user interface for switching between MMU and no-MMU?
>>
>> Actually they both are. The user interface is a dropbox in the processor
>> generator software where user chooses memory management option.
>> Once it (and a bunch of other parameters) is chosen the bitstream with
>> CPU and peripherals can be generated.
>>
>>> With the major changes of address layout, the no-MMU variation should
>>> be a set of new boards or a machine level parameterisation (i.e. QOM
>>> property of the machine). It needs to be user-visible as different on
>>> the machine level.
>>
>> Why? The layouts are hard-coded based on MMU presence anyway.
>
> So this really means that MMUness is board level property, not a CPU
> level property.

CPU is primary, because one can generate RTL for CPU only. If a whole
FPGA bitstream is generated, address space layout will be chosen in
accordance with MMU presence in CPU.

> To clarify, can you tell me the QEMU command line difference between
> MMU and noMMU?

There is no difference. You specify -cpu without full MMU -- you get
noMMU address space layout. There are no noMMU cores in the QEMU
mainline ATM, but there are cpu3400, dsp3400, lx106 and de108 in the
xtensa-cores branch of my tree
( https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores ).

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Sun, Sep 27, 2015 at 2:48 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Mon, Sep 28, 2015 at 12:28 AM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 12:01 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Sun, Sep 27, 2015 at 9:43 PM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> On Sun, Sep 27, 2015 at 11:13 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>>> On Sun, Sep 27, 2015 at 8:38 PM, Peter Crosthwaite
>>>>>>> +    mmu = xtensa_option_enabled(env->config, XTENSA_OPTION_MMU);
>>>>>>
>>>>>> This looks backwards, the board should be in charge of itself and the
>>>>>> CPU config, rather than spying on the CPU setup to rewire the board.
>>>>>
>>>>> Well, it's an FPGA board and all connections are a part of bitstream.
>>>>> It's generated that way, I'm just following the specification here.
>>>>>
>>>>
>>>> OK, but the xtensa-CPU is not the bitstream, this board is. What
>>>> exactly is the user interface for switching between MMU and no-MMU?
>>>
>>> Actually they both are. The user interface is a dropbox in the processor
>>> generator software where user chooses memory management option.
>>> Once it (and a bunch of other parameters) is chosen the bitstream with
>>> CPU and peripherals can be generated.
>>>
>>>> With the major changes of address layout, the no-MMU variation should
>>>> be a set of new boards or a machine level parameterisation (i.e. QOM
>>>> property of the machine). It needs to be user-visible as different on
>>>> the machine level.
>>>
>>> Why? The layouts are hard-coded based on MMU presence anyway.
>>
>> So this really means that MMUness is board level property, not a CPU
>> level property.
>
> CPU is primary, because one can generate RTL for CPU only. If a whole
> FPGA bitstream is generated, address space layout will be chosen in
> accordance with MMU presence in CPU.
>
>> To clarify, can you tell me the QEMU command line difference between
>> MMU and noMMU?
>
> There is no difference. You specify -cpu without full MMU -- you get
> noMMU address space layout.

Ok but I think this is what we want to avoid. Using -cpu to switch up
the board/SoC architecture. The address space layout is SoC level (and
in your case a bitstream constitutes and entire SoC). There's a
lengthy discussion on this here:

http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03979.html

going back and forth from that point in the thread. Your hardware
model is more accurate that what was proposed by OP in that thread,
but it is good to keep the interfaces consistent with other machine
models.

Regards,
Peter

> There are no noMMU cores in the QEMU
> mainline ATM, but there are cpu3400, dsp3400, lx106 and de108 in the
> xtensa-cores branch of my tree
> ( https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores ).
>
> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Mon, Sep 28, 2015 at 12:59 AM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sun, Sep 27, 2015 at 2:48 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Mon, Sep 28, 2015 at 12:28 AM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> To clarify, can you tell me the QEMU command line difference between
>>> MMU and noMMU?
>>
>> There is no difference. You specify -cpu without full MMU -- you get
>> noMMU address space layout.
>
> Ok but I think this is what we want to avoid. Using -cpu to switch up
> the board/SoC architecture. The address space layout is SoC level (and
> in your case a bitstream constitutes and entire SoC). There's a
> lengthy discussion on this here:
>
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03979.html
>
> going back and forth from that point in the thread. Your hardware
> model is more accurate that what was proposed by OP in that thread,
> but it is good to keep the interfaces consistent with other machine
> models.

The message by the link says:

  As Andreas says, we need to model real actual hardware,
  not some abstraction that kind of matches the kernel's
  abstractions.

Changing address space layout according to CPU type is what happens
in actual hardware. There are no user-controllable settings that would
allow mismatching address space layout and CPU type on XTFPGA
boards. There's also no SoC level mentioned in the developer guides
for the corresponding boards. So I'm not sure what you're proposing to do.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Tue, Sep 29, 2015 at 3:34 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Mon, Sep 28, 2015 at 12:59 AM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 2:48 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Mon, Sep 28, 2015 at 12:28 AM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> To clarify, can you tell me the QEMU command line difference between
>>>> MMU and noMMU?
>>>
>>> There is no difference. You specify -cpu without full MMU -- you get
>>> noMMU address space layout.
>>
>> Ok but I think this is what we want to avoid. Using -cpu to switch up
>> the board/SoC architecture. The address space layout is SoC level (and
>> in your case a bitstream constitutes and entire SoC). There's a
>> lengthy discussion on this here:
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03979.html
>>
>> going back and forth from that point in the thread. Your hardware
>> model is more accurate that what was proposed by OP in that thread,
>> but it is good to keep the interfaces consistent with other machine
>> models.
>
> The message by the link says:
>
>   As Andreas says, we need to model real actual hardware,
>   not some abstraction that kind of matches the kernel's
>   abstractions.
>
> Changing address space layout according to CPU type is what happens
> in actual hardware. There are no user-controllable settings that would
> allow mismatching address space layout and CPU type on XTFPGA
> boards.

But that would actually be the semantics of -cpu. Change just the CPU.

>From earlier in this thread:

"
>> The user interface is a dropbox in the processor
>> generator software where user chooses memory management option.
>> Once it (and a bunch of other parameters) is chosen the bitstream with
>> CPU and peripherals can be generated.
"

The key part to me is "bitstream with peripherals". Using qemu
terminology, the dropbox you describe is not a CPU configurator, it is
a machine (or SoC) configuration. Changing the MMU option in your
devtools configurates both the CPU proper and the system address map.

You want some sort of container that can serve as this higher level
configurable object. The drop down menus in your cad tools then map to
QOM props on the container level (not the CPU level). Currently you
don't have a SoC/Machine split, but that is OK, you can make these
props on the Machine (now that machines are fully QOMified). I suggest
whatever that MMU dropbox is textually labelled as in you cad tools,
you create the QOM property with the same name. The machine level then
forcibly sets the same feature on the CPU.

If you do that, there may no longer be a need for multiple xtensa CPU
types. There is only one xtensa-cpu type, It is just massively
configurable with all the devtool options propagated from the machine.
This was the general plan with microblaze and Alistair started on
propertyifying the cad tool configurable options individually (theres
a long way to go on that project).

-cpu is then defeatured for xtensa, which makes sense, as based on
your description, it is impossible to substitute an xtensa CPU without
global consequences.

You can then create a handful of reference machines with common
favourable settings, or take some sort of generated output from you
cad to drive a fully custom configuration.

> There's also no SoC level mentioned in the developer guides
> for the corresponding boards.

Just terminology mismatch. The output of your software tools is really
a SoC, not a CPU.

Regards,
Peter

> So I'm not sure what you're proposing to do.
>
> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Maydell]

On 29 September 2015 at 11:34, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Mon, Sep 28, 2015 at 12:59 AM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Sun, Sep 27, 2015 at 2:48 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Mon, Sep 28, 2015 at 12:28 AM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> To clarify, can you tell me the QEMU command line difference between
>>>> MMU and noMMU?
>>>
>>> There is no difference. You specify -cpu without full MMU -- you get
>>> noMMU address space layout.
>>
>> Ok but I think this is what we want to avoid. Using -cpu to switch up
>> the board/SoC architecture. The address space layout is SoC level (and
>> in your case a bitstream constitutes and entire SoC). There's a
>> lengthy discussion on this here:
>>
>> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg03979.html
>>
>> going back and forth from that point in the thread. Your hardware
>> model is more accurate that what was proposed by OP in that thread,
>> but it is good to keep the interfaces consistent with other machine
>> models.
>
> The message by the link says:
>
>   As Andreas says, we need to model real actual hardware,
>   not some abstraction that kind of matches the kernel's
>   abstractions.
>
> Changing address space layout according to CPU type is what happens
> in actual hardware. There are no user-controllable settings that would
> allow mismatching address space layout and CPU type on XTFPGA
> boards. There's also no SoC level mentioned in the developer guides
> for the corresponding boards. So I'm not sure what you're proposing to do.

I think this should clearly be different machine models
(possibly implemented using different SoC models). This isn't a
CPU-dependent thing at all, it's just your dev tools are hiding
"change the devices and other board/soc level things" behind a
CPU-type dropdown, which it can get away with because the whole
implementation is in a single FPGA.

Compare vexpress-a9 vs vexpress-a15 (which are modelling hardware
with a daughterboard with CPU and devices on it, which is sort
of analogous I think).

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Tue, Sep 29, 2015 at 10:20 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> You want some sort of container that can serve as this higher level
> configurable object. The drop down menus in your cad tools then map to
> QOM props on the container level (not the CPU level). Currently you
> don't have a SoC/Machine split, but that is OK, you can make these
> props on the Machine (now that machines are fully QOMified). I suggest
> whatever that MMU dropbox is textually labelled as in you cad tools,
> you create the QOM property with the same name. The machine level then
> forcibly sets the same feature on the CPU.

I don't get it. It can't forcibly set anything on CPU: it will change the CPU.
The CPU is primary, the machine is just a reference platform used for
demos. The user doesn't have two different machines: one for MMU cores
and another for noMMU cores, it has one, its name is the name of target
FPGA board.

> If you do that, there may no longer be a need for multiple xtensa CPU
> types. There is only one xtensa-cpu type, It is just massively
> configurable with all the devtool options propagated from the machine.

I think this is applicable to all CPUs with long enough history.
Xtensa is not special here.

> This was the general plan with microblaze and Alistair started on
> propertyifying the cad tool configurable options individually (theres
> a long way to go on that project).

> You can then create a handful of reference machines with common
> favourable settings, or take some sort of generated output from you
> cad to drive a fully custom configuration.

I do it now, but it has nothing to do with machine. Xtensa CPU is
configurable. I can import its configuration and create QEMU CPU
instance. We don't know anything about the machine that will use
it. XTFPGA is special, because it can be built together with the CPU.
Boards produced by customers may have much more rigid structure,
e.g. esp8266 is all fixed, the CPU is fixed as well.


That said I still don't understand, what good is having a machine with
QOM properties? And what good is having two machines, of which
only one can be used with each CPU instead of only the correct one?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Tue, Sep 29, 2015 at 11:42 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 29 September 2015 at 11:34, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> Changing address space layout according to CPU type is what happens
>> in actual hardware. There are no user-controllable settings that would
>> allow mismatching address space layout and CPU type on XTFPGA
>> boards. There's also no SoC level mentioned in the developer guides
>> for the corresponding boards. So I'm not sure what you're proposing to do.
>
> I think this should clearly be different machine models
> (possibly implemented using different SoC models).

There are already 4 different machine models, lx60, lx200, ml605 and
kc705, these are real boards for which bitstreams can be created.

> This isn't a
> CPU-dependent thing at all, it's just your dev tools are hiding
> "change the devices and other board/soc level things" behind a
> CPU-type dropdown, which it can get away with because the whole
> implementation is in a single FPGA.

Why duplicating each machine and then only allowing each CPU to be
used with only one variant of each machine? It doesn't match what
users do with boards and bitstreams, at all.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Maydell]

On 30 September 2015 at 21:17, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Tue, Sep 29, 2015 at 11:42 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 29 September 2015 at 11:34, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> Changing address space layout according to CPU type is what happens
>>> in actual hardware. There are no user-controllable settings that would
>>> allow mismatching address space layout and CPU type on XTFPGA
>>> boards. There's also no SoC level mentioned in the developer guides
>>> for the corresponding boards. So I'm not sure what you're proposing to do.
>>
>> I think this should clearly be different machine models
>> (possibly implemented using different SoC models).
>
> There are already 4 different machine models, lx60, lx200, ml605 and
> kc705, these are real boards for which bitstreams can be created.
>
>> This isn't a
>> CPU-dependent thing at all, it's just your dev tools are hiding
>> "change the devices and other board/soc level things" behind a
>> CPU-type dropdown, which it can get away with because the whole
>> implementation is in a single FPGA.
>
> Why duplicating each machine and then only allowing each CPU to be
> used with only one variant of each machine? It doesn't match what
> users do with boards and bitstreams, at all.

Because for QEMU the CPU is really just the CPU. Your other
random devices don't live in the CPU, even if you're
programming your FPGA with a single bitstream that's
got the CPUs and a set of devices in it. You can't push
that stuff into the CPU model, it should live in the
board model.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Wed, Sep 30, 2015 at 11:45 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 30 September 2015 at 21:17, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Tue, Sep 29, 2015 at 11:42 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> On 29 September 2015 at 11:34, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>> Changing address space layout according to CPU type is what happens
>>>> in actual hardware. There are no user-controllable settings that would
>>>> allow mismatching address space layout and CPU type on XTFPGA
>>>> boards. There's also no SoC level mentioned in the developer guides
>>>> for the corresponding boards. So I'm not sure what you're proposing to do.
>>>
>>> I think this should clearly be different machine models
>>> (possibly implemented using different SoC models).
>>
>> There are already 4 different machine models, lx60, lx200, ml605 and
>> kc705, these are real boards for which bitstreams can be created.
>>
>>> This isn't a
>>> CPU-dependent thing at all, it's just your dev tools are hiding
>>> "change the devices and other board/soc level things" behind a
>>> CPU-type dropdown, which it can get away with because the whole
>>> implementation is in a single FPGA.
>>
>> Why duplicating each machine and then only allowing each CPU to be
>> used with only one variant of each machine? It doesn't match what
>> users do with boards and bitstreams, at all.
>
> Because for QEMU the CPU is really just the CPU. Your other
> random devices don't live in the CPU, even if you're
> programming your FPGA with a single bitstream that's
> got the CPUs and a set of devices in it. You can't push
> that stuff into the CPU model, it should live in the
> board model.

I'm not pushing them into CPU model and they do live in the board
model. After all the changes in question were for the hw/xtensa/xtfpga.c
But the board is made in such way that CPUs with and without MMU
see onboard RAM and peripherals at different physical addresses.
I still can't understand what's wrong with that.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Wed, Sep 30, 2015 at 1:54 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 11:45 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 30 September 2015 at 21:17, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Tue, Sep 29, 2015 at 11:42 PM, Peter Maydell
>>> <peter.maydell@linaro.org> wrote:
>>>> On 29 September 2015 at 11:34, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>>> Changing address space layout according to CPU type is what happens
>>>>> in actual hardware. There are no user-controllable settings that would
>>>>> allow mismatching address space layout and CPU type on XTFPGA
>>>>> boards. There's also no SoC level mentioned in the developer guides
>>>>> for the corresponding boards. So I'm not sure what you're proposing to do.
>>>>
>>>> I think this should clearly be different machine models
>>>> (possibly implemented using different SoC models).
>>>
>>> There are already 4 different machine models, lx60, lx200, ml605 and
>>> kc705, these are real boards for which bitstreams can be created.
>>>
>>>> This isn't a
>>>> CPU-dependent thing at all, it's just your dev tools are hiding
>>>> "change the devices and other board/soc level things" behind a
>>>> CPU-type dropdown, which it can get away with because the whole
>>>> implementation is in a single FPGA.
>>>
>>> Why duplicating each machine and then only allowing each CPU to be
>>> used with only one variant of each machine? It doesn't match what
>>> users do with boards and bitstreams, at all.
>>
>> Because for QEMU the CPU is really just the CPU. Your other
>> random devices don't live in the CPU, even if you're
>> programming your FPGA with a single bitstream that's
>> got the CPUs and a set of devices in it. You can't push
>> that stuff into the CPU model, it should live in the
>> board model.
>
> I'm not pushing them into CPU model and they do live in the board
> model. After all the changes in question were for the hw/xtensa/xtfpga.c
> But the board is made in such way that CPUs with and without MMU
> see onboard RAM and peripherals at different physical addresses.

Which makes MMU vs noMMU a board level property, not a CPU property.
It should appear in QEMU as such.

Regards,
Peter

> I still can't understand what's wrong with that.
>
> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 1:54 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Wed, Sep 30, 2015 at 11:45 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> Because for QEMU the CPU is really just the CPU. Your other
>>> random devices don't live in the CPU, even if you're
>>> programming your FPGA with a single bitstream that's
>>> got the CPUs and a set of devices in it. You can't push
>>> that stuff into the CPU model, it should live in the
>>> board model.
>>
>> I'm not pushing them into CPU model and they do live in the board
>> model. After all the changes in question were for the hw/xtensa/xtfpga.c
>> But the board is made in such way that CPUs with and without MMU
>> see onboard RAM and peripherals at different physical addresses.
>
> Which makes MMU vs noMMU a board level property, not a CPU property.
> It should appear in QEMU as such.

Ok, one last try: can this property be queried from the selected CPU
and passed to SoC initialization code?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Wed, Sep 30, 2015 at 1:54 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Wed, Sep 30, 2015 at 11:45 PM, Peter Maydell
>>> <peter.maydell@linaro.org> wrote:
>>>> Because for QEMU the CPU is really just the CPU. Your other
>>>> random devices don't live in the CPU, even if you're
>>>> programming your FPGA with a single bitstream that's
>>>> got the CPUs and a set of devices in it. You can't push
>>>> that stuff into the CPU model, it should live in the
>>>> board model.
>>>
>>> I'm not pushing them into CPU model and they do live in the board
>>> model. After all the changes in question were for the hw/xtensa/xtfpga.c
>>> But the board is made in such way that CPUs with and without MMU
>>> see onboard RAM and peripherals at different physical addresses.
>>
>> Which makes MMU vs noMMU a board level property, not a CPU property.
>> It should appear in QEMU as such.
>
> Ok, one last try: can this property be queried from the selected CPU
> and passed to SoC initialization code?
>

No, what I am trying to say is go the other way round. The property is
on the SoC, and it is used to select the CPU type.

Regards,
Peter

> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Wed, Sep 30, 2015 at 3:23 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Wed, Sep 30, 2015 at 1:54 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>> On Wed, Sep 30, 2015 at 11:45 PM, Peter Maydell
>>>> <peter.maydell@linaro.org> wrote:
>>>>> Because for QEMU the CPU is really just the CPU. Your other
>>>>> random devices don't live in the CPU, even if you're
>>>>> programming your FPGA with a single bitstream that's
>>>>> got the CPUs and a set of devices in it. You can't push
>>>>> that stuff into the CPU model, it should live in the
>>>>> board model.
>>>>
>>>> I'm not pushing them into CPU model and they do live in the board
>>>> model. After all the changes in question were for the hw/xtensa/xtfpga.c
>>>> But the board is made in such way that CPUs with and without MMU
>>>> see onboard RAM and peripherals at different physical addresses.
>>>
>>> Which makes MMU vs noMMU a board level property, not a CPU property.
>>> It should appear in QEMU as such.
>>
>> Ok, one last try: can this property be queried from the selected CPU
>> and passed to SoC initialization code?
>>
>
> No, what I am trying to say is go the other way round. The property is
> on the SoC, and it is used to select the CPU type.
>

Another thought that may help/clarify. Maybe xtfpga, or a lot of it's
content belongs as an object in hw/cpu/. Its not a CPU (QEMU
terminology), but it's not a board either, it is something inbetween.
There are two concepts in the ARM world, that are in the middle, SoC's
and MPCores. MPCores live in hw/cpu/. Some notes.

* the fact that ARM boards still create the CPUs on the side of MPU
cores is a design issue I am trying to fix.
* ARMs virt machine using CPU type to select MPCore is a legacy.

Regards,
Peter

> Regards,
> Peter
>
>> --
>> Thanks.
>> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> Which makes MMU vs noMMU a board level property, not a CPU property.
>>> It should appear in QEMU as such.
>>
>> Ok, one last try: can this property be queried from the selected CPU
>> and passed to SoC initialization code?
>
> No, what I am trying to say is go the other way round. The property is
> on the SoC, and it is used to select the CPU type.

This is not what happens in reality. In reality the MMU type is the
CPU property.
A set of CPU properties comes from the outside along with the CPU name, and
that's the only possible set of properties for this name.
XTFPGA bitstream build scripts lay out its address space based on MMU
type of the selected CPU.

So let's make XTFPGA an SoC with a property that selects its address space
layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU, it
queries MMU type from it and instantiates XTFPGA SoC with proper address
space. That seems to be closest to what happens in reality.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Wed, Sep 30, 2015 at 3:42 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> Which makes MMU vs noMMU a board level property, not a CPU property.
>>>> It should appear in QEMU as such.
>>>
>>> Ok, one last try: can this property be queried from the selected CPU
>>> and passed to SoC initialization code?
>>
>> No, what I am trying to say is go the other way round. The property is
>> on the SoC, and it is used to select the CPU type.
>
> This is not what happens in reality. In reality the MMU type is the
> CPU property.
> A set of CPU properties comes from the outside along with the CPU name, and
> that's the only possible set of properties for this name.
> XTFPGA bitstream build scripts lay out its address space based on MMU
> type of the selected CPU.
>
> So let's make XTFPGA an SoC with a property that selects its address space
> layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU,

So lx60/lx200 sound like isolated FPGA chips (Virtex 4 lx60?) while
ml605/kc705 are commonly available fully-featured FPGA dev boards.

Is lx60 short for a particular board featuring this part or is it more abstract?

> it
> queries MMU type from it and instantiates XTFPGA SoC with proper address
> space. That seems to be closest to what happens in reality.
>

What is your intended user command lines? E.g. how do I boot with
ml605 no-mmu and then with mmu.

Regards,
Peter

> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Wed, Sep 30, 2015 at 3:42 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
>>>> <crosthwaitepeter@gmail.com> wrote:
>>>>> Which makes MMU vs noMMU a board level property, not a CPU property.
>>>>> It should appear in QEMU as such.
>>>>
>>>> Ok, one last try: can this property be queried from the selected CPU
>>>> and passed to SoC initialization code?
>>>
>>> No, what I am trying to say is go the other way round. The property is
>>> on the SoC, and it is used to select the CPU type.
>>
>> This is not what happens in reality. In reality the MMU type is the
>> CPU property.
>> A set of CPU properties comes from the outside along with the CPU name, and
>> that's the only possible set of properties for this name.
>> XTFPGA bitstream build scripts lay out its address space based on MMU
>> type of the selected CPU.
>>
>> So let's make XTFPGA an SoC with a property that selects its address space
>> layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU,
>
> So lx60/lx200 sound like isolated FPGA chips (Virtex 4 lx60?) while
> ml605/kc705 are commonly available fully-featured FPGA dev boards.

Actually they're all fully-featured FPGA dev boards, from different times.
lx60 and lx200 boards were produced by Avnet (yes, Virtex 4 based),
ml605 and kc705 are from Xilinx.

> Is lx60 short for a particular board featuring this part or is it more abstract?

Less memory, smaller onboard FLASH, different audio subsystem (not
currently modeled).

>> it
>> queries MMU type from it and instantiates XTFPGA SoC with proper address
>> space. That seems to be closest to what happens in reality.
>
> What is your intended user command lines? E.g. how do I boot with
> ml605 no-mmu and then with mmu.

'-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline yet,
can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores),
'-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't
have full MMU
and dc233c does.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Thu, Oct 1, 2015 at 11:25 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Wed, Sep 30, 2015 at 3:42 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Thu, Oct 1, 2015 at 1:23 AM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> On Wed, Sep 30, 2015 at 3:07 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>>>> On Thu, Oct 1, 2015 at 12:02 AM, Peter Crosthwaite
>>>>> <crosthwaitepeter@gmail.com> wrote:
>>>>>> Which makes MMU vs noMMU a board level property, not a CPU property.
>>>>>> It should appear in QEMU as such.
>>>>>
>>>>> Ok, one last try: can this property be queried from the selected CPU
>>>>> and passed to SoC initialization code?
>>>>
>>>> No, what I am trying to say is go the other way round. The property is
>>>> on the SoC, and it is used to select the CPU type.
>>>
>>> This is not what happens in reality. In reality the MMU type is the
>>> CPU property.
>>> A set of CPU properties comes from the outside along with the CPU name, and
>>> that's the only possible set of properties for this name.
>>> XTFPGA bitstream build scripts lay out its address space based on MMU
>>> type of the selected CPU.
>>>
>>> So let's make XTFPGA an SoC with a property that selects its address space
>>> layout. The machine (lx60/lx200/ml605/kc705) is configured with a CPU,
>>
>> So lx60/lx200 sound like isolated FPGA chips (Virtex 4 lx60?) while
>> ml605/kc705 are commonly available fully-featured FPGA dev boards.
>
> Actually they're all fully-featured FPGA dev boards, from different times.
> lx60 and lx200 boards were produced by Avnet (yes, Virtex 4 based),
> ml605 and kc705 are from Xilinx.
>
>> Is lx60 short for a particular board featuring this part or is it more abstract?
>
> Less memory, smaller onboard FLASH, different audio subsystem (not
> currently modeled).
>
>>> it
>>> queries MMU type from it and instantiates XTFPGA SoC with proper address
>>> space. That seems to be closest to what happens in reality.
>>
>> What is your intended user command lines? E.g. how do I boot with
>> ml605 no-mmu and then with mmu.
>
> '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline yet,
> can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores),
> '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't
> have full MMU
> and dc233c does.
>

So where I was going with this, is due to the way your devtools do
memory maps, -cpu is the wrong switch altogether as what your tools
call a "CPU" is different to what QEMU does. I am thinking:

-M ml605 -global ml605.bitstream=xtensa-dc233c

dc233c is a SoC that selects the correct xtensa CPU (either via
property or an target-extensa CPU also named "dc233c"). When you set
the bitstream property on the machine, it instantiates the correct SoC
which in turn already knows the correct memory map and selects the CPU
for you.

When we have multi-arch working, we can then code-share the FPGA board
definitions between arches e.g. petalogix-ml605 can be rewritten so
that this works:

qemu-system-multi -M ml605 -global ml605.bitstream=petalogix-mmu
qemu-system-multi -M ml605 -global ml605.bitstream=xtensa-dc233c

Names may needs some fine tuning.

Regards,
Peter

> --
> Thanks.
> -- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Max Filippov]

On Thu, Oct 1, 2015 at 10:18 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Thu, Oct 1, 2015 at 11:25 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite
>> <crosthwaitepeter@gmail.com> wrote:
>>> What is your intended user command lines? E.g. how do I boot with
>>> ml605 no-mmu and then with mmu.
>>
>> '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline yet,
>> can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores),
>> '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't
>> have full MMU
>> and dc233c does.
>
> So where I was going with this, is due to the way your devtools do
> memory maps, -cpu is the wrong switch altogether as what your tools
> call a "CPU" is different to what QEMU does. I am thinking:
>
> -M ml605 -global ml605.bitstream=xtensa-dc233c
>
> dc233c is a SoC that selects the correct xtensa CPU (either via
> property or an target-extensa CPU also named "dc233c"). When you set
> the bitstream property on the machine, it instantiates the correct SoC
> which in turn already knows the correct memory map and selects the CPU
> for you.
>
> When we have multi-arch working, we can then code-share the FPGA board
> definitions between arches e.g. petalogix-ml605 can be rewritten so
> that this works:
>
> qemu-system-multi -M ml605 -global ml605.bitstream=petalogix-mmu
> qemu-system-multi -M ml605 -global ml605.bitstream=xtensa-dc233c
>
> Names may needs some fine tuning.

I'm completely lost here. I have a problem at hand: noMMU cores
can't run on XTFPGA boards because of wrong device memory map.
The board is not meant to be used outside target-xtensa at all, and
even for target-xtensa it's just a demo board.

What problem do I solve by implementing it in the way you suggest?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 3/3] target-xtensa: xtfpga: support noMMU cores

[Peter Crosthwaite]

On Thu, Oct 1, 2015 at 1:13 PM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> On Thu, Oct 1, 2015 at 10:18 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 11:25 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
>>> On Thu, Oct 1, 2015 at 9:06 PM, Peter Crosthwaite
>>> <crosthwaitepeter@gmail.com> wrote:
>>>> What is your intended user command lines? E.g. how do I boot with
>>>> ml605 no-mmu and then with mmu.
>>>
>>> '-M ml605 -cpu de108' will boot noMMU variant (de108 is not in the mainline yet,
>>> can be seen in the https://github.com/OSLL/qemu-xtensa/commits/xtensa-cores),
>>> '-M ml605 -cpu dc233c' will boot full MMU variant, as de108 doesn't
>>> have full MMU
>>> and dc233c does.
>>
>> So where I was going with this, is due to the way your devtools do
>> memory maps, -cpu is the wrong switch altogether as what your tools
>> call a "CPU" is different to what QEMU does. I am thinking:
>>
>> -M ml605 -global ml605.bitstream=xtensa-dc233c
>>
>> dc233c is a SoC that selects the correct xtensa CPU (either via
>> property or an target-extensa CPU also named "dc233c"). When you set
>> the bitstream property on the machine, it instantiates the correct SoC
>> which in turn already knows the correct memory map and selects the CPU
>> for you.
>>
>> When we have multi-arch working, we can then code-share the FPGA board
>> definitions between arches e.g. petalogix-ml605 can be rewritten so
>> that this works:
>>
>> qemu-system-multi -M ml605 -global ml605.bitstream=petalogix-mmu
>> qemu-system-multi -M ml605 -global ml605.bitstream=xtensa-dc233c
>>
>> Names may needs some fine tuning.
>
> I'm completely lost here. I have a problem at hand: noMMU cores
> can't run on XTFPGA boards because of wrong device memory map.

I would word it differently, - You have no support for your noMMU
designs at all.

The two designs may have the same peripheral set so they can and
should code share (probably to the point where they just have two
const tables just diffing the memory maps) but they are ultimately
different.

> The board is not meant to be used outside target-xtensa at all, and
> even for target-xtensa it's just a demo board.
>
> What problem do I solve by implementing it in the way you suggest?
>

You change the interchangable component from just a CPU (QEMU
definition of "CPU") to the whole FPGA bitstream which in reality is
what happens (unless we open the FPGA partial reconfiguration
can-of-worms). So the user interface is matched to what really happens
on the boards. The code is also more easily patched in the future,
should anyone want to reuse xtensa ml605 with total FPGA design
replacement.

Regards,
Peter

> --
> Thanks.
> -- Max