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é <phi...@redhat.com>
Tested-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> Signed-off-by: Peter Maydell <peter.mayd...@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);
> }
> }
>
>
