> On Jun 23, 2022, at 8:39 AM, Cédric Le Goater <clg@xxxxxxxx> wrote:
>
> On 6/23/22 14:57, Peter Maydell wrote:
>> On Thu, 23 Jun 2022 at 13:37, Peter Delevoryas <pdel@xxxxxx> wrote:
>>>
>>> Note: sysbus_mmio_map(), sysbus_mmio_map_overlap(), and others are still
>>> using get_system_memory indirectly.
>>>
>>> Signed-off-by: Peter Delevoryas <pdel@xxxxxx>
>>> ---
>>> hw/arm/aspeed.c | 8 ++++----
>>> hw/arm/aspeed_ast10x0.c | 5 ++---
>>> hw/arm/aspeed_ast2600.c | 2 +-
>>> hw/arm/aspeed_soc.c | 6 +++---
>>> 4 files changed, 10 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
>>> index 8dae155183..3aa74e88fb 100644
>>> --- a/hw/arm/aspeed.c
>>> +++ b/hw/arm/aspeed.c
>>> @@ -371,7 +371,7 @@ static void aspeed_machine_init(MachineState *machine)
>>> amc->uart_default);
>>> qdev_realize(DEVICE(&bmc->soc), NULL, &error_abort);
>>>
>>> - memory_region_add_subregion(get_system_memory(),
>>> + memory_region_add_subregion(bmc->soc.system_memory,
>>> sc->memmap[ASPEED_DEV_SDRAM],
>>> &bmc->ram_container);
>> This is board code, it shouldn't be reaching into the internals
>> of the SoC object like this. The board code probably already
>> has the right MemoryRegion because it was the one that passed
>> it to the SoC link porperty in the first place.
>
> It's a bit messy currently. May be I got it wrong initially. The
> board allocates a ram container region in which the machine ram
> region is mapped. See commit ad1a9782186d ("aspeed: add a RAM memory
> region container")
>
> There is an extra region after ram in the ram container to catch
> invalid access done by FW. That's how FW determines the size of
> ram. See commit ebe31c0a8ef7 ("aspeed: add a max_ram_size property
> to the memory controller")
>
> But I think I can safely move all the RAM logic under the board.
> I will send a patch.
Ah yes, I noticed that this was odd too, and that the DRAM probably
should have been mapped inside the SoC code. I didn’t know exactly
how to solve it easily though. Thanks for sending a patch Cedric!!
>
> Thanks,
>
> C.
>
>> thanks
>> -- PMM
>
