From: Alistair Francis <alistair23@gmail.com>
To: Bin Meng <bmeng.cn@gmail.com>
Cc: Bin Meng <bin.meng@windriver.com>,
"open list:RISC-V" <qemu-riscv@nongnu.org>,
Anup Patel <anup.patel@wdc.com>,
"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
Atish Patra <atish.patra@wdc.com>,
Alistair Francis <Alistair.Francis@wdc.com>,
Ivan Griffin <ivan.griffin@emdalo.com>
Subject: Re: [RESEND PATCH 8/9] hw/riscv: microchip_pfsoc: Correct DDR memory map
Date: Tue, 27 Oct 2020 13:55:39 -0700 [thread overview]
Message-ID: <CAKmqyKPx0GeMCtBsPJ_VXf7ukMKemXMebEMaXSCABg+qN7va9Q@mail.gmail.com> (raw)
In-Reply-To: <20201027141740.18336-9-bmeng.cn@gmail.com>
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
>
>
next prev parent reply other threads:[~2020-10-27 21:08 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-27 14:17 [RESEND PATCH 0/9] hw/riscv: microchip_pfsoc: Support factory HSS boot out of the box Bin Meng
2020-10-27 14:17 ` [RESEND PATCH 1/9] hw/misc: Add Microchip PolarFire SoC DDR Memory Controller support Bin Meng
2020-10-27 20:45 ` Alistair Francis
2020-10-27 14:17 ` [RESEND PATCH 2/9] hw/riscv: microchip_pfsoc: Connect DDR memory controller modules Bin Meng
2020-10-27 17:37 ` Alistair Francis
2020-10-28 1:43 ` Bin Meng
2020-10-28 14:13 ` Alistair Francis
2020-10-27 14:17 ` [RESEND PATCH 3/9] hw/misc: Add Microchip PolarFire SoC IOSCB module support Bin Meng
2020-10-27 20:48 ` Alistair Francis
2020-10-27 14:17 ` [RESEND PATCH 4/9] hw/riscv: microchip_pfsoc: Connect the IOSCB module Bin Meng
2020-10-27 17:42 ` Alistair Francis
2020-10-27 14:17 ` [RESEND PATCH 5/9] hw/misc: Add Microchip PolarFire SoC SYSREG module support Bin Meng
2020-10-27 20:51 ` Alistair Francis
2020-10-27 14:17 ` [RESEND PATCH 6/9] hw/riscv: microchip_pfsoc: Connect the SYSREG module Bin Meng
2020-10-27 17:42 ` Alistair Francis
2020-10-27 14:17 ` [RESEND PATCH 7/9] hw/riscv: microchip_pfsoc: Map debug memory Bin Meng
2020-10-27 17:30 ` Alistair Francis
2020-10-28 2:08 ` Bin Meng
2020-10-27 14:17 ` [RESEND PATCH 8/9] hw/riscv: microchip_pfsoc: Correct DDR memory map Bin Meng
2020-10-27 20:55 ` Alistair Francis [this message]
2020-10-28 2:06 ` Bin Meng
2020-10-27 14:17 ` [RESEND PATCH 9/9] hw/riscv: microchip_pfsoc: Hook the I2C1 controller Bin Meng
2020-10-27 17:19 ` Alistair Francis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKmqyKPx0GeMCtBsPJ_VXf7ukMKemXMebEMaXSCABg+qN7va9Q@mail.gmail.com \
--to=alistair23@gmail.com \
--cc=Alistair.Francis@wdc.com \
--cc=anup.patel@wdc.com \
--cc=atish.patra@wdc.com \
--cc=bin.meng@windriver.com \
--cc=bmeng.cn@gmail.com \
--cc=ivan.griffin@emdalo.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-riscv@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).