Re: [PATCH 05/21] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell <peter.maydell@linaro.org>]

On Mon, 20 Jan 2025 at 20:38, Bernhard Beschow <shentey@gmail.com> wrote:
>
> As a first step, implement the bare minimum: CPUs, RAM, interrupt controller,
> serial. All other devices of the A53 memory map are represented as
> TYPE_UNIMPLEMENTED_DEVICE, i.e. the whole memory map is provided. This allows
> for running Linux without it crashing due to invalid memory accesses.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>  MAINTAINERS                    |   9 +
>  docs/system/arm/imx8mp-evk.rst |  47 +++++
>  docs/system/target-arm.rst     |   1 +
>  include/hw/arm/fsl-imx8mp.h    | 189 +++++++++++++++++
>  hw/arm/fsl-imx8mp.c            | 371 +++++++++++++++++++++++++++++++++
>  hw/arm/imx8mp-evk.c            |  55 +++++
>  hw/arm/Kconfig                 |  13 ++
>  hw/arm/meson.build             |   2 +
>  8 files changed, 687 insertions(+)
>  create mode 100644 docs/system/arm/imx8mp-evk.rst
>  create mode 100644 include/hw/arm/fsl-imx8mp.h
>  create mode 100644 hw/arm/fsl-imx8mp.c
>  create mode 100644 hw/arm/imx8mp-evk.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 846b81e3ec..cace8cf25b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -819,6 +819,15 @@ F: hw/pci-host/designware.c
>  F: include/hw/pci-host/designware.h
>  F: docs/system/arm/mcimx7d-sabre.rst
>
> +MCIMX8MP-EVK / i.MX8MP
> +M: Bernhard Beschow <shentey@gmail.com>
> +L: qemu-arm@nongnu.org
> +S: Odd Fixes

I'd rather we didn't take on a new board model that
starts out in the "odd fixes" state. If you don't
have enough time to look after it, it seems unlikely
that anybody else is going to want to take it over.

> +F: hw/arm/imx8mp-evk.c
> +F: hw/arm/fsl-imx8mp.c
> +F: include/hw/arm/fsl-imx8mp.h
> +F: docs/system/arm/imx8mp-evk.rst
> +
>  MPS2 / MPS3
>  M: Peter Maydell <peter.maydell@linaro.org>
>  L: qemu-arm@nongnu.org

> +static void fsl_imx8mp_realize(DeviceState *dev, Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    FslImx8mpState *s = FSL_IMX8MP(dev);
> +    DeviceState *gicdev = DEVICE(&s->gic);
> +    int i;
> +
> +    if (ms->smp.cpus > FSL_IMX8MP_NUM_CPUS) {
> +        error_setg(errp, "%s: Only %d CPUs are supported (%d requested)",
> +                   TYPE_FSL_IMX8MP, FSL_IMX8MP_NUM_CPUS, ms->smp.cpus);
> +        return;
> +    }
> +
> +    /* CPUs */
> +    for (i = 0; i < ms->smp.cpus; i++) {
> +        /* On uniprocessor, the CBAR is set to 0 */
> +        if (ms->smp.cpus > 1) {
> +            object_property_set_int(OBJECT(&s->cpu[i]), "reset-cbar",
> +                                    fsl_imx8mp_memmap[FSL_IMX8MP_GIC_DIST].addr,
> +                                    &error_abort);
> +        }
> +
> +        /*
> +         * Magic value form Linux output: "arch_timer: cp15 timer(s) running at
> +         * 8.00MHz (phys)".

"from"

> +         */
> +        object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 8000000,
> +                                &error_abort);
> +
> +        if (i) {
> +            /*
> +             * Secondary CPUs start in powered-down state (and can be
> +             * powered up via the SRC system reset controller)
> +             */
> +            object_property_set_bool(OBJECT(&s->cpu[i]), "start-powered-off",
> +                                     true, &error_abort);
> +        }
> +
> +        if (!qdev_realize(DEVICE(&s->cpu[i]), NULL, errp)) {
> +            return;
> +        }
> +    }
> +

> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index fe47e259b8..adb4ed8076 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -577,6 +577,19 @@ config FSL_IMX7
>      select SDHCI
>      select UNIMP
>
> +config FSL_IMX8MP
> +    bool
> +    imply TEST_DEVICES

Why do we need TEST_DEVICES ?

> +    select ARM_GIC
> +    select IMX
> +    select UNIMP
> +
> +config FSL_IMX8MP_EVK
> +    bool
> +    default y
> +    depends on TCG && AARCH64
> +    select FSL_IMX8MP
> +
>  config ARM_SMMUV3
>      bool

Looks good otherwise.

thanks
-- PMM