On 08/16/2018 02:48 PM, Peter Maydell wrote: > On 7 August 2018 at 08:57, Joel Stanley <[email protected]> wrote: >> From: Cédric Le Goater <[email protected]> >> >> This will be used to construct a memory region beyond the RAM region >> to let firmwares scan the address space with load/store to guess how >> much RAM the SoC has. >> >> Signed-off-by: Cédric Le Goater <[email protected]> >> Signed-off-by: Joel Stanley <[email protected]> >> --- >> hw/arm/aspeed.c | 31 +++++++++++++++++++++++++++++++ >> hw/arm/aspeed_soc.c | 2 ++ >> hw/misc/aspeed_sdmc.c | 3 +++ >> include/hw/misc/aspeed_sdmc.h | 1 + >> 4 files changed, 37 insertions(+) >> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c >> index bb9d33848d3f..e078269266bc 100644 >> --- a/hw/arm/aspeed.c >> +++ b/hw/arm/aspeed.c >> @@ -31,6 +31,7 @@ static struct arm_boot_info aspeed_board_binfo = { >> typedef struct AspeedBoardState { >> AspeedSoCState soc; >> MemoryRegion ram; >> + MemoryRegion max_ram; >> } AspeedBoardState; >> >> typedef struct AspeedBoardConfig { >> @@ -127,6 +128,27 @@ static const AspeedBoardConfig aspeed_boards[] = { >> }, >> }; >> >> +/* >> + * The max ram region is for firmwares that scan the address space >> + * with load/store to guess how much RAM the SoC has. >> + */ >> +static uint64_t max_ram_read(void *opaque, hwaddr offset, unsigned size) >> +{ >> + return 0; >> +} >> + >> +static void max_ram_write(void *opaque, hwaddr offset, uint64_t value, >> + unsigned size) >> +{ >> + /* Disacard writes */ >> +} >> + >> +static const MemoryRegionOps max_ram_ops = { >> + .read = max_ram_read, >> + .write = max_ram_write, >> + .endianness = DEVICE_NATIVE_ENDIAN, >> +}; >> + >> #define FIRMWARE_ADDR 0x0 >> >> static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, >> @@ -187,6 +209,7 @@ static void aspeed_board_init(MachineState *machine, >> AspeedBoardState *bmc; >> AspeedSoCClass *sc; >> DriveInfo *drive0 = drive_get(IF_MTD, 0, 0); >> + ram_addr_t max_ram_size; >> >> bmc = g_new0(AspeedBoardState, 1); >> object_initialize(&bmc->soc, (sizeof(bmc->soc)), cfg->soc_name); >> @@ -226,6 +249,14 @@ static void aspeed_board_init(MachineState *machine, >> object_property_add_const_link(OBJECT(&bmc->soc), "ram", >> OBJECT(&bmc->ram), >> &error_abort); >> >> + max_ram_size = object_property_get_uint(OBJECT(&bmc->soc), >> "max-ram-size", >> + &error_abort); >> + memory_region_init_io(&bmc->max_ram, NULL, &max_ram_ops, NULL, >> + "max_ram", max_ram_size - ram_size); >> + memory_region_add_subregion(get_system_memory(), >> + sc->info->sdram_base + ram_size, >> + &bmc->max_ram); > > I'm surprised that you need the IO ops, ie that it doesn't work > just to define an empty container region with memory_region_init().
Initially, there was some logging in the IO ops, which was a nice to have. But that was dropped in this patch. No a big issue I think. Thanks, C.
