include/hw/arm/fsl-imx6ul.h | 2 +- hw/arm/fsl-imx6ul.c | 62 +++++++++++-------------------------- hw/arm/mcimx6ul-evk.c | 2 +- 3 files changed, 20 insertions(+), 46 deletions(-)
The i.MX6UL always has a single Cortex-A7 CPU (we set FSL_IMX6UL_NUM_CPUS
to 1 in line with this). This means that all the code in fsl-imx6ul.c to
handle multiple CPUs is dead code, and Coverity is now complaining that
it is unreachable (CID 1403008, 1403011).
Remove the unreachable code and the only-executes-once loops,
and replace the single-entry cpu[] array in the FSLIMX6ULState
with a simple cpu member.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
The only real reason to put this into 4.1 is because it fixes
some Coverity issues, and it would be nice to be able to get
down to no Coverity issues for the release. I think that pre-rc1
that's a reasonable reason to put this in.
Disclaimer: tested with "make check" as I have no test image for
this board.
include/hw/arm/fsl-imx6ul.h | 2 +-
hw/arm/fsl-imx6ul.c | 62 +++++++++++--------------------------
hw/arm/mcimx6ul-evk.c | 2 +-
3 files changed, 20 insertions(+), 46 deletions(-)
diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index 9e94e98f8ee..eda389aec7d 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -61,7 +61,7 @@ typedef struct FslIMX6ULState {
DeviceState parent_obj;
/*< public >*/
- ARMCPU cpu[FSL_IMX6UL_NUM_CPUS];
+ ARMCPU cpu;
A15MPPrivState a7mpcore;
IMXGPTState gpt[FSL_IMX6UL_NUM_GPTS];
IMXEPITState epit[FSL_IMX6UL_NUM_EPITS];
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index f8601654388..b074177a71d 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -29,16 +29,12 @@
static void fsl_imx6ul_init(Object *obj)
{
- MachineState *ms = MACHINE(qdev_get_machine());
FslIMX6ULState *s = FSL_IMX6UL(obj);
char name[NAME_SIZE];
int i;
- for (i = 0; i < MIN(ms->smp.cpus, FSL_IMX6UL_NUM_CPUS); i++) {
- snprintf(name, NAME_SIZE, "cpu%d", i);
- object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
- "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
- }
+ object_initialize_child(obj, "cpu0", &s->cpu, sizeof(s->cpu),
+ "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
/*
* A7MPCORE
@@ -161,42 +157,25 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
MachineState *ms = MACHINE(qdev_get_machine());
FslIMX6ULState *s = FSL_IMX6UL(dev);
int i;
- qemu_irq irq;
char name[NAME_SIZE];
- unsigned int smp_cpus = ms->smp.cpus;
+ SysBusDevice *sbd;
+ DeviceState *d;
- if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
- error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
- TYPE_FSL_IMX6UL, FSL_IMX6UL_NUM_CPUS, smp_cpus);
+ if (ms->smp.cpus > 1) {
+ error_setg(errp, "%s: Only a single CPU is supported (%d requested)",
+ TYPE_FSL_IMX6UL, ms->smp.cpus);
return;
}
- for (i = 0; i < smp_cpus; i++) {
- Object *o = OBJECT(&s->cpu[i]);
-
- object_property_set_int(o, QEMU_PSCI_CONDUIT_SMC,
- "psci-conduit", &error_abort);
-
- /* On uniprocessor, the CBAR is set to 0 */
- if (smp_cpus > 1) {
- object_property_set_int(o, FSL_IMX6UL_A7MPCORE_ADDR,
- "reset-cbar", &error_abort);
- }
-
- if (i) {
- /* Secondary CPUs start in PSCI powered-down state */
- object_property_set_bool(o, true,
- "start-powered-off", &error_abort);
- }
-
- object_property_set_bool(o, true, "realized", &error_abort);
- }
+ object_property_set_int(OBJECT(&s->cpu), QEMU_PSCI_CONDUIT_SMC,
+ "psci-conduit", &error_abort);
+ object_property_set_bool(OBJECT(&s->cpu), true,
+ "realized", &error_abort);
/*
* A7MPCORE
*/
- object_property_set_int(OBJECT(&s->a7mpcore), smp_cpus, "num-cpu",
- &error_abort);
+ object_property_set_int(OBJECT(&s->a7mpcore), 1, "num-cpu", &error_abort);
object_property_set_int(OBJECT(&s->a7mpcore),
FSL_IMX6UL_MAX_IRQ + GIC_INTERNAL,
"num-irq", &error_abort);
@@ -204,18 +183,13 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
&error_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(&s->a7mpcore), 0, FSL_IMX6UL_A7MPCORE_ADDR);
- for (i = 0; i < smp_cpus; i++) {
- SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
- DeviceState *d = DEVICE(qemu_get_cpu(i));
+ sbd = SYS_BUS_DEVICE(&s->a7mpcore);
+ d = DEVICE(&s->cpu);
- irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
- sysbus_connect_irq(sbd, i, irq);
- sysbus_connect_irq(sbd, i + smp_cpus, qdev_get_gpio_in(d, ARM_CPU_FIQ));
- sysbus_connect_irq(sbd, i + 2 * smp_cpus,
- qdev_get_gpio_in(d, ARM_CPU_VIRQ));
- sysbus_connect_irq(sbd, i + 3 * smp_cpus,
- qdev_get_gpio_in(d, ARM_CPU_VFIQ));
- }
+ sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(d, ARM_CPU_IRQ));
+ sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(d, ARM_CPU_FIQ));
+ sysbus_connect_irq(sbd, 2, qdev_get_gpio_in(d, ARM_CPU_VIRQ));
+ sysbus_connect_irq(sbd, 3, qdev_get_gpio_in(d, ARM_CPU_VFIQ));
/*
* A7MPCORE DAP
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index bbffb11c2a8..1f6f4aed97c 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -71,7 +71,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
}
if (!qtest_enabled()) {
- arm_load_kernel(&s->soc.cpu[0], &boot_info);
+ arm_load_kernel(&s->soc.cpu, &boot_info);
}
}
--
2.20.1
On 7/12/19 1:50 PM, Peter Maydell wrote:
> The i.MX6UL always has a single Cortex-A7 CPU (we set FSL_IMX6UL_NUM_CPUS
> to 1 in line with this). This means that all the code in fsl-imx6ul.c to
> handle multiple CPUs is dead code, and Coverity is now complaining that
> it is unreachable (CID 1403008, 1403011).
>
> Remove the unreachable code and the only-executes-once loops,
> and replace the single-entry cpu[] array in the FSLIMX6ULState
> with a simple cpu member.
The header says "Based on hw/arm/fsl-imx7.c" which has
FSL_IMX7_NUM_CPUS=2, makes sense.
Looking at the i.MX 6:
include/hw/arm/fsl-imx6.h:38:#define FSL_IMX6_NUM_CPUS 4
Ok, we can have 1/2/4 cpus in the family.
The UltraLite series is part of the i.MX 6 family, I wonder why we
needed a different FslIMX6ULState than FslIMX6State.
Anyway, not related to this cleanup patch, so meanwhile:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> The only real reason to put this into 4.1 is because it fixes
> some Coverity issues, and it would be nice to be able to get
> down to no Coverity issues for the release. I think that pre-rc1
> that's a reasonable reason to put this in.
Agreed.
> Disclaimer: tested with "make check" as I have no test image for
> this board.
>
> include/hw/arm/fsl-imx6ul.h | 2 +-
> hw/arm/fsl-imx6ul.c | 62 +++++++++++--------------------------
> hw/arm/mcimx6ul-evk.c | 2 +-
> 3 files changed, 20 insertions(+), 46 deletions(-)
>
> diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
> index 9e94e98f8ee..eda389aec7d 100644
> --- a/include/hw/arm/fsl-imx6ul.h
> +++ b/include/hw/arm/fsl-imx6ul.h
> @@ -61,7 +61,7 @@ typedef struct FslIMX6ULState {
> DeviceState parent_obj;
>
> /*< public >*/
> - ARMCPU cpu[FSL_IMX6UL_NUM_CPUS];
> + ARMCPU cpu;
> A15MPPrivState a7mpcore;
> IMXGPTState gpt[FSL_IMX6UL_NUM_GPTS];
> IMXEPITState epit[FSL_IMX6UL_NUM_EPITS];
> diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
> index f8601654388..b074177a71d 100644
> --- a/hw/arm/fsl-imx6ul.c
> +++ b/hw/arm/fsl-imx6ul.c
> @@ -29,16 +29,12 @@
>
> static void fsl_imx6ul_init(Object *obj)
> {
> - MachineState *ms = MACHINE(qdev_get_machine());
> FslIMX6ULState *s = FSL_IMX6UL(obj);
> char name[NAME_SIZE];
> int i;
>
> - for (i = 0; i < MIN(ms->smp.cpus, FSL_IMX6UL_NUM_CPUS); i++) {
> - snprintf(name, NAME_SIZE, "cpu%d", i);
> - object_initialize_child(obj, name, &s->cpu[i], sizeof(s->cpu[i]),
> - "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
> - }
> + object_initialize_child(obj, "cpu0", &s->cpu, sizeof(s->cpu),
> + "cortex-a7-" TYPE_ARM_CPU, &error_abort, NULL);
>
> /*
> * A7MPCORE
> @@ -161,42 +157,25 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
> MachineState *ms = MACHINE(qdev_get_machine());
> FslIMX6ULState *s = FSL_IMX6UL(dev);
> int i;
> - qemu_irq irq;
> char name[NAME_SIZE];
> - unsigned int smp_cpus = ms->smp.cpus;
> + SysBusDevice *sbd;
> + DeviceState *d;
>
> - if (smp_cpus > FSL_IMX6UL_NUM_CPUS) {
> - error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> - TYPE_FSL_IMX6UL, FSL_IMX6UL_NUM_CPUS, smp_cpus);
> + if (ms->smp.cpus > 1) {
> + error_setg(errp, "%s: Only a single CPU is supported (%d requested)",
> + TYPE_FSL_IMX6UL, ms->smp.cpus);
> return;
> }
>
> - for (i = 0; i < smp_cpus; i++) {
> - Object *o = OBJECT(&s->cpu[i]);
> -
> - object_property_set_int(o, QEMU_PSCI_CONDUIT_SMC,
> - "psci-conduit", &error_abort);
> -
> - /* On uniprocessor, the CBAR is set to 0 */
> - if (smp_cpus > 1) {
> - object_property_set_int(o, FSL_IMX6UL_A7MPCORE_ADDR,
> - "reset-cbar", &error_abort);
> - }
> -
> - if (i) {
> - /* Secondary CPUs start in PSCI powered-down state */
> - object_property_set_bool(o, true,
> - "start-powered-off", &error_abort);
> - }
> -
> - object_property_set_bool(o, true, "realized", &error_abort);
> - }
> + object_property_set_int(OBJECT(&s->cpu), QEMU_PSCI_CONDUIT_SMC,
> + "psci-conduit", &error_abort);
> + object_property_set_bool(OBJECT(&s->cpu), true,
> + "realized", &error_abort);
>
> /*
> * A7MPCORE
> */
> - object_property_set_int(OBJECT(&s->a7mpcore), smp_cpus, "num-cpu",
> - &error_abort);
> + object_property_set_int(OBJECT(&s->a7mpcore), 1, "num-cpu", &error_abort);
> object_property_set_int(OBJECT(&s->a7mpcore),
> FSL_IMX6UL_MAX_IRQ + GIC_INTERNAL,
> "num-irq", &error_abort);
> @@ -204,18 +183,13 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error **errp)
> &error_abort);
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->a7mpcore), 0, FSL_IMX6UL_A7MPCORE_ADDR);
>
> - for (i = 0; i < smp_cpus; i++) {
> - SysBusDevice *sbd = SYS_BUS_DEVICE(&s->a7mpcore);
> - DeviceState *d = DEVICE(qemu_get_cpu(i));
> + sbd = SYS_BUS_DEVICE(&s->a7mpcore);
> + d = DEVICE(&s->cpu);
>
> - irq = qdev_get_gpio_in(d, ARM_CPU_IRQ);
> - sysbus_connect_irq(sbd, i, irq);
> - sysbus_connect_irq(sbd, i + smp_cpus, qdev_get_gpio_in(d, ARM_CPU_FIQ));
> - sysbus_connect_irq(sbd, i + 2 * smp_cpus,
> - qdev_get_gpio_in(d, ARM_CPU_VIRQ));
> - sysbus_connect_irq(sbd, i + 3 * smp_cpus,
> - qdev_get_gpio_in(d, ARM_CPU_VFIQ));
> - }
> + sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(d, ARM_CPU_IRQ));
> + sysbus_connect_irq(sbd, 1, qdev_get_gpio_in(d, ARM_CPU_FIQ));
> + sysbus_connect_irq(sbd, 2, qdev_get_gpio_in(d, ARM_CPU_VIRQ));
> + sysbus_connect_irq(sbd, 3, qdev_get_gpio_in(d, ARM_CPU_VFIQ));
>
> /*
> * A7MPCORE DAP
> diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
> index bbffb11c2a8..1f6f4aed97c 100644
> --- a/hw/arm/mcimx6ul-evk.c
> +++ b/hw/arm/mcimx6ul-evk.c
> @@ -71,7 +71,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
> }
>
> if (!qtest_enabled()) {
> - arm_load_kernel(&s->soc.cpu[0], &boot_info);
> + arm_load_kernel(&s->soc.cpu, &boot_info);
> }
> }
>
>
© 2016 - 2024 Red Hat, Inc.