* [PATCH v2] aspeed: Set the dram container at the SoC level
@ 2022-06-23 20:21 Cédric Le Goater
2022-06-23 21:36 ` Peter Delevoryas
0 siblings, 1 reply; 2+ messages in thread
From: Cédric Le Goater @ 2022-06-23 20:21 UTC (permalink / raw)
To: qemu-arm, qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Joel Stanley, Peter Delevoryas,
Cédric Le Goater
Currently, the Aspeed machines allocate a ram container region in
which the machine ram region is mapped. See commit ad1a9782186d
("aspeed: add a RAM memory region container"). An extra region is
mapped 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").
Let's move all the logic under the SoC where it should be. It will
also ease the work on multi SoC support.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes in v2:
- handle errors
include/hw/arm/aspeed_soc.h | 2 ++
hw/arm/aspeed.c | 39 ++---------------------------------
hw/arm/aspeed_ast2600.c | 6 ++++--
hw/arm/aspeed_soc.c | 41 +++++++++++++++++++++++++++++++++++--
4 files changed, 47 insertions(+), 41 deletions(-)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 02a5a9ffcbd3..e8a104823d35 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -50,6 +50,7 @@ struct AspeedSoCState {
A15MPPrivState a7mpcore;
ARMv7MState armv7m;
MemoryRegion *dram_mr;
+ MemoryRegion dram_container;
MemoryRegion sram;
AspeedVICState vic;
AspeedRtcState rtc;
@@ -165,5 +166,6 @@ enum {
qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
void aspeed_soc_uart_init(AspeedSoCState *s);
+bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
#endif /* ASPEED_SOC_H */
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index a06f7c1b62a9..dc09773b0ba5 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -174,27 +174,6 @@ struct AspeedMachineState {
#define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
#define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2
-/*
- * 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)
-{
- /* Discard writes */
-}
-
-static const MemoryRegionOps max_ram_ops = {
- .read = max_ram_read,
- .write = max_ram_write,
- .endianness = DEVICE_NATIVE_ENDIAN,
-};
-
#define AST_SMP_MAILBOX_BASE 0x1e6e2180
#define AST_SMP_MBOX_FIELD_ENTRY (AST_SMP_MAILBOX_BASE + 0x0)
#define AST_SMP_MBOX_FIELD_GOSIGN (AST_SMP_MAILBOX_BASE + 0x4)
@@ -324,20 +303,16 @@ static void aspeed_machine_init(MachineState *machine)
AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
AspeedSoCClass *sc;
DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
- ram_addr_t max_ram_size;
int i;
NICInfo *nd = &nd_table[0];
- memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
- 4 * GiB);
- memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
-
object_initialize_child(OBJECT(machine), "soc", &bmc->soc, amc->soc_name);
sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
/*
- * This will error out if isize is not supported by memory controller.
+ * This will error out if the RAM size is not supported by the
+ * memory controller of the SoC.
*/
object_property_set_uint(OBJECT(&bmc->soc), "ram-size", machine->ram_size,
&error_fatal);
@@ -369,16 +344,6 @@ 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(),
- sc->memmap[ASPEED_DEV_SDRAM],
- &bmc->ram_container);
-
- 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 - machine->ram_size);
- memory_region_add_subregion(&bmc->ram_container, machine->ram_size, &bmc->max_ram);
-
aspeed_board_init_flashes(&bmc->soc.fmc,
bmc->fmc_model ? bmc->fmc_model : amc->fmc_model,
amc->num_cs, 0);
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index b0a4199b6960..f70b17d3f9cf 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -197,8 +197,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
object_initialize_child(obj, "sdmc", &s->sdmc, typename);
object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
"ram-size");
- object_property_add_alias(obj, "max-ram-size", OBJECT(&s->sdmc),
- "max-ram-size");
for (i = 0; i < sc->wdts_num; i++) {
snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
@@ -271,6 +269,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
/* IO space */
create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
ASPEED_SOC_IOMEM_SIZE);
+ /* RAM */
+ if (!aspeed_soc_dram_init(s, errp)) {
+ return;
+ }
/* Video engine stub */
create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 30574d4276ab..f5300288745b 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -11,6 +11,7 @@
*/
#include "qemu/osdep.h"
+#include "qemu/units.h"
#include "qapi/error.h"
#include "hw/misc/unimp.h"
#include "hw/arm/aspeed_soc.h"
@@ -191,8 +192,6 @@ static void aspeed_soc_init(Object *obj)
object_initialize_child(obj, "sdmc", &s->sdmc, typename);
object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
"ram-size");
- object_property_add_alias(obj, "max-ram-size", OBJECT(&s->sdmc),
- "max-ram-size");
for (i = 0; i < sc->wdts_num; i++) {
snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
@@ -237,6 +236,11 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
ASPEED_SOC_IOMEM_SIZE);
+ /* RAM */
+ if (!aspeed_soc_dram_init(s, errp)) {
+ return;
+ }
+
/* Video engine stub */
create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
0x1000);
@@ -561,3 +565,36 @@ void aspeed_soc_uart_init(AspeedSoCState *s)
serial_hd(i), DEVICE_LITTLE_ENDIAN);
}
}
+
+bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp)
+{
+ AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+ ram_addr_t ram_size, max_ram_size;
+ DeviceState *dev;
+
+ memory_region_init(&s->dram_container, OBJECT(s), "ram-container", 4 * GiB);
+ memory_region_add_subregion(&s->dram_container, 0, s->dram_mr);
+
+ /*
+ * Add a memory region beyond the RAM region to let firmwares scan
+ * the address space with load/store and guess how much RAM the
+ * SoC has.
+ */
+ ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
+ &error_abort);
+ max_ram_size = object_property_get_uint(OBJECT(&s->sdmc), "max-ram-size",
+ &error_abort);
+
+ dev = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
+ qdev_prop_set_string(dev, "name", "ram-empty");
+ qdev_prop_set_uint64(dev, "size", max_ram_size - ram_size);
+ if (!sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp)) {
+ return false;
+ }
+ memory_region_add_subregion_overlap(&s->dram_container, ram_size,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0), -1000);
+
+ memory_region_add_subregion(get_system_memory(),
+ sc->memmap[ASPEED_DEV_SDRAM], &s->dram_container);
+ return true;
+}
--
2.35.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] aspeed: Set the dram container at the SoC level
2022-06-23 20:21 [PATCH v2] aspeed: Set the dram container at the SoC level Cédric Le Goater
@ 2022-06-23 21:36 ` Peter Delevoryas
0 siblings, 0 replies; 2+ messages in thread
From: Peter Delevoryas @ 2022-06-23 21:36 UTC (permalink / raw)
Cc: Peter Delevoryas, qemu-arm, Cameron Esfahani via, Peter Maydell,
Andrew Jeffery, Joel Stanley, Cédric Le Goater
> On Jun 23, 2022, at 1:21 PM, Cédric Le Goater <clg@kaod.org> wrote:
>
> Currently, the Aspeed machines allocate a ram container region in
> which the machine ram region is mapped. See commit ad1a9782186d
> ("aspeed: add a RAM memory region container"). An extra region is
> mapped 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").
>
> Let's move all the logic under the SoC where it should be. It will
> also ease the work on multi SoC support.
Looks good!
Reviewed-by: Peter Delevoryas <pdel@fb.com>
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Changes in v2:
>
> - handle errors
>
> include/hw/arm/aspeed_soc.h | 2 ++
> hw/arm/aspeed.c | 39 ++---------------------------------
> hw/arm/aspeed_ast2600.c | 6 ++++--
> hw/arm/aspeed_soc.c | 41 +++++++++++++++++++++++++++++++++++--
> 4 files changed, 47 insertions(+), 41 deletions(-)
>
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 02a5a9ffcbd3..e8a104823d35 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -50,6 +50,7 @@ struct AspeedSoCState {
> A15MPPrivState a7mpcore;
> ARMv7MState armv7m;
> MemoryRegion *dram_mr;
> + MemoryRegion dram_container;
> MemoryRegion sram;
> AspeedVICState vic;
> AspeedRtcState rtc;
> @@ -165,5 +166,6 @@ enum {
>
> qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int dev);
> void aspeed_soc_uart_init(AspeedSoCState *s);
> +bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp);
>
> #endif /* ASPEED_SOC_H */
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index a06f7c1b62a9..dc09773b0ba5 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -174,27 +174,6 @@ struct AspeedMachineState {
> #define BLETCHLEY_BMC_HW_STRAP1 AST2600_EVB_HW_STRAP1
> #define BLETCHLEY_BMC_HW_STRAP2 AST2600_EVB_HW_STRAP2
>
> -/*
> - * 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)
> -{
> - /* Discard writes */
> -}
> -
> -static const MemoryRegionOps max_ram_ops = {
> - .read = max_ram_read,
> - .write = max_ram_write,
> - .endianness = DEVICE_NATIVE_ENDIAN,
> -};
> -
> #define AST_SMP_MAILBOX_BASE 0x1e6e2180
> #define AST_SMP_MBOX_FIELD_ENTRY (AST_SMP_MAILBOX_BASE + 0x0)
> #define AST_SMP_MBOX_FIELD_GOSIGN (AST_SMP_MAILBOX_BASE + 0x4)
> @@ -324,20 +303,16 @@ static void aspeed_machine_init(MachineState *machine)
> AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
> AspeedSoCClass *sc;
> DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
> - ram_addr_t max_ram_size;
> int i;
> NICInfo *nd = &nd_table[0];
>
> - memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
> - 4 * GiB);
> - memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
> -
> object_initialize_child(OBJECT(machine), "soc", &bmc->soc, amc->soc_name);
>
> sc = ASPEED_SOC_GET_CLASS(&bmc->soc);
>
> /*
> - * This will error out if isize is not supported by memory controller.
> + * This will error out if the RAM size is not supported by the
> + * memory controller of the SoC.
> */
> object_property_set_uint(OBJECT(&bmc->soc), "ram-size", machine->ram_size,
> &error_fatal);
> @@ -369,16 +344,6 @@ 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(),
> - sc->memmap[ASPEED_DEV_SDRAM],
> - &bmc->ram_container);
> -
> - 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 - machine->ram_size);
> - memory_region_add_subregion(&bmc->ram_container, machine->ram_size, &bmc->max_ram);
> -
> aspeed_board_init_flashes(&bmc->soc.fmc,
> bmc->fmc_model ? bmc->fmc_model : amc->fmc_model,
> amc->num_cs, 0);
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index b0a4199b6960..f70b17d3f9cf 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -197,8 +197,6 @@ static void aspeed_soc_ast2600_init(Object *obj)
> object_initialize_child(obj, "sdmc", &s->sdmc, typename);
> object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
> "ram-size");
> - object_property_add_alias(obj, "max-ram-size", OBJECT(&s->sdmc),
> - "max-ram-size");
>
> for (i = 0; i < sc->wdts_num; i++) {
> snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
> @@ -271,6 +269,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
> /* IO space */
> create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
> ASPEED_SOC_IOMEM_SIZE);
> + /* RAM */
> + if (!aspeed_soc_dram_init(s, errp)) {
> + return;
> + }
>
> /* Video engine stub */
> create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 30574d4276ab..f5300288745b 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -11,6 +11,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "qemu/units.h"
> #include "qapi/error.h"
> #include "hw/misc/unimp.h"
> #include "hw/arm/aspeed_soc.h"
> @@ -191,8 +192,6 @@ static void aspeed_soc_init(Object *obj)
> object_initialize_child(obj, "sdmc", &s->sdmc, typename);
> object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
> "ram-size");
> - object_property_add_alias(obj, "max-ram-size", OBJECT(&s->sdmc),
> - "max-ram-size");
>
> for (i = 0; i < sc->wdts_num; i++) {
> snprintf(typename, sizeof(typename), "aspeed.wdt-%s", socname);
> @@ -237,6 +236,11 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_DEV_IOMEM],
> ASPEED_SOC_IOMEM_SIZE);
>
> + /* RAM */
> + if (!aspeed_soc_dram_init(s, errp)) {
> + return;
> + }
> +
> /* Video engine stub */
> create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_DEV_VIDEO],
> 0x1000);
> @@ -561,3 +565,36 @@ void aspeed_soc_uart_init(AspeedSoCState *s)
> serial_hd(i), DEVICE_LITTLE_ENDIAN);
> }
> }
> +
> +bool aspeed_soc_dram_init(AspeedSoCState *s, Error **errp)
> +{
> + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> + ram_addr_t ram_size, max_ram_size;
> + DeviceState *dev;
> +
> + memory_region_init(&s->dram_container, OBJECT(s), "ram-container", 4 * GiB);
> + memory_region_add_subregion(&s->dram_container, 0, s->dram_mr);
> +
> + /*
> + * Add a memory region beyond the RAM region to let firmwares scan
> + * the address space with load/store and guess how much RAM the
> + * SoC has.
> + */
> + ram_size = object_property_get_uint(OBJECT(&s->sdmc), "ram-size",
> + &error_abort);
> + max_ram_size = object_property_get_uint(OBJECT(&s->sdmc), "max-ram-size",
> + &error_abort);
> +
> + dev = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
> + qdev_prop_set_string(dev, "name", "ram-empty");
> + qdev_prop_set_uint64(dev, "size", max_ram_size - ram_size);
> + if (!sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), errp)) {
> + return false;
> + }
> + memory_region_add_subregion_overlap(&s->dram_container, ram_size,
> + sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0), -1000);
> +
> + memory_region_add_subregion(get_system_memory(),
> + sc->memmap[ASPEED_DEV_SDRAM], &s->dram_container);
> + return true;
> +}
> --
> 2.35.3
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-06-23 21:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-23 20:21 [PATCH v2] aspeed: Set the dram container at the SoC level Cédric Le Goater
2022-06-23 21:36 ` Peter Delevoryas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.