* [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage
@ 2020-06-23 7:21 Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 1/3] hw/arm/aspeed: Remove extraneous MemoryRegion object owner Philippe Mathieu-Daudé
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:21 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Philippe Mathieu-Daudé,
qemu-arm, Cédric Le Goater, Joel Stanley
Yet another cleanup.
Simplify aspeed machine QOM usage.
Since v1, addressed Cédric review comments:
- Rename AspeedBoardState -> AspeedMachineState
Philippe Mathieu-Daudé (3):
hw/arm/aspeed: Remove extraneous MemoryRegion object owner
hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState
hw/arm/aspeed: QOM'ify AspeedMachineState
include/hw/arm/aspeed.h | 12 +++---------
hw/arm/aspeed.c | 33 ++++++++++++++++++---------------
2 files changed, 21 insertions(+), 24 deletions(-)
--
2.21.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] hw/arm/aspeed: Remove extraneous MemoryRegion object owner
2020-06-23 7:21 [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Philippe Mathieu-Daudé
@ 2020-06-23 7:21 ` Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 2/3] hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:21 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Philippe Mathieu-Daudé,
qemu-arm, Cédric Le Goater, Joel Stanley
I'm confused by this code, 'bmc' is created as:
bmc = g_new0(AspeedBoardState, 1);
Then we use it as QOM owner for different MemoryRegion objects.
But looking at memory_region_init_ram (similarly for ROM):
void memory_region_init_ram(MemoryRegion *mr,
struct Object *owner,
const char *name,
uint64_t size,
Error **errp)
{
DeviceState *owner_dev;
Error *err = NULL;
memory_region_init_ram_nomigrate(mr, owner, name, size, &err);
if (err) {
error_propagate(errp, err);
return;
}
/* This will assert if owner is neither NULL nor a DeviceState.
* We only want the owner here for the purposes of defining a
* unique name for migration. TODO: Ideally we should implement
* a naming scheme for Objects which are not DeviceStates, in
* which case we can relax this restriction.
*/
owner_dev = DEVICE(owner);
vmstate_register_ram(mr, owner_dev);
}
The expected assertion is not triggered ('bmc' is not NULL neither
a DeviceState).
'bmc' structure is defined as:
struct AspeedBoardState {
AspeedSoCState soc;
MemoryRegion ram_container;
MemoryRegion max_ram;
};
What happens is when using 'OBJECT(bmc)', the QOM macros cast the
memory pointed by bmc, which first member is 'soc', which is
initialized ...:
object_initialize_child(OBJECT(machine), "soc",
&bmc->soc, amc->soc_name);
The 'soc' object is indeed a DeviceState, so the assertion passes.
Since this is fragile and only happens to work by luck, remove the
dangerous OBJECT(bmc) owner argument.
Note, this probably breaks migration for this machine.
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/arm/aspeed.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 0ad08a2b4c..31765792a2 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -329,12 +329,12 @@ static void aspeed_machine_init(MachineState *machine)
* needed by the flash modules of the Aspeed machines.
*/
if (ASPEED_MACHINE(machine)->mmio_exec) {
- memory_region_init_alias(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+ memory_region_init_alias(boot_rom, NULL, "aspeed.boot_rom",
&fl->mmio, 0, fl->size);
memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
boot_rom);
} else {
- memory_region_init_rom(boot_rom, OBJECT(bmc), "aspeed.boot_rom",
+ memory_region_init_rom(boot_rom, NULL, "aspeed.boot_rom",
fl->size, &error_abort);
memory_region_add_subregion(get_system_memory(), FIRMWARE_ADDR,
boot_rom);
@@ -345,7 +345,7 @@ static void aspeed_machine_init(MachineState *machine)
if (machine->kernel_filename && sc->num_cpus > 1) {
/* With no u-boot we must set up a boot stub for the secondary CPU */
MemoryRegion *smpboot = g_new(MemoryRegion, 1);
- memory_region_init_ram(smpboot, OBJECT(bmc), "aspeed.smpboot",
+ memory_region_init_ram(smpboot, NULL, "aspeed.smpboot",
0x80, &error_abort);
memory_region_add_subregion(get_system_memory(),
AST_SMP_MAILBOX_BASE, smpboot);
--
2.21.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState
2020-06-23 7:21 [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 1/3] hw/arm/aspeed: Remove extraneous MemoryRegion object owner Philippe Mathieu-Daudé
@ 2020-06-23 7:21 ` Philippe Mathieu-Daudé
2020-06-23 8:12 ` Cédric Le Goater
2020-06-23 7:21 ` [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState Philippe Mathieu-Daudé
2020-06-25 13:18 ` [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Peter Maydell
3 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:21 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Philippe Mathieu-Daudé,
qemu-arm, Cédric Le Goater, Joel Stanley
To have a more consistent naming, rename AspeedBoardState
as AspeedMachineState.
Suggested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/arm/aspeed.h | 4 ++--
hw/arm/aspeed.c | 20 ++++++++++----------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 95b4daece8..5114ba0bd4 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -11,7 +11,7 @@
#include "hw/boards.h"
-typedef struct AspeedBoardState AspeedBoardState;
+typedef struct AspeedMachineState AspeedMachineState;
#define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed")
#define ASPEED_MACHINE(obj) \
@@ -45,7 +45,7 @@ typedef struct AspeedMachineClass {
const char *spi_model;
uint32_t num_cs;
uint32_t macs_mask;
- void (*i2c_init)(AspeedBoardState *bmc);
+ void (*i2c_init)(AspeedMachineState *bmc);
} AspeedMachineClass;
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 31765792a2..680345beca 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -32,7 +32,7 @@ static struct arm_boot_info aspeed_board_binfo = {
.board_id = -1, /* device-tree-only board */
};
-struct AspeedBoardState {
+struct AspeedMachineState {
AspeedSoCState soc;
MemoryRegion ram_container;
MemoryRegion max_ram;
@@ -253,7 +253,7 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
static void aspeed_machine_init(MachineState *machine)
{
- AspeedBoardState *bmc;
+ AspeedMachineState *bmc;
AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
AspeedSoCClass *sc;
DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
@@ -261,7 +261,7 @@ static void aspeed_machine_init(MachineState *machine)
int i;
NICInfo *nd = &nd_table[0];
- bmc = g_new0(AspeedBoardState, 1);
+ bmc = g_new0(AspeedMachineState, 1);
memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
4 * GiB);
@@ -374,7 +374,7 @@ static void aspeed_machine_init(MachineState *machine)
arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
}
-static void palmetto_bmc_i2c_init(AspeedBoardState *bmc)
+static void palmetto_bmc_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
DeviceState *dev;
@@ -396,7 +396,7 @@ static void palmetto_bmc_i2c_init(AspeedBoardState *bmc)
object_property_set_int(OBJECT(dev), 110000, "temperature3", &error_abort);
}
-static void ast2500_evb_i2c_init(AspeedBoardState *bmc)
+static void ast2500_evb_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
uint8_t *eeprom_buf = g_malloc0(8 * 1024);
@@ -413,13 +413,13 @@ static void ast2500_evb_i2c_init(AspeedBoardState *bmc)
i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
}
-static void ast2600_evb_i2c_init(AspeedBoardState *bmc)
+static void ast2600_evb_i2c_init(AspeedMachineState *bmc)
{
/* Start with some devices on our I2C busses */
ast2500_evb_i2c_init(bmc);
}
-static void romulus_bmc_i2c_init(AspeedBoardState *bmc)
+static void romulus_bmc_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
@@ -428,7 +428,7 @@ static void romulus_bmc_i2c_init(AspeedBoardState *bmc)
i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
}
-static void swift_bmc_i2c_init(AspeedBoardState *bmc)
+static void swift_bmc_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
@@ -457,7 +457,7 @@ static void swift_bmc_i2c_init(AspeedBoardState *bmc)
i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 12), "tmp105", 0x4a);
}
-static void sonorapass_bmc_i2c_init(AspeedBoardState *bmc)
+static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
@@ -501,7 +501,7 @@ static void sonorapass_bmc_i2c_init(AspeedBoardState *bmc)
}
-static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
+static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
{
AspeedSoCState *soc = &bmc->soc;
uint8_t *eeprom_buf = g_malloc0(8 * 1024);
--
2.21.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState
2020-06-23 7:21 [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 1/3] hw/arm/aspeed: Remove extraneous MemoryRegion object owner Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 2/3] hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState Philippe Mathieu-Daudé
@ 2020-06-23 7:21 ` Philippe Mathieu-Daudé
2020-06-23 8:12 ` Cédric Le Goater
2020-06-25 13:18 ` [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Peter Maydell
3 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-06-23 7:21 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Andrew Jeffery, Philippe Mathieu-Daudé,
qemu-arm, Cédric Le Goater, Joel Stanley
AspeedMachineState seems crippled. We use incorrectly 2
different structures to do the same thing. Merge them
altogether:
- Move AspeedMachine fields to AspeedMachineState
- AspeedMachineState is now QOM
- Remove unused AspeedMachine structure
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/arm/aspeed.h | 8 +-------
hw/arm/aspeed.c | 11 +++++++----
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
index 5114ba0bd4..09da9d9acc 100644
--- a/include/hw/arm/aspeed.h
+++ b/include/hw/arm/aspeed.h
@@ -15,13 +15,7 @@ typedef struct AspeedMachineState AspeedMachineState;
#define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed")
#define ASPEED_MACHINE(obj) \
- OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE)
-
-typedef struct AspeedMachine {
- MachineState parent_obj;
-
- bool mmio_exec;
-} AspeedMachine;
+ OBJECT_CHECK(AspeedMachineState, (obj), TYPE_ASPEED_MACHINE)
#define ASPEED_MAC0_ON (1 << 0)
#define ASPEED_MAC1_ON (1 << 1)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 680345beca..ccf127b328 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -33,9 +33,14 @@ static struct arm_boot_info aspeed_board_binfo = {
};
struct AspeedMachineState {
+ /* Private */
+ MachineState parent_obj;
+ /* Public */
+
AspeedSoCState soc;
MemoryRegion ram_container;
MemoryRegion max_ram;
+ bool mmio_exec;
};
/* Palmetto hardware value: 0x120CE416 */
@@ -253,7 +258,7 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
static void aspeed_machine_init(MachineState *machine)
{
- AspeedMachineState *bmc;
+ AspeedMachineState *bmc = ASPEED_MACHINE(machine);
AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
AspeedSoCClass *sc;
DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
@@ -261,8 +266,6 @@ static void aspeed_machine_init(MachineState *machine)
int i;
NICInfo *nd = &nd_table[0];
- bmc = g_new0(AspeedMachineState, 1);
-
memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
4 * GiB);
memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
@@ -751,7 +754,7 @@ static const TypeInfo aspeed_machine_types[] = {
}, {
.name = TYPE_ASPEED_MACHINE,
.parent = TYPE_MACHINE,
- .instance_size = sizeof(AspeedMachine),
+ .instance_size = sizeof(AspeedMachineState),
.instance_init = aspeed_machine_instance_init,
.class_size = sizeof(AspeedMachineClass),
.class_init = aspeed_machine_class_init,
--
2.21.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState
2020-06-23 7:21 ` [PATCH v2 2/3] hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState Philippe Mathieu-Daudé
@ 2020-06-23 8:12 ` Cédric Le Goater
0 siblings, 0 replies; 7+ messages in thread
From: Cédric Le Goater @ 2020-06-23 8:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Joel Stanley
On 6/23/20 9:21 AM, Philippe Mathieu-Daudé wrote:
> To have a more consistent naming, rename AspeedBoardState
> as AspeedMachineState.
>
> Suggested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/arm/aspeed.h | 4 ++--
> hw/arm/aspeed.c | 20 ++++++++++----------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 95b4daece8..5114ba0bd4 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -11,7 +11,7 @@
>
> #include "hw/boards.h"
>
> -typedef struct AspeedBoardState AspeedBoardState;
> +typedef struct AspeedMachineState AspeedMachineState;
>
> #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed")
> #define ASPEED_MACHINE(obj) \
> @@ -45,7 +45,7 @@ typedef struct AspeedMachineClass {
> const char *spi_model;
> uint32_t num_cs;
> uint32_t macs_mask;
> - void (*i2c_init)(AspeedBoardState *bmc);
> + void (*i2c_init)(AspeedMachineState *bmc);
> } AspeedMachineClass;
>
>
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 31765792a2..680345beca 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -32,7 +32,7 @@ static struct arm_boot_info aspeed_board_binfo = {
> .board_id = -1, /* device-tree-only board */
> };
>
> -struct AspeedBoardState {
> +struct AspeedMachineState {
> AspeedSoCState soc;
> MemoryRegion ram_container;
> MemoryRegion max_ram;
> @@ -253,7 +253,7 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
>
> static void aspeed_machine_init(MachineState *machine)
> {
> - AspeedBoardState *bmc;
> + AspeedMachineState *bmc;
> AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
> AspeedSoCClass *sc;
> DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
> @@ -261,7 +261,7 @@ static void aspeed_machine_init(MachineState *machine)
> int i;
> NICInfo *nd = &nd_table[0];
>
> - bmc = g_new0(AspeedBoardState, 1);
> + bmc = g_new0(AspeedMachineState, 1);
>
> memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
> 4 * GiB);
> @@ -374,7 +374,7 @@ static void aspeed_machine_init(MachineState *machine)
> arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo);
> }
>
> -static void palmetto_bmc_i2c_init(AspeedBoardState *bmc)
> +static void palmetto_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
> DeviceState *dev;
> @@ -396,7 +396,7 @@ static void palmetto_bmc_i2c_init(AspeedBoardState *bmc)
> object_property_set_int(OBJECT(dev), 110000, "temperature3", &error_abort);
> }
>
> -static void ast2500_evb_i2c_init(AspeedBoardState *bmc)
> +static void ast2500_evb_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
> uint8_t *eeprom_buf = g_malloc0(8 * 1024);
> @@ -413,13 +413,13 @@ static void ast2500_evb_i2c_init(AspeedBoardState *bmc)
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
> }
>
> -static void ast2600_evb_i2c_init(AspeedBoardState *bmc)
> +static void ast2600_evb_i2c_init(AspeedMachineState *bmc)
> {
> /* Start with some devices on our I2C busses */
> ast2500_evb_i2c_init(bmc);
> }
>
> -static void romulus_bmc_i2c_init(AspeedBoardState *bmc)
> +static void romulus_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
>
> @@ -428,7 +428,7 @@ static void romulus_bmc_i2c_init(AspeedBoardState *bmc)
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 11), "ds1338", 0x32);
> }
>
> -static void swift_bmc_i2c_init(AspeedBoardState *bmc)
> +static void swift_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
>
> @@ -457,7 +457,7 @@ static void swift_bmc_i2c_init(AspeedBoardState *bmc)
> i2c_create_slave(aspeed_i2c_get_bus(DEVICE(&soc->i2c), 12), "tmp105", 0x4a);
> }
>
> -static void sonorapass_bmc_i2c_init(AspeedBoardState *bmc)
> +static void sonorapass_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
>
> @@ -501,7 +501,7 @@ static void sonorapass_bmc_i2c_init(AspeedBoardState *bmc)
>
> }
>
> -static void witherspoon_bmc_i2c_init(AspeedBoardState *bmc)
> +static void witherspoon_bmc_i2c_init(AspeedMachineState *bmc)
> {
> AspeedSoCState *soc = &bmc->soc;
> uint8_t *eeprom_buf = g_malloc0(8 * 1024);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState
2020-06-23 7:21 ` [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState Philippe Mathieu-Daudé
@ 2020-06-23 8:12 ` Cédric Le Goater
0 siblings, 0 replies; 7+ messages in thread
From: Cédric Le Goater @ 2020-06-23 8:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, Joel Stanley
On 6/23/20 9:21 AM, Philippe Mathieu-Daudé wrote:
> AspeedMachineState seems crippled. We use incorrectly 2
> different structures to do the same thing. Merge them
> altogether:
> - Move AspeedMachine fields to AspeedMachineState
> - AspeedMachineState is now QOM
> - Remove unused AspeedMachine structure
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
> include/hw/arm/aspeed.h | 8 +-------
> hw/arm/aspeed.c | 11 +++++++----
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/arm/aspeed.h b/include/hw/arm/aspeed.h
> index 5114ba0bd4..09da9d9acc 100644
> --- a/include/hw/arm/aspeed.h
> +++ b/include/hw/arm/aspeed.h
> @@ -15,13 +15,7 @@ typedef struct AspeedMachineState AspeedMachineState;
>
> #define TYPE_ASPEED_MACHINE MACHINE_TYPE_NAME("aspeed")
> #define ASPEED_MACHINE(obj) \
> - OBJECT_CHECK(AspeedMachine, (obj), TYPE_ASPEED_MACHINE)
> -
> -typedef struct AspeedMachine {
> - MachineState parent_obj;
> -
> - bool mmio_exec;
> -} AspeedMachine;
> + OBJECT_CHECK(AspeedMachineState, (obj), TYPE_ASPEED_MACHINE)
>
> #define ASPEED_MAC0_ON (1 << 0)
> #define ASPEED_MAC1_ON (1 << 1)
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 680345beca..ccf127b328 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -33,9 +33,14 @@ static struct arm_boot_info aspeed_board_binfo = {
> };
>
> struct AspeedMachineState {
> + /* Private */
> + MachineState parent_obj;
> + /* Public */
> +
> AspeedSoCState soc;
> MemoryRegion ram_container;
> MemoryRegion max_ram;
> + bool mmio_exec;
> };
>
> /* Palmetto hardware value: 0x120CE416 */
> @@ -253,7 +258,7 @@ static void sdhci_attach_drive(SDHCIState *sdhci, DriveInfo *dinfo)
>
> static void aspeed_machine_init(MachineState *machine)
> {
> - AspeedMachineState *bmc;
> + AspeedMachineState *bmc = ASPEED_MACHINE(machine);
> AspeedMachineClass *amc = ASPEED_MACHINE_GET_CLASS(machine);
> AspeedSoCClass *sc;
> DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
> @@ -261,8 +266,6 @@ static void aspeed_machine_init(MachineState *machine)
> int i;
> NICInfo *nd = &nd_table[0];
>
> - bmc = g_new0(AspeedMachineState, 1);
> -
> memory_region_init(&bmc->ram_container, NULL, "aspeed-ram-container",
> 4 * GiB);
> memory_region_add_subregion(&bmc->ram_container, 0, machine->ram);
> @@ -751,7 +754,7 @@ static const TypeInfo aspeed_machine_types[] = {
> }, {
> .name = TYPE_ASPEED_MACHINE,
> .parent = TYPE_MACHINE,
> - .instance_size = sizeof(AspeedMachine),
> + .instance_size = sizeof(AspeedMachineState),
> .instance_init = aspeed_machine_instance_init,
> .class_size = sizeof(AspeedMachineClass),
> .class_init = aspeed_machine_class_init,
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage
2020-06-23 7:21 [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-06-23 7:21 ` [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState Philippe Mathieu-Daudé
@ 2020-06-25 13:18 ` Peter Maydell
3 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2020-06-25 13:18 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Andrew Jeffery, qemu-arm, Joel Stanley, QEMU Developers,
Cédric Le Goater
On Tue, 23 Jun 2020 at 08:21, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Yet another cleanup.
> Simplify aspeed machine QOM usage.
>
> Since v1, addressed Cédric review comments:
> - Rename AspeedBoardState -> AspeedMachineState
>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-25 13:19 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-23 7:21 [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 1/3] hw/arm/aspeed: Remove extraneous MemoryRegion object owner Philippe Mathieu-Daudé
2020-06-23 7:21 ` [PATCH v2 2/3] hw/arm/aspeed: Rename AspeedBoardState as AspeedMachineState Philippe Mathieu-Daudé
2020-06-23 8:12 ` Cédric Le Goater
2020-06-23 7:21 ` [PATCH v2 3/3] hw/arm/aspeed: QOM'ify AspeedMachineState Philippe Mathieu-Daudé
2020-06-23 8:12 ` Cédric Le Goater
2020-06-25 13:18 ` [PATCH v2 0/3] hw/arm/aspeed: Improve QOM usage Peter Maydell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).