diff mbox series

[RESEND,8/9] hw/riscv: microchip_pfsoc: Correct DDR memory map

Message ID 20201027141740.18336-9-bmeng.cn@gmail.com
State Superseded
Headers show
Series hw/riscv: microchip_pfsoc: Support factory HSS boot out of the box | expand

Commit Message

Bin Meng Oct. 27, 2020, 2:17 p.m. UTC
From: Bin Meng <bin.meng@windriver.com>

When system memory is larger than 1 GiB (high memory), PolarFire SoC
maps it at address 0x10_0000_0000. Address 0xC000_0000 and above is
aliased to the same 1 GiB low memory with different cache attributes.

At present QEMU maps the system memory contiguously from 0x8000_0000.
This corrects the wrong QEMU logic. Note address 0x14_0000_0000 is
the alias to the high memory, and even physical memory is only 1 GiB,
the HSS codes still tries to probe the high memory alias address.
It seems there is no issue on the real hardware, so we will have to
take that into the consideration in our emulation.

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

 hw/riscv/microchip_pfsoc.c         | 49 +++++++++++++++++++++++++++---
 include/hw/riscv/microchip_pfsoc.h |  5 ++-
 2 files changed, 48 insertions(+), 6 deletions(-)

Comments

Alistair Francis Oct. 27, 2020, 8:55 p.m. UTC | #1
On Tue, Oct 27, 2020 at 7:48 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>

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

>

> When system memory is larger than 1 GiB (high memory), PolarFire SoC

> maps it at address 0x10_0000_0000. Address 0xC000_0000 and above is

> aliased to the same 1 GiB low memory with different cache attributes.

>

> At present QEMU maps the system memory contiguously from 0x8000_0000.

> This corrects the wrong QEMU logic. Note address 0x14_0000_0000 is

> the alias to the high memory, and even physical memory is only 1 GiB,

> the HSS codes still tries to probe the high memory alias address.

> It seems there is no issue on the real hardware, so we will have to

> take that into the consideration in our emulation.

>

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

> ---

>

>  hw/riscv/microchip_pfsoc.c         | 49 +++++++++++++++++++++++++++---

>  include/hw/riscv/microchip_pfsoc.h |  5 ++-

>  2 files changed, 48 insertions(+), 6 deletions(-)

>

> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c

> index b9c2f73e7c..c595c9c967 100644

> --- a/hw/riscv/microchip_pfsoc.c

> +++ b/hw/riscv/microchip_pfsoc.c

> @@ -102,7 +102,10 @@ static const struct MemmapEntry {

>      [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },

>      [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },

>      [MICROCHIP_PFSOC_IOSCB] =           { 0x30000000, 0x10000000 },

> -    [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },

> +    [MICROCHIP_PFSOC_DRAM_LO] =         { 0x80000000, 0x40000000 },

> +    [MICROCHIP_PFSOC_DRAM_LO_ALIAS] =   { 0xc0000000, 0x40000000 },

> +    [MICROCHIP_PFSOC_DRAM_HI] =       { 0x1000000000,        0x0 },

> +    [MICROCHIP_PFSOC_DRAM_HI_ALIAS] = { 0x1400000000,        0x0 },

>  };

>

>  static void microchip_pfsoc_soc_instance_init(Object *obj)

> @@ -405,7 +408,11 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)

>      const struct MemmapEntry *memmap = microchip_pfsoc_memmap;

>      MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine);

>      MemoryRegion *system_memory = get_system_memory();

> -    MemoryRegion *main_mem = g_new(MemoryRegion, 1);

> +    MemoryRegion *mem_low = g_new(MemoryRegion, 1);

> +    MemoryRegion *mem_low_alias = g_new(MemoryRegion, 1);

> +    MemoryRegion *mem_high = g_new(MemoryRegion, 1);

> +    MemoryRegion *mem_high_alias = g_new(MemoryRegion, 1);

> +    uint64_t mem_high_size;

>      DriveInfo *dinfo = drive_get_next(IF_SD);

>

>      /* Sanity check on RAM size */

> @@ -422,10 +429,42 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)

>      qdev_realize(DEVICE(&s->soc), NULL, &error_abort);

>

>      /* Register RAM */

> -    memory_region_init_ram(main_mem, NULL, "microchip.icicle.kit.ram",

> -                           machine->ram_size, &error_fatal);

> +    memory_region_init_ram(mem_low, NULL, "microchip.icicle.kit.ram_low",

> +                           memmap[MICROCHIP_PFSOC_DRAM_LO].size,

> +                           &error_fatal);

> +    memory_region_init_alias(mem_low_alias, NULL,

> +                             "microchip.icicle.kit.ram_low.alias",

> +                             mem_low, 0,

> +                             memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].size);

> +    memory_region_add_subregion(system_memory,

> +                                memmap[MICROCHIP_PFSOC_DRAM_LO].base,

> +                                mem_low);

> +    memory_region_add_subregion(system_memory,

> +                                memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].base,

> +                                mem_low_alias);

> +

> +    /*

> +     * Map 1 GiB high memory because HSS will do memory test against the high

> +     * memory address range regardless of physical memory installed.

> +     *

> +     * See memory_tests() in mss_ddr.c in the HSS source code.

> +     */

> +    mem_high_size = machine->ram_size - 1 * GiB;

> +    if (mem_high_size < 1 * GiB) {

> +        mem_high_size = 1 * GiB;


This now means that the machine requires at least 2GB of memory!

Can you increase the default_ram_size so we will return an error if
the user specified less than 2GB instead of just increasing it without
their knowledge?

Alistair

> +    }

> +

> +    memory_region_init_ram(mem_high, NULL, "microchip.icicle.kit.ram_high",

> +                           mem_high_size, &error_fatal);

> +    memory_region_init_alias(mem_high_alias, NULL,

> +                             "microchip.icicle.kit.ram_high.alias",

> +                             mem_high, 0, mem_high_size);

> +    memory_region_add_subregion(system_memory,

> +                                memmap[MICROCHIP_PFSOC_DRAM_HI].base,

> +                                mem_high);

>      memory_region_add_subregion(system_memory,

> -                                memmap[MICROCHIP_PFSOC_DRAM].base, main_mem);

> +                                memmap[MICROCHIP_PFSOC_DRAM_HI_ALIAS].base,

> +                                mem_high_alias);

>

>      /* Load the firmware */

>      riscv_find_and_load_firmware(machine, BIOS_FILENAME, RESET_VECTOR, NULL);

> diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h

> index 245c82db61..dc05688d94 100644

> --- a/include/hw/riscv/microchip_pfsoc.h

> +++ b/include/hw/riscv/microchip_pfsoc.h

> @@ -104,7 +104,10 @@ enum {

>      MICROCHIP_PFSOC_ENVM_CFG,

>      MICROCHIP_PFSOC_ENVM_DATA,

>      MICROCHIP_PFSOC_IOSCB,

> -    MICROCHIP_PFSOC_DRAM,

> +    MICROCHIP_PFSOC_DRAM_LO,

> +    MICROCHIP_PFSOC_DRAM_LO_ALIAS,

> +    MICROCHIP_PFSOC_DRAM_HI,

> +    MICROCHIP_PFSOC_DRAM_HI_ALIAS

>  };

>

>  enum {

> --

> 2.25.1

>

>
diff mbox series

Patch

diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index b9c2f73e7c..c595c9c967 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -102,7 +102,10 @@  static const struct MemmapEntry {
     [MICROCHIP_PFSOC_ENVM_CFG] =        { 0x20200000,     0x1000 },
     [MICROCHIP_PFSOC_ENVM_DATA] =       { 0x20220000,    0x20000 },
     [MICROCHIP_PFSOC_IOSCB] =           { 0x30000000, 0x10000000 },
-    [MICROCHIP_PFSOC_DRAM] =            { 0x80000000,        0x0 },
+    [MICROCHIP_PFSOC_DRAM_LO] =         { 0x80000000, 0x40000000 },
+    [MICROCHIP_PFSOC_DRAM_LO_ALIAS] =   { 0xc0000000, 0x40000000 },
+    [MICROCHIP_PFSOC_DRAM_HI] =       { 0x1000000000,        0x0 },
+    [MICROCHIP_PFSOC_DRAM_HI_ALIAS] = { 0x1400000000,        0x0 },
 };
 
 static void microchip_pfsoc_soc_instance_init(Object *obj)
@@ -405,7 +408,11 @@  static void microchip_icicle_kit_machine_init(MachineState *machine)
     const struct MemmapEntry *memmap = microchip_pfsoc_memmap;
     MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine);
     MemoryRegion *system_memory = get_system_memory();
-    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
+    MemoryRegion *mem_low = g_new(MemoryRegion, 1);
+    MemoryRegion *mem_low_alias = g_new(MemoryRegion, 1);
+    MemoryRegion *mem_high = g_new(MemoryRegion, 1);
+    MemoryRegion *mem_high_alias = g_new(MemoryRegion, 1);
+    uint64_t mem_high_size;
     DriveInfo *dinfo = drive_get_next(IF_SD);
 
     /* Sanity check on RAM size */
@@ -422,10 +429,42 @@  static void microchip_icicle_kit_machine_init(MachineState *machine)
     qdev_realize(DEVICE(&s->soc), NULL, &error_abort);
 
     /* Register RAM */
-    memory_region_init_ram(main_mem, NULL, "microchip.icicle.kit.ram",
-                           machine->ram_size, &error_fatal);
+    memory_region_init_ram(mem_low, NULL, "microchip.icicle.kit.ram_low",
+                           memmap[MICROCHIP_PFSOC_DRAM_LO].size,
+                           &error_fatal);
+    memory_region_init_alias(mem_low_alias, NULL,
+                             "microchip.icicle.kit.ram_low.alias",
+                             mem_low, 0,
+                             memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].size);
+    memory_region_add_subregion(system_memory,
+                                memmap[MICROCHIP_PFSOC_DRAM_LO].base,
+                                mem_low);
+    memory_region_add_subregion(system_memory,
+                                memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].base,
+                                mem_low_alias);
+
+    /*
+     * Map 1 GiB high memory because HSS will do memory test against the high
+     * memory address range regardless of physical memory installed.
+     *
+     * See memory_tests() in mss_ddr.c in the HSS source code.
+     */
+    mem_high_size = machine->ram_size - 1 * GiB;
+    if (mem_high_size < 1 * GiB) {
+        mem_high_size = 1 * GiB;
+    }
+
+    memory_region_init_ram(mem_high, NULL, "microchip.icicle.kit.ram_high",
+                           mem_high_size, &error_fatal);
+    memory_region_init_alias(mem_high_alias, NULL,
+                             "microchip.icicle.kit.ram_high.alias",
+                             mem_high, 0, mem_high_size);
+    memory_region_add_subregion(system_memory,
+                                memmap[MICROCHIP_PFSOC_DRAM_HI].base,
+                                mem_high);
     memory_region_add_subregion(system_memory,
-                                memmap[MICROCHIP_PFSOC_DRAM].base, main_mem);
+                                memmap[MICROCHIP_PFSOC_DRAM_HI_ALIAS].base,
+                                mem_high_alias);
 
     /* Load the firmware */
     riscv_find_and_load_firmware(machine, BIOS_FILENAME, RESET_VECTOR, NULL);
diff --git a/include/hw/riscv/microchip_pfsoc.h b/include/hw/riscv/microchip_pfsoc.h
index 245c82db61..dc05688d94 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -104,7 +104,10 @@  enum {
     MICROCHIP_PFSOC_ENVM_CFG,
     MICROCHIP_PFSOC_ENVM_DATA,
     MICROCHIP_PFSOC_IOSCB,
-    MICROCHIP_PFSOC_DRAM,
+    MICROCHIP_PFSOC_DRAM_LO,
+    MICROCHIP_PFSOC_DRAM_LO_ALIAS,
+    MICROCHIP_PFSOC_DRAM_HI,
+    MICROCHIP_PFSOC_DRAM_HI_ALIAS
 };
 
 enum {