* [PATCH] arm/aspeed: Compute the number of CPUs from the SoC definition
@ 2020-05-19 9:16 Cédric Le Goater
2020-05-20 6:27 ` Markus Armbruster
0 siblings, 1 reply; 3+ messages in thread
From: Cédric Le Goater @ 2020-05-19 9:16 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, qemu-devel, Markus Armbruster, qemu-arm,
Joel Stanley, Cédric Le Goater
Commit ece09beec457 ("aspeed: introduce a configurable number of CPU
per machine") was a convient change during bringup but the Aspeed SoCs
have a fixed number of CPUs : one for the AST2400 and AST2500, and two
for the AST2600.
Remove the "num-cpu" property from the SoC state and use the fixed
number of CPUs defined in the SoC class instead. Compute the default,
min, max number of CPUs of the machine directly from the SoC class
definition.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
include/hw/arm/aspeed_soc.h | 1 -
hw/arm/aspeed.c | 29 ++++++++++++++++++++++++-----
hw/arm/aspeed_ast2600.c | 20 +++++++-------------
hw/arm/aspeed_soc.c | 9 +--------
4 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 78b9f6ae532f..914115f3ef77 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -40,7 +40,6 @@ typedef struct AspeedSoCState {
/*< public >*/
ARMCPU cpu[ASPEED_CPUS_NUM];
- uint32_t num_cpus;
A15MPPrivState a7mpcore;
MemoryRegion *dram_mr;
MemoryRegion sram;
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 4d57d1e4363b..6f8f4b88f8ab 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -283,8 +283,6 @@ static void aspeed_machine_init(MachineState *machine)
&error_abort);
object_property_set_int(OBJECT(&bmc->soc), amc->num_cs, "num-cs",
&error_abort);
- object_property_set_int(OBJECT(&bmc->soc), machine->smp.cpus, "num-cpus",
- &error_abort);
object_property_set_link(OBJECT(&bmc->soc), OBJECT(&bmc->ram_container),
"dram", &error_abort);
if (machine->kernel_filename) {
@@ -337,7 +335,7 @@ static void aspeed_machine_init(MachineState *machine)
}
}
- if (machine->kernel_filename && bmc->soc.num_cpus > 1) {
+ 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",
@@ -352,7 +350,7 @@ static void aspeed_machine_init(MachineState *machine)
aspeed_board_binfo.ram_size = ram_size;
aspeed_board_binfo.loader_start = sc->memmap[ASPEED_SDRAM];
- aspeed_board_binfo.nb_cpus = bmc->soc.num_cpus;
+ aspeed_board_binfo.nb_cpus = sc->num_cpus;
if (amc->i2c_init) {
amc->i2c_init(bmc);
@@ -549,12 +547,17 @@ static void aspeed_machine_class_props_init(ObjectClass *oc)
"boot directly from CE0 flash device", &error_abort);
}
+static int aspeed_soc_num_cpus(const char *soc_name)
+{
+ AspeedSoCClass *sc = ASPEED_SOC_CLASS(object_class_by_name(soc_name));
+ return sc->num_cpus;
+}
+
static void aspeed_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
mc->init = aspeed_machine_init;
- mc->max_cpus = ASPEED_CPUS_NUM;
mc->no_floppy = 1;
mc->no_cdrom = 1;
mc->no_parallel = 1;
@@ -576,6 +579,8 @@ static void aspeed_machine_palmetto_class_init(ObjectClass *oc, void *data)
amc->num_cs = 1;
amc->i2c_init = palmetto_bmc_i2c_init;
mc->default_ram_size = 256 * MiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
@@ -591,6 +596,8 @@ static void aspeed_machine_ast2500_evb_class_init(ObjectClass *oc, void *data)
amc->num_cs = 1;
amc->i2c_init = ast2500_evb_i2c_init;
mc->default_ram_size = 512 * MiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data)
@@ -606,6 +613,8 @@ static void aspeed_machine_romulus_class_init(ObjectClass *oc, void *data)
amc->num_cs = 2;
amc->i2c_init = romulus_bmc_i2c_init;
mc->default_ram_size = 512 * MiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)
@@ -621,6 +630,8 @@ static void aspeed_machine_sonorapass_class_init(ObjectClass *oc, void *data)
amc->num_cs = 2;
amc->i2c_init = sonorapass_bmc_i2c_init;
mc->default_ram_size = 512 * MiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_swift_class_init(ObjectClass *oc, void *data)
@@ -636,6 +647,8 @@ static void aspeed_machine_swift_class_init(ObjectClass *oc, void *data)
amc->num_cs = 2;
amc->i2c_init = swift_bmc_i2c_init;
mc->default_ram_size = 512 * MiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_witherspoon_class_init(ObjectClass *oc, void *data)
@@ -651,6 +664,8 @@ static void aspeed_machine_witherspoon_class_init(ObjectClass *oc, void *data)
amc->num_cs = 2;
amc->i2c_init = witherspoon_bmc_i2c_init;
mc->default_ram_size = 512 * MiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
@@ -667,6 +682,8 @@ static void aspeed_machine_ast2600_evb_class_init(ObjectClass *oc, void *data)
amc->num_cs = 1;
amc->i2c_init = ast2600_evb_i2c_init;
mc->default_ram_size = 1 * GiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
@@ -683,6 +700,8 @@ static void aspeed_machine_tacoma_class_init(ObjectClass *oc, void *data)
amc->num_cs = 2;
amc->i2c_init = witherspoon_bmc_i2c_init; /* Same board layout */
mc->default_ram_size = 1 * GiB;
+ mc->default_cpus = mc->min_cpus = mc->max_cpus =
+ aspeed_soc_num_cpus(amc->soc_name);
};
static const TypeInfo aspeed_machine_types[] = {
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index c6e0ab84ac86..114b94f8f44d 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -256,17 +256,11 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_VIDEO],
0x1000);
- if (s->num_cpus > sc->num_cpus) {
- warn_report("%s: invalid number of CPUs %d, using default %d",
- sc->name, s->num_cpus, sc->num_cpus);
- s->num_cpus = sc->num_cpus;
- }
-
/* CPU */
- for (i = 0; i < s->num_cpus; i++) {
+ for (i = 0; i < sc->num_cpus; i++) {
object_property_set_int(OBJECT(&s->cpu[i]), QEMU_PSCI_CONDUIT_SMC,
"psci-conduit", &error_abort);
- if (s->num_cpus > 1) {
+ if (sc->num_cpus > 1) {
object_property_set_int(OBJECT(&s->cpu[i]),
ASPEED_A7MPCORE_ADDR,
"reset-cbar", &error_abort);
@@ -290,7 +284,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
}
/* A7MPCORE */
- object_property_set_int(OBJECT(&s->a7mpcore), s->num_cpus, "num-cpu",
+ object_property_set_int(OBJECT(&s->a7mpcore), sc->num_cpus, "num-cpu",
&error_abort);
object_property_set_int(OBJECT(&s->a7mpcore),
ASPEED_SOC_AST2600_MAX_IRQ + GIC_INTERNAL,
@@ -300,18 +294,18 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
&error_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(&s->a7mpcore), 0, ASPEED_A7MPCORE_ADDR);
- for (i = 0; i < s->num_cpus; i++) {
+ for (i = 0; i < sc->num_cpus; i++) {
SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
DeviceState *d = DEVICE(qemu_get_cpu(i));
irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
sysbus_connect_irq(sbd, i, irq);
irq = qdev_get_gpio_in(d, ARM_CPU_FIQ);
- sysbus_connect_irq(sbd, i + s->num_cpus, irq);
+ sysbus_connect_irq(sbd, i + sc->num_cpus, irq);
irq = qdev_get_gpio_in(d, ARM_CPU_VIRQ);
- sysbus_connect_irq(sbd, i + 2 * s->num_cpus, irq);
+ sysbus_connect_irq(sbd, i + 2 * sc->num_cpus, irq);
irq = qdev_get_gpio_in(d, ARM_CPU_VFIQ);
- sysbus_connect_irq(sbd, i + 3 * s->num_cpus, irq);
+ sysbus_connect_irq(sbd, i + 3 * sc->num_cpus, irq);
}
/* SRAM */
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 696c7fda14b7..984d29087dce 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -243,14 +243,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("aspeed.video", sc->memmap[ASPEED_VIDEO],
0x1000);
- if (s->num_cpus > sc->num_cpus) {
- warn_report("%s: invalid number of CPUs %d, using default %d",
- sc->name, s->num_cpus, sc->num_cpus);
- s->num_cpus = sc->num_cpus;
- }
-
/* CPU */
- for (i = 0; i < s->num_cpus; i++) {
+ for (i = 0; i < sc->num_cpus; i++) {
object_property_set_bool(OBJECT(&s->cpu[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
@@ -461,7 +455,6 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
aspeed_soc_get_irq(s, ASPEED_SDHCI));
}
static Property aspeed_soc_properties[] = {
- DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
DEFINE_PROP_LINK("dram", AspeedSoCState, dram_mr, TYPE_MEMORY_REGION,
MemoryRegion *),
DEFINE_PROP_END_OF_LIST(),
--
2.25.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] arm/aspeed: Compute the number of CPUs from the SoC definition
2020-05-19 9:16 [PATCH] arm/aspeed: Compute the number of CPUs from the SoC definition Cédric Le Goater
@ 2020-05-20 6:27 ` Markus Armbruster
2020-05-20 6:49 ` Cédric Le Goater
0 siblings, 1 reply; 3+ messages in thread
From: Markus Armbruster @ 2020-05-20 6:27 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel, Joel Stanley
I figure this replaces my "[PATCH 05/24] aspeed: Don't create unwanted
"cortex-a7-arm-cpu" devices". Correct?
Let's mention it gets rid of unrealized CPUs then. Let me try.
Cédric Le Goater <clg@kaod.org> writes:
> Commit ece09beec457 ("aspeed: introduce a configurable number of CPU
> per machine") was a convient change during bringup but the Aspeed SoCs
> have a fixed number of CPUs : one for the AST2400 and AST2500, and two
> for the AST2600.
Please add something like:
When the configured number of CPUs is less than the SoC's fixed
number, the "unconfigured" CPUs are left unrealized.
> Remove the "num-cpu" property from the SoC state and use the fixed
> number of CPUs defined in the SoC class instead. Compute the default,
> min, max number of CPUs of the machine directly from the SoC class
> definition.
Suggest to add:
Visible in "info qom-tree"; here's the change for ast2600-evb:
/machine (ast2600-evb-machine)
/peripheral (container)
/peripheral-anon (container)
/soc (ast2600-a1)
/a7mpcore (a15mpcore_priv)
/a15mp-priv-container[0] (qemu:memory-region)
/gic (arm_gic)
/gic_cpu[0] (qemu:memory-region)
/gic_cpu[1] (qemu:memory-region)
+ /gic_cpu[2] (qemu:memory-region)
/gic_dist[0] (qemu:memory-region)
/gic_vcpu[0] (qemu:memory-region)
/gic_viface[0] (qemu:memory-region)
/gic_viface[1] (qemu:memory-region)
+ /gic_viface[2] (qemu:memory-region)
/unnamed-gpio-in[0] (irq)
[...]
+ /unnamed-gpio-in[160] (irq)
[same for 161 to 190...]
+ /unnamed-gpio-in[191] (irq)
Also visible in "info qtree"; here's the change for ast2600-evb:
bus: main-system-bus
type System
dev: a15mpcore_priv, id ""
gpio-in "" 128
- gpio-out "sysbus-irq" 5
- num-cpu = 1 (0x1)
+ gpio-out "sysbus-irq" 10
+ num-cpu = 2 (0x2)
num-irq = 160 (0xa0)
mmio 0000000040460000/0000000000008000
dev: arm_gic, id ""
- gpio-in "" 160
- num-cpu = 1 (0x1)
+ gpio-in "" 192
+ num-cpu = 2 (0x2)
num-irq = 160 (0xa0)
revision = 2 (0x2)
has-security-extensions = true
has-virtualization-extensions = true
num-priority-bits = 8 (0x8)
mmio ffffffffffffffff/0000000000001000
mmio ffffffffffffffff/0000000000002000
mmio ffffffffffffffff/0000000000001000
mmio ffffffffffffffff/0000000000002000
mmio ffffffffffffffff/0000000000000100
+ mmio ffffffffffffffff/0000000000000100
+ mmio ffffffffffffffff/0000000000000200
mmio ffffffffffffffff/0000000000000200
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Patch looks good.
Preferably with an improved commit message:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
I'd like to pick your patch into v2 of my "Fixes around device
realization". May I amend your commit message as shown above?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm/aspeed: Compute the number of CPUs from the SoC definition
2020-05-20 6:27 ` Markus Armbruster
@ 2020-05-20 6:49 ` Cédric Le Goater
0 siblings, 0 replies; 3+ messages in thread
From: Cédric Le Goater @ 2020-05-20 6:49 UTC (permalink / raw)
To: Markus Armbruster
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, qemu-devel, Joel Stanley
On 5/20/20 8:27 AM, Markus Armbruster wrote:
> I figure this replaces my "[PATCH 05/24] aspeed: Don't create unwanted
> "cortex-a7-arm-cpu" devices". Correct?
>
> Let's mention it gets rid of unrealized CPUs then. Let me try.
>
> Cédric Le Goater <clg@kaod.org> writes:
>
>> Commit ece09beec457 ("aspeed: introduce a configurable number of CPU
>> per machine") was a convient change during bringup but the Aspeed SoCs
>> have a fixed number of CPUs : one for the AST2400 and AST2500, and two
>> for the AST2600.
>
> Please add something like:
>
> When the configured number of CPUs is less than the SoC's fixed
> number, the "unconfigured" CPUs are left unrealized.
>
>> Remove the "num-cpu" property from the SoC state and use the fixed
>> number of CPUs defined in the SoC class instead. Compute the default,
>> min, max number of CPUs of the machine directly from the SoC class
>> definition.
>
> Suggest to add:
>
> Visible in "info qom-tree"; here's the change for ast2600-evb:
>
> /machine (ast2600-evb-machine)
> /peripheral (container)
> /peripheral-anon (container)
> /soc (ast2600-a1)
> /a7mpcore (a15mpcore_priv)
> /a15mp-priv-container[0] (qemu:memory-region)
> /gic (arm_gic)
> /gic_cpu[0] (qemu:memory-region)
> /gic_cpu[1] (qemu:memory-region)
> + /gic_cpu[2] (qemu:memory-region)
> /gic_dist[0] (qemu:memory-region)
> /gic_vcpu[0] (qemu:memory-region)
> /gic_viface[0] (qemu:memory-region)
> /gic_viface[1] (qemu:memory-region)
> + /gic_viface[2] (qemu:memory-region)
> /unnamed-gpio-in[0] (irq)
> [...]
> + /unnamed-gpio-in[160] (irq)
> [same for 161 to 190...]
> + /unnamed-gpio-in[191] (irq)
>
> Also visible in "info qtree"; here's the change for ast2600-evb:
>
> bus: main-system-bus
> type System
> dev: a15mpcore_priv, id ""
> gpio-in "" 128
> - gpio-out "sysbus-irq" 5
> - num-cpu = 1 (0x1)
> + gpio-out "sysbus-irq" 10
> + num-cpu = 2 (0x2)
> num-irq = 160 (0xa0)
> mmio 0000000040460000/0000000000008000
> dev: arm_gic, id ""
> - gpio-in "" 160
> - num-cpu = 1 (0x1)
> + gpio-in "" 192
> + num-cpu = 2 (0x2)
> num-irq = 160 (0xa0)
> revision = 2 (0x2)
> has-security-extensions = true
> has-virtualization-extensions = true
> num-priority-bits = 8 (0x8)
> mmio ffffffffffffffff/0000000000001000
> mmio ffffffffffffffff/0000000000002000
> mmio ffffffffffffffff/0000000000001000
> mmio ffffffffffffffff/0000000000002000
> mmio ffffffffffffffff/0000000000000100
> + mmio ffffffffffffffff/0000000000000100
> + mmio ffffffffffffffff/0000000000000200
> mmio ffffffffffffffff/0000000000000200
>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>
> Patch looks good.
>
> Preferably with an improved commit message:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> I'd like to pick your patch into v2 of my "Fixes around device
> realization". May I amend your commit message as shown above?
Sure.
Thanks,
C.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-20 6:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-05-19 9:16 [PATCH] arm/aspeed: Compute the number of CPUs from the SoC definition Cédric Le Goater
2020-05-20 6:27 ` Markus Armbruster
2020-05-20 6:49 ` Cédric Le Goater
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).