Re: [PATCH v2 17/24] hw/riscv: pfsoc: Correct PolarFire SoC DDR aliases

[Conor Dooley <conor.dooley@microchip.com>]

[-- Attachment #1: Type: text/plain, Size: 2966 bytes --]

On Fri, Sep 04, 2026 at 11:57:36PM +0800, Bin Meng wrote:
> The 64-bit cached and non-cached windows alias the complete physical
> DDR from offset zero. Treating them as only the portion above 1 GiB

This is not strictly true, how these regions overlap depends on how the
FPGA itself is configured. In the past, the regions were set up
sequentially and someone may choose to set their device up that way if
they want to, provided they change their devicetree to match.

I think the change is worth making, but the commit doing so should cite
reality and explain that this aliasing is the case in the reference
designs provided by microchip, not present it as the whole truth.

Ditto for the comment.

> leaves valid Icicle Kit memory nodes unbacked with the board's 2 GiB.
> 
> Map both high windows over the full machine RAM.
> 
> Signed-off-by: Bin Meng <bin.meng@processmission.com>
> Reviewed-by: Chao Liu <chao.liu@processmission.com>
> ---
> 
> (no changes since v1)
> 
>  hw/riscv/microchip_pfsoc.c | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 87c9c89cb0..83ac352c1e 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -573,15 +573,27 @@ static void microchip_icicle_kit_machine_init(MachineState *machine)
>                              TYPE_MICROCHIP_PFSOC);
>      qdev_realize(DEVICE(&s->soc), NULL, &error_fatal);
>  
> -    /* Split RAM into low and high regions using aliases to machine->ram */
> +    /*
> +     * The four CPU-visible windows alias the same physical DDR from offset
> +     * zero. For the Icicle Kit's 2 GiB of DDR, they map as follows:
> +     *
> +     * CPU address     Attribute           Visible size   DDR range
> +     * 0x0080000000    32-bit cached       1 GiB          [0, 1 GiB)
> +     * 0x00c0000000    32-bit non-cached   1 GiB          [0, 1 GiB)
> +     * 0x1000000000    64-bit cached       2 GiB          [0, 2 GiB)
> +     * 0x1400000000    64-bit non-cached   2 GiB          [0, 2 GiB)
> +     *
> +     * "Low" and "high" describe the CPU address windows, not the lower and
> +     * upper portions of physical DDR.
> +     */
>      mem_low_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> -    mem_high_size = machine->ram_size - mem_low_size;
> +    mem_high_size = machine->ram_size;
>      memory_region_init_alias(mem_low, NULL,
>                               "microchip.icicle.kit.ram_low", machine->ram,
>                               0, mem_low_size);
>      memory_region_init_alias(mem_high, NULL,
>                               "microchip.icicle.kit.ram_high", machine->ram,
> -                             mem_low_size, mem_high_size);
> +                             0, mem_high_size);
>  
>      /* Register RAM */
>      memory_region_add_subregion(system_memory,
> -- 
> 2.53.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]