1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH 0/7] arm: aspeed: Extend SDRAM controller
  4. View patch

[Qemu-devel] [PATCH 6/7] aspeed: add a max_ram_size property to the memory controller

Joel Stanley posted 7 patches 7 years, 2 months ago
[Qemu-devel] [PATCH 6/7] aspeed: add a max_ram_size property to the memory controller
Posted by Joel Stanley 7 years, 2 months ago 
From: Cédric Le Goater <clg@kaod.org>

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 <clg@kaod.org>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 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);
+
     aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort);
     aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort);
 
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index e68911af0f90..a27233d4876b 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -155,6 +155,8 @@ static void aspeed_soc_init(Object *obj)
                          sc->info->silicon_rev);
     object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
                               "ram-size", &error_abort);
+    object_property_add_alias(obj, "max-ram-size", OBJECT(&s->sdmc),
+                              "max-ram-size", &error_abort);
 
     for (i = 0; i < sc->info->wdts_num; i++) {
         object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index 89de3138aff0..eec77f243508 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -242,12 +242,14 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error **errp)
     case AST2400_A0_SILICON_REV:
     case AST2400_A1_SILICON_REV:
         s->ram_bits = ast2400_rambits(s);
+        s->max_ram_size = 512 << 20;
         s->fixed_conf = ASPEED_SDMC_VGA_COMPAT |
             ASPEED_SDMC_DRAM_SIZE(s->ram_bits);
         break;
     case AST2500_A0_SILICON_REV:
     case AST2500_A1_SILICON_REV:
         s->ram_bits = ast2500_rambits(s);
+        s->max_ram_size = 1024 << 20;
         s->fixed_conf = ASPEED_SDMC_HW_VERSION(1) |
             ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
             ASPEED_SDMC_CACHE_INITIAL_DONE |
@@ -275,6 +277,7 @@ static const VMStateDescription vmstate_aspeed_sdmc = {
 static Property aspeed_sdmc_properties[] = {
     DEFINE_PROP_UINT32("silicon-rev", AspeedSDMCState, silicon_rev, 0),
     DEFINE_PROP_UINT64("ram-size", AspeedSDMCState, ram_size, 0),
+    DEFINE_PROP_UINT64("max-ram-size", AspeedSDMCState, max_ram_size, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h
index e079c66a7d73..b3c926acae90 100644
--- a/include/hw/misc/aspeed_sdmc.h
+++ b/include/hw/misc/aspeed_sdmc.h
@@ -27,6 +27,7 @@ typedef struct AspeedSDMCState {
     uint32_t silicon_rev;
     uint32_t ram_bits;
     uint64_t ram_size;
+    uint64_t max_ram_size;
     uint32_t fixed_conf;
 
 } AspeedSDMCState;
-- 
2.17.1
Re: [Qemu-devel] [PATCH 6/7] aspeed: add a max_ram_size property to the memory controller
Posted by Cédric Le Goater 7 years, 2 months ago 
On 08/07/2018 09:57 AM, Joel Stanley wrote:
> From: Cédric Le Goater <clg@kaod.org>
> 
> 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.

This is in another patch I can send later on.

Thanks,

C. 

> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  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);
> +
>      aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort);
>      aspeed_board_init_flashes(&bmc->soc.spi[0], cfg->spi_model, &error_abort);
>  
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index e68911af0f90..a27233d4876b 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -155,6 +155,8 @@ static void aspeed_soc_init(Object *obj)
>                           sc->info->silicon_rev);
>      object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
>                                "ram-size", &error_abort);
> +    object_property_add_alias(obj, "max-ram-size", OBJECT(&s->sdmc),
> +                              "max-ram-size", &error_abort);
>  
>      for (i = 0; i < sc->info->wdts_num; i++) {
>          object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
> diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
> index 89de3138aff0..eec77f243508 100644
> --- a/hw/misc/aspeed_sdmc.c
> +++ b/hw/misc/aspeed_sdmc.c
> @@ -242,12 +242,14 @@ static void aspeed_sdmc_realize(DeviceState *dev, Error **errp)
>      case AST2400_A0_SILICON_REV:
>      case AST2400_A1_SILICON_REV:
>          s->ram_bits = ast2400_rambits(s);
> +        s->max_ram_size = 512 << 20;
>          s->fixed_conf = ASPEED_SDMC_VGA_COMPAT |
>              ASPEED_SDMC_DRAM_SIZE(s->ram_bits);
>          break;
>      case AST2500_A0_SILICON_REV:
>      case AST2500_A1_SILICON_REV:
>          s->ram_bits = ast2500_rambits(s);
> +        s->max_ram_size = 1024 << 20;
>          s->fixed_conf = ASPEED_SDMC_HW_VERSION(1) |
>              ASPEED_SDMC_VGA_APERTURE(ASPEED_SDMC_VGA_64MB) |
>              ASPEED_SDMC_CACHE_INITIAL_DONE |
> @@ -275,6 +277,7 @@ static const VMStateDescription vmstate_aspeed_sdmc = {
>  static Property aspeed_sdmc_properties[] = {
>      DEFINE_PROP_UINT32("silicon-rev", AspeedSDMCState, silicon_rev, 0),
>      DEFINE_PROP_UINT64("ram-size", AspeedSDMCState, ram_size, 0),
> +    DEFINE_PROP_UINT64("max-ram-size", AspeedSDMCState, max_ram_size, 0),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/misc/aspeed_sdmc.h b/include/hw/misc/aspeed_sdmc.h
> index e079c66a7d73..b3c926acae90 100644
> --- a/include/hw/misc/aspeed_sdmc.h
> +++ b/include/hw/misc/aspeed_sdmc.h
> @@ -27,6 +27,7 @@ typedef struct AspeedSDMCState {
>      uint32_t silicon_rev;
>      uint32_t ram_bits;
>      uint64_t ram_size;
> +    uint64_t max_ram_size;
>      uint32_t fixed_conf;
>  
>  } AspeedSDMCState;
>
Re: [Qemu-devel] [Qemu-arm] [PATCH 6/7] aspeed: add a max_ram_size property to the memory controller
Posted by Cédric Le Goater 7 years, 2 months ago 
On 08/07/2018 12:29 PM, Cédric Le Goater wrote:
> On 08/07/2018 09:57 AM, Joel Stanley wrote:
>> From: Cédric Le Goater <clg@kaod.org>
>>
>> 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.
> 
> This is in another patch I can send later on.

I now see that you have merged two patches. This is fine then.


Thanks,

C.
Re: [Qemu-devel] [PATCH 6/7] aspeed: add a max_ram_size property to the memory controller
Posted by Peter Maydell 7 years, 2 months ago 
On 7 August 2018 at 08:57, Joel Stanley <joel@jms.id.au> wrote:
> From: Cédric Le Goater <clg@kaod.org>
>
> 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 <clg@kaod.org>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  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().

thanks
-- PMM
Re: [Qemu-devel] [PATCH 6/7] aspeed: add a max_ram_size property to the memory controller
Posted by Cédric Le Goater 7 years, 2 months ago 
On 08/16/2018 02:48 PM, Peter Maydell wrote:
> On 7 August 2018 at 08:57, Joel Stanley <joel@jms.id.au> wrote:
>> From: Cédric Le Goater <clg@kaod.org>
>>
>> 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 <clg@kaod.org>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>  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.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.