Re: [PATCH v3 3/7] hw/riscv/microchip_pfsoc.c: add an Icicle Kit fdt address function

[Alistair Francis]

On Sun, Jan 22, 2023 at 5:16 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Conor,
>
> Thanks for the Icicle-kit walk-through! I'll not claim that I fully 
> understood it,
> but I understood enough to handle the situation ATM.
>
> Without this change, this is where the FDT is being installed in the board 
> when
> I start it with 8Gb of RAM (retrieved via 'info roms'):
>
> addr=00000000bfe00000 size=0x00a720 mem=ram name="fdt"
>
> Which surprised me at first because this is almost at the end of the LO area 
> which has
> 1Gb and I figured it would be in the middle of another RAM area. I took 
> another read
> at what we're doing in riscv_load_fdt():
>
> -----------
> temp = (dram_base < 3072 * MiB) ?  MIN(dram_end, 3072 * MiB) : dram_end;
> fdt_addr = QEMU_ALIGN_DOWN(temp - fdtsize, 2 * MiB);
> -----------
>
> This code can be read as "if the starting address of the RAM is lower than 
> 3Gb, put
> the FDT no further than 3Gb (0xc0000000). Otherwise, put it at the end of 
> dram",
> where "dram_base" is the starting address of the RAM block that the function
> receives.
>
> For icicle-kit, this is being passed as  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
> 0x80000000, which is 2Gb.
>
> So, regardless of how much RAM we have (dram_end), the FDT will always be 
> capped at
> 3Gb. At this moment, this fits exactly at the end of the LO area for the 
> Icicle Kit.
> Which is funny because this 3Gb restriction was added by commit 1a475d39ef54 
> to fix
> 32 bit guest boot and it happened to also work for the Microchip SoC.
>
> So yeah, I thought that I was fixing a bug and in the end I caused one. This 
> patch
> needs to go.
>
>
> Alistair, I believe I should re-send v2, this time explaining why the 
> existing function
> will not break the Microchip board because we'll never put the FDT out of the 
> LO area
> of the board. Does this work for you?

I think that's fine. My only worry is that we are losing some
flexibility that some future board might want.

Alistair