[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 14/33] hw/cpu/arm: Handle 'has_el2/3' properties once in MPCo
From: |
Fabiano Rosas |
Subject: |
Re: [PATCH 14/33] hw/cpu/arm: Handle 'has_el2/3' properties once in MPCore parent |
Date: |
Fri, 12 Jan 2024 18:33:13 -0300 |
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> Move the 'has_el2' and 'has_el3' properties to the abstract
> QOM parent.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> include/hw/cpu/cortex_mpcore.h | 5 +++++
> hw/arm/exynos4210.c | 10 ++++++++--
> hw/arm/vexpress.c | 6 ++++++
> hw/arm/xilinx_zynq.c | 6 ++++++
> hw/cpu/a15mpcore.c | 18 ++++++------------
> hw/cpu/a9mpcore.c | 5 +----
> hw/cpu/cortex_mpcore.c | 3 +++
> 7 files changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/include/hw/cpu/cortex_mpcore.h b/include/hw/cpu/cortex_mpcore.h
> index 0e7cca9e93..387552468c 100644
> --- a/include/hw/cpu/cortex_mpcore.h
> +++ b/include/hw/cpu/cortex_mpcore.h
> @@ -30,6 +30,8 @@
> * QEMU interface:
> * + QOM property "num-cores" which set the number of cores present in
> * the cluster.
> + * + QOM properties "cpu-has-el3", "cpu-has-el2" which set whether the CPUs
> + * have the exception level features present.
> */
> #define TYPE_CORTEX_MPCORE_PRIV "cortex_mpcore_priv"
> OBJECT_DECLARE_TYPE(CortexMPPrivState, CortexMPPrivClass, CORTEX_MPCORE_PRIV)
> @@ -53,6 +55,9 @@ struct CortexMPPrivState {
>
> /* Properties */
> uint32_t num_cores;
> +
> + bool cpu_has_el3;
> + bool cpu_has_el2;
> };
>
> #define TYPE_A9MPCORE_PRIV "a9mpcore_priv"
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index ea1364499d..7386a8fe57 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -548,7 +548,7 @@ static void exynos4210_realize(DeviceState *socdev, Error
> **errp)
> Exynos4210State *s = EXYNOS4210_SOC(socdev);
> MemoryRegion *system_mem = get_system_memory();
> SysBusDevice *busdev;
> - DeviceState *dev, *uart[4], *pl330[3];
> + DeviceState *dev, *mpdev, *uart[4], *pl330[3];
> int i, n;
>
> for (n = 0; n < EXYNOS4210_NCPUS; n++) {
> @@ -582,7 +582,13 @@ static void exynos4210_realize(DeviceState *socdev,
> Error **errp)
> }
>
> /* Private memory region and Internal GIC */
> - qdev_prop_set_uint32(DEVICE(&s->a9mpcore), "num-cores",
> EXYNOS4210_NCPUS);
> + mpdev = DEVICE(&s->a9mpcore);
> + qdev_prop_set_uint32(mpdev, "num-cores", EXYNOS4210_NCPUS);
> + /*
> + * By default A9 CPUs have EL3 enabled. This board does not currently
> + * support EL3 so the CPU EL3 property is disabled before realization.
> + */
> + qdev_prop_set_bit(mpdev, "cpu-has-el3", false);
> busdev = SYS_BUS_DEVICE(&s->a9mpcore);
> sysbus_realize(busdev, &error_fatal);
> sysbus_mmio_map(busdev, 0, EXYNOS4210_SMP_PRIVATE_BASE_ADDR);
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index a320d1c181..294b6f15f2 100644
> --- a/hw/arm/vexpress.c
> +++ b/hw/arm/vexpress.c
> @@ -239,6 +239,12 @@ static void init_cpus(MachineState *ms, const char
> *cpu_type,
> * wires itself up to the CPU's generic_timer gpio out lines.
> */
> dev = qdev_new(privdev);
> + if (!secure) {
> + qdev_prop_set_bit(dev, "cpu-has-el3", false);
> + }
> + if (!virt) {
> + qdev_prop_set_bit(dev, "cpu-has-el2", false);
> + }
> qdev_prop_set_uint32(dev, "num-cpu", smp_cpus);
> busdev = SYS_BUS_DEVICE(dev);
> sysbus_realize_and_unref(busdev, &error_fatal);
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index fc9c927d09..28430dcfba 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -240,6 +240,12 @@ static void zynq_init(MachineState *machine)
>
> dev = qdev_new(TYPE_A9MPCORE_PRIV);
> qdev_prop_set_uint32(dev, "num-cpu", 1);
> + /*
> + * By default A9 CPUs have EL3 enabled. This board does not
> + * currently support EL3 so the CPU EL3 property is disabled before
> + * realization.
> + */
> + qdev_prop_set_bit(dev, "cpu-has-el3", false);
> busdev = SYS_BUS_DEVICE(dev);
> sysbus_realize_and_unref(busdev, &error_fatal);
> sysbus_mmio_map(busdev, 0, MPCORE_PERIPHBASE);
> diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
> index 16874426e1..a16544fdde 100644
> --- a/hw/cpu/a15mpcore.c
> +++ b/hw/cpu/a15mpcore.c
> @@ -52,9 +52,6 @@ static void a15mp_priv_realize(DeviceState *dev, Error
> **errp)
> SysBusDevice *gicsbd;
> Error *local_err = NULL;
> int i;
> - bool has_el3;
> - bool has_el2 = false;
> - Object *cpuobj;
>
> cc->parent_realize(dev, &local_err);
> if (local_err) {
> @@ -70,14 +67,11 @@ static void a15mp_priv_realize(DeviceState *dev, Error
> **errp)
> /* Make the GIC's TZ support match the CPUs. We assume that
> * either all the CPUs have TZ, or none do.
> */
> - cpuobj = OBJECT(qemu_get_cpu(0));
> - has_el3 = object_property_find(cpuobj, "has_el3") &&
> - object_property_get_bool(cpuobj, "has_el3", &error_abort);
> - qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
> + qdev_prop_set_bit(gicdev, "has-security-extensions",
> + c->cpu_has_el3);
> /* Similarly for virtualization support */
> - has_el2 = object_property_find(cpuobj, "has_el2") &&
> - object_property_get_bool(cpuobj, "has_el2", &error_abort);
> - qdev_prop_set_bit(gicdev, "has-virtualization-extensions", has_el2);
> + qdev_prop_set_bit(gicdev, "has-virtualization-extensions",
> + c->cpu_has_el2);
> }
>
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) {
> @@ -112,7 +106,7 @@ static void a15mp_priv_realize(DeviceState *dev, Error
> **errp)
> qdev_get_gpio_in(gicdev,
> ppibase +
> timer_irq[irq]));
> }
> - if (has_el2) {
> + if (c->cpu_has_el2) {
> /* Connect the GIC maintenance interrupt to PPI ID 25 */
> sysbus_connect_irq(SYS_BUS_DEVICE(gicdev), i + 4 * c->num_cores,
> qdev_get_gpio_in(gicdev, ppibase + 25));
> @@ -134,7 +128,7 @@ static void a15mp_priv_realize(DeviceState *dev, Error
> **errp)
> sysbus_mmio_get_region(gicsbd, 0));
> memory_region_add_subregion(&c->container, 0x2000,
> sysbus_mmio_get_region(gicsbd, 1));
> - if (has_el2) {
> + if (c->cpu_has_el2) {
> memory_region_add_subregion(&c->container, 0x4000,
> sysbus_mmio_get_region(gicsbd, 2));
> memory_region_add_subregion(&c->container, 0x6000,
> diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
> index 9c138f4442..54949314f8 100644
> --- a/hw/cpu/a9mpcore.c
> +++ b/hw/cpu/a9mpcore.c
> @@ -51,7 +51,6 @@ static void a9mp_priv_realize(DeviceState *dev, Error
> **errp)
> SysBusDevice *scubusdev, *gicbusdev, *gtimerbusdev, *mptimerbusdev,
> *wdtbusdev;
> Error *local_err = NULL;
> - bool has_el3;
> CPUState *cpu0;
> Object *cpuobj;
>
> @@ -86,9 +85,7 @@ static void a9mp_priv_realize(DeviceState *dev, Error
> **errp)
> /* Make the GIC's TZ support match the CPUs. We assume that
> * either all the CPUs have TZ, or none do.
> */
> - has_el3 = object_property_find(cpuobj, "has_el3") &&
> - object_property_get_bool(cpuobj, "has_el3", &error_abort);
> - qdev_prop_set_bit(gicdev, "has-security-extensions", has_el3);
> + qdev_prop_set_bit(gicdev, "has-security-extensions", c->cpu_has_el3);
>
> if (!sysbus_realize(SYS_BUS_DEVICE(&s->gic), errp)) {
> return;
> diff --git a/hw/cpu/cortex_mpcore.c b/hw/cpu/cortex_mpcore.c
> index d7ea633e4e..549b81f708 100644
> --- a/hw/cpu/cortex_mpcore.c
> +++ b/hw/cpu/cortex_mpcore.c
> @@ -27,6 +27,9 @@ static Property cortex_mpcore_priv_properties[] = {
> DEFINE_PROP_UINT32("num-cores", CortexMPPrivState, num_cores, 1),
> DEFINE_PROP_UINT32("num-cpu", CortexMPPrivState, num_cores, 1), /* alias
> */
>
> + DEFINE_PROP_BOOL("cpu-has-el3", CortexMPPrivState, cpu_has_el3, true),
> + DEFINE_PROP_BOOL("cpu-has-el2", CortexMPPrivState, cpu_has_el2, false),
Are we missing setting cpu_has_el2 somewhere else? This^ results in fewer
cpregs being registered and is what breaks migration.
You can test with:
$ (echo "migrate file:migfile"; echo "quit") | ./qemu-system-arm -M ast2600-evb
-monitor stdio
$ ./scripts/analyze-migration.py -f migfile | grep array_len
Before series:
$ ./scripts/analyze-migration.py -f migfile | grep array_len
"cpreg_vmstate_array_len": "0x0000010a",
"cpreg_vmstate_array_len": "0x0000010a",
After series:
$ ./scripts/analyze-migration.py -f migfile | grep array_len
"cpreg_vmstate_array_len": "0x000000df",
"cpreg_vmstate_array_len": "0x000000df",
- Re: [PATCH 14/33] hw/cpu/arm: Handle 'has_el2/3' properties once in MPCore parent,
Fabiano Rosas <=