Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell]

On Wed, 26 Feb 2025 at 06:36, Bernhard Beschow  wrote:
>
>
>
> Am 25. Februar 2025 17:00:53 UTC schrieb Peter Maydell 
> :
> >On Tue, 25 Feb 2025 at 15:42, Peter Maydell  wrote:
> >> The C compiler for the OpenSUSE CI job doesn't seem to like this:
> >> https://gitlab.com/pm215/qemu/-/jobs/9239416833
> >>
> >> ../hw/arm/fsl-imx8mp.c: In function ‘fsl_imx8mp_realize’:
> >> ../hw/arm/fsl-imx8mp.c:382:15: error: initializer element is not constant
> >>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART1].addr, 
> >> FSL_IMX8MP_UART1_IRQ },
> >>^
> >> ../hw/arm/fsl-imx8mp.c:382:15: note: (near initialization for
> >> ‘serial_table[0].addr’)
> >> ../hw/arm/fsl-imx8mp.c:383:15: error: initializer element is not constant
> >>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART2].addr, 
> >> FSL_IMX8MP_UART2_IRQ },
> >>^
> >>
> >> This is (gcc 7.5.0 "cc (SUSE Linux) 7.5.0") apparently. That's
> >> a pretty old compiler, only just within the bounds of our
> >> version requirements (which are 7.4 or better), so I'm guessing
> >> it's just not as smart about figuring out that the
> >> initializer here really is a constant value.
> >>
> >> I'll fix this up by dropping the "const" from the serial_table[]
> >> etc definitions.
> >
> >More specifically, you have to drop 'static const', leaving just 'struct'.
> >Minimal repro: https://godbolt.org/z/5css4hv67
>
> I haven't checked, but this might be caused by the multiplications (... * KiB)

The godbolt repro case just uses plain constant values in the memmap
array, so it's not the multiplication that's at fault.

-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Bernhard Beschow]

Am 25. Februar 2025 17:00:53 UTC schrieb Peter Maydell 
:
>On Tue, 25 Feb 2025 at 15:42, Peter Maydell  wrote:
>> The C compiler for the OpenSUSE CI job doesn't seem to like this:
>> https://gitlab.com/pm215/qemu/-/jobs/9239416833
>>
>> ../hw/arm/fsl-imx8mp.c: In function ‘fsl_imx8mp_realize’:
>> ../hw/arm/fsl-imx8mp.c:382:15: error: initializer element is not constant
>>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART1].addr, 
>> FSL_IMX8MP_UART1_IRQ },
>>^
>> ../hw/arm/fsl-imx8mp.c:382:15: note: (near initialization for
>> ‘serial_table[0].addr’)
>> ../hw/arm/fsl-imx8mp.c:383:15: error: initializer element is not constant
>>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART2].addr, 
>> FSL_IMX8MP_UART2_IRQ },
>>^
>>
>> This is (gcc 7.5.0 "cc (SUSE Linux) 7.5.0") apparently. That's
>> a pretty old compiler, only just within the bounds of our
>> version requirements (which are 7.4 or better), so I'm guessing
>> it's just not as smart about figuring out that the
>> initializer here really is a constant value.
>>
>> I'll fix this up by dropping the "const" from the serial_table[]
>> etc definitions.
>
>More specifically, you have to drop 'static const', leaving just 'struct'.
>Minimal repro: https://godbolt.org/z/5css4hv67

I haven't checked, but this is probably caused by the multiplications (... * 
KiB) which the old compiler might not perform at compile time. Thanks for 
looking into it without another iteration of the series!

Best regards,
Bernhard

>
>-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Bernhard Beschow]

Am 25. Februar 2025 17:00:53 UTC schrieb Peter Maydell 
:
>On Tue, 25 Feb 2025 at 15:42, Peter Maydell  wrote:
>> The C compiler for the OpenSUSE CI job doesn't seem to like this:
>> https://gitlab.com/pm215/qemu/-/jobs/9239416833
>>
>> ../hw/arm/fsl-imx8mp.c: In function ‘fsl_imx8mp_realize’:
>> ../hw/arm/fsl-imx8mp.c:382:15: error: initializer element is not constant
>>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART1].addr, 
>> FSL_IMX8MP_UART1_IRQ },
>>^
>> ../hw/arm/fsl-imx8mp.c:382:15: note: (near initialization for
>> ‘serial_table[0].addr’)
>> ../hw/arm/fsl-imx8mp.c:383:15: error: initializer element is not constant
>>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART2].addr, 
>> FSL_IMX8MP_UART2_IRQ },
>>^
>>
>> This is (gcc 7.5.0 "cc (SUSE Linux) 7.5.0") apparently. That's
>> a pretty old compiler, only just within the bounds of our
>> version requirements (which are 7.4 or better), so I'm guessing
>> it's just not as smart about figuring out that the
>> initializer here really is a constant value.
>>
>> I'll fix this up by dropping the "const" from the serial_table[]
>> etc definitions.
>
>More specifically, you have to drop 'static const', leaving just 'struct'.
>Minimal repro: https://godbolt.org/z/5css4hv67

I haven't checked, but this might be caused by the multiplications (... * KiB) 
which the old compiler might not perform at compile time. Thanks for looking 
into it without another iteration of the series!

Best regards,
Bernhard


>
>-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell]

On Tue, 25 Feb 2025 at 15:42, Peter Maydell  wrote:
> The C compiler for the OpenSUSE CI job doesn't seem to like this:
> https://gitlab.com/pm215/qemu/-/jobs/9239416833
>
> ../hw/arm/fsl-imx8mp.c: In function ‘fsl_imx8mp_realize’:
> ../hw/arm/fsl-imx8mp.c:382:15: error: initializer element is not constant
>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART1].addr, FSL_IMX8MP_UART1_IRQ 
> },
>^
> ../hw/arm/fsl-imx8mp.c:382:15: note: (near initialization for
> ‘serial_table[0].addr’)
> ../hw/arm/fsl-imx8mp.c:383:15: error: initializer element is not constant
>  { fsl_imx8mp_memmap[FSL_IMX8MP_UART2].addr, FSL_IMX8MP_UART2_IRQ 
> },
>^
>
> This is (gcc 7.5.0 "cc (SUSE Linux) 7.5.0") apparently. That's
> a pretty old compiler, only just within the bounds of our
> version requirements (which are 7.4 or better), so I'm guessing
> it's just not as smart about figuring out that the
> initializer here really is a constant value.
>
> I'll fix this up by dropping the "const" from the serial_table[]
> etc definitions.

More specifically, you have to drop 'static const', leaving just 'struct'.
Minimal repro: https://godbolt.org/z/5css4hv67

-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell]

On Sun, 23 Feb 2025 at 11:47, Bernhard Beschow  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.

> +static const struct {
> +hwaddr addr;
> +size_t size;
> +const char *name;
> +} fsl_imx8mp_memmap[] = {


> +[FSL_IMX8MP_UART2] = { 0x3089, 64 * KiB, "uart2" },
> +[FSL_IMX8MP_UART3] = { 0x3088, 64 * KiB, "uart3" },
> +[FSL_IMX8MP_UART1] = { 0x3086, 64 * KiB, "uart1" },


> +/* UARTs */
> +for (i = 0; i < FSL_IMX8MP_NUM_UARTS; i++) {
> +static const struct {
> +hwaddr addr;
> +unsigned int irq;
> +} serial_table[FSL_IMX8MP_NUM_UARTS] = {
> +{ fsl_imx8mp_memmap[FSL_IMX8MP_UART1].addr, FSL_IMX8MP_UART1_IRQ 
> },
> +{ fsl_imx8mp_memmap[FSL_IMX8MP_UART2].addr, FSL_IMX8MP_UART2_IRQ 
> },
> +{ fsl_imx8mp_memmap[FSL_IMX8MP_UART3].addr, FSL_IMX8MP_UART3_IRQ 
> },
> +{ fsl_imx8mp_memmap[FSL_IMX8MP_UART4].addr, FSL_IMX8MP_UART4_IRQ 
> },
> +};

The C compiler for the OpenSUSE CI job doesn't seem to like this:
https://gitlab.com/pm215/qemu/-/jobs/9239416833

../hw/arm/fsl-imx8mp.c: In function ‘fsl_imx8mp_realize’:
../hw/arm/fsl-imx8mp.c:382:15: error: initializer element is not constant
 { fsl_imx8mp_memmap[FSL_IMX8MP_UART1].addr, FSL_IMX8MP_UART1_IRQ },
   ^
../hw/arm/fsl-imx8mp.c:382:15: note: (near initialization for
‘serial_table[0].addr’)
../hw/arm/fsl-imx8mp.c:383:15: error: initializer element is not constant
 { fsl_imx8mp_memmap[FSL_IMX8MP_UART2].addr, FSL_IMX8MP_UART2_IRQ },
   ^

This is (gcc 7.5.0 "cc (SUSE Linux) 7.5.0") apparently. That's
a pretty old compiler, only just within the bounds of our
version requirements (which are 7.4 or better), so I'm guessing
it's just not as smart about figuring out that the
initializer here really is a constant value.

I'll fix this up by dropping the "const" from the serial_table[]
etc definitions.

-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell]

On Sun, 23 Feb 2025 at 11:47, Bernhard Beschow  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 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Bernhard Beschow]

Am 10. Februar 2025 17:30:01 UTC schrieb Peter Maydell 
:
>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  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 
>> ---
>
>
>
>> --- /dev/null
>> +++ b/docs/system/arm/imx8mp-evk.rst
>> @@ -0,0 +1,56 @@
>> +NXP i.MX 8M Plus Evaluation Kit (``imx8mp-evk``)
>> +
>> +
>> +The QEMU i.MX 8M Plus EVK board emulation is intended to emulate a plain 
>> i.MX 8M
>> +Plus system on chip (SoC). All peripherals the real board has such as flash 
>> and
>> +I2C devices are intended to be added via configuration, e.g. command line.
>
>Why do this? If they're on the real hardware board, we should
>be creating them by default in the machine model in QEMU.
>Command line options are for devices that are optional and
>pluggable by the user on the hardware.

I'll rephrase the board description to be similar to the mcimx7d-sabre machine. 
In fact, I've modeled this machine like it.

Best regards,
Bernhard

>
>> +Supported devices
>> +-
>> +
>> +The ``imx8mp-evk`` machine implements the following devices:
>> +
>> + * Up to 4 Cortex-A53 Cores
>
>"cores"
>
>> + * Generic Interrupt Controller (GICv3)
>> + * 4 UARTs
>> +
>> +Boot options
>> +
>> +
>> +The ``imx8mp-evk`` machine can start using the standard -kernel 
>> functionality
>> +for loading a Linux kernel.
>> +
>> +Direct Linux Kernel Boot
>> +
>> +
>> +Probably the easiest way to get started with a whole Linux system on the 
>> machine
>> +is to generate an image with Buildroot. Version 2024.11.1 is tested at the 
>> time
>> +of writing and involves two steps. First run the following commands in the
>> +toplevel directory of the Buildroot source tree:
>> +
>> +.. code-block:: bash
>> +
>> +  $ echo "BR2_TARGET_ROOTFS_CPIO=y" >> configs/freescale_imx8mpevk_defconfig
>> +  $ make freescale_imx8mpevk_defconfig
>> +  $ make
>> +
>> +Once finished successfully there is an ``output/image`` subfolder. Navigate 
>> into
>> +it patch the device tree needs to be patched with the following commands 
>> which
>> +will remove the ``cpu-idle-states`` properties from CPU nodes:
>
>This sentence seems to be confused in the middle -- cut-and-paste
>error ?
>
>> +
>> +.. code-block:: bash
>> +
>> +  $ dtc imx8mp-evk.dtb | sed '/cpu-idle-states/d' > imx8mp-evk-patched.dts
>> +  $ dtc imx8mp-evk-patched.dts -o imx8mp-evk-patched.dtb
>> +
>> +Now that everything is prepared the newly built image can be run in the QEMU
>> +``imx8mp-evk`` machine:
>
>> +#define NAME_SIZE 20
>> +
>> +static void fsl_imx8mp_init(Object *obj)
>> +{
>> +MachineState *ms = MACHINE(qdev_get_machine());
>> +FslImx8mpState *s = FSL_IMX8MP(obj);
>> +char name[NAME_SIZE];
>
>Better than fixed sized char arrays for object names
>is to use
>   g_autofree char *name = g_strdup_printf("cpu%d", i);
>inside the block of each for() loop etc.
>
>> +int i;
>> +
>> +for (i = 0; i < MIN(ms->smp.cpus, FSL_IMX8MP_NUM_CPUS); i++) {
>> +snprintf(name, NAME_SIZE, "cpu%d", i);
>> +object_initialize_child(obj, name, &s->cpu[i],
>> +ARM_CPU_TYPE_NAME("cortex-a53"));
>> +}
>> +
>> +object_initialize_child(obj, "gic", &s->gic, TYPE_ARM_GICV3);
>> +
>> +for (i = 0; i < FSL_IMX8MP_NUM_UARTS; i++) {
>> +snprintf(name, NAME_SIZE, "uart%d", i + 1);
>> +object_initialize_child(obj, name, &s->uart[i], TYPE_IMX_SERIAL);
>> +}
>> +}
>> +
>> +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 from Linux output: "arch_timer: cp15 timer(s) 
>> running at
>> + * 8.00MHz (phys)".
>> + */
>> +object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 800,
>> +&error_abort);
>
>This is the CNTFRQ value in Hz. The datasheet a

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Bernhard Beschow]

Am 11. Februar 2025 10:06:19 UTC schrieb Peter Maydell 
:
>On Mon, 10 Feb 2025 at 22:44, Bernhard Beschow  wrote:
>>
>>
>>
>> Am 10. Februar 2025 17:30:01 UTC schrieb Peter Maydell 
>> :
>> >On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  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 
>> >> ---
>> >
>> >
>> >
>> >> --- /dev/null
>> >> +++ b/docs/system/arm/imx8mp-evk.rst
>> >> @@ -0,0 +1,56 @@
>> >> +NXP i.MX 8M Plus Evaluation Kit (``imx8mp-evk``)
>> >> +
>> >> +
>> >> +The QEMU i.MX 8M Plus EVK board emulation is intended to emulate a plain 
>> >> i.MX 8M
>> >> +Plus system on chip (SoC). All peripherals the real board has such as 
>> >> flash and
>> >> +I2C devices are intended to be added via configuration, e.g. command 
>> >> line.
>> >
>> >Why do this? If they're on the real hardware board, we should
>> >be creating them by default in the machine model in QEMU.
>> >Command line options are for devices that are optional and
>> >pluggable by the user on the hardware.
>>
>> My goal is to be able to model a custom, proprietary machine which runs in a 
>> CI. In order to avoid peripherals getting in the way, the idea is to have a 
>> machine which essentially exposes the plain SoC, and allow users to 
>> "decorate" it themselves. Is an EVK machine the wrong approach for this? Are 
>> there any other ways to achieve this, e.g. with -nodefaults?
>
>That's not really a usage model QEMU currently supports. We generally
>speaking model what the actual hardware does. 

Why not support such usage model? The machine in this series works amazingly 
well for our purposes. If modeling a concrete board implies hardcoding 
peripherals, would modeling an artificial "board" which just contains the SoC 
work instead? IOW should I drop the "-evk" suffix from the machine and claim it 
only models the SoC?

>In the same way,
>if you were running CI on the real EVK board your CI would have
>to deal with those devices being present.
>
>> >> +/*
>> >> + * Magic value from Linux output: "arch_timer: cp15 timer(s) 
>> >> running at
>> >> + * 8.00MHz (phys)".
>> >> + */
>> >> +object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 800,
>> >> +&error_abort);
>> >
>> >This is the CNTFRQ value in Hz. The datasheet actually tells you
>> >this, so we don't need to call it a magic number (section 4.11.4.1.6.4
>> >of the i.MX 8M Plus Applications Processor Reference Manual says the
>> >base frequency of the system counter is 8MHz).
>>
>> Ah, so then it's the "nxp,sysctr-timer"-compatible counter in
>> the device tree? I've stared my own implementation but didn't
>> see the relation to the cp15 timer. Thanks for clarifying this.
>
>Yeah; I haven't actually checked carefully whether there
>are NXP specifics here, as that device-tree compat string
>suggests. But it's almost certainly the same general principle:
>there's a memory-mapped system counter which is the global
>source of the system count, and each CPU consumes that count
>and uses it to feed its generic timers and counters. So if
>you mess with the enable on the memory mapped system counter
>then this causes the CPU's timers/counters to stop ticking.

Yeah, this also is how I understand the documentation of the system counter in 
the reference manual.

>
>I modelled this similarly to the in-tree M-profile
>sse-counter and sse-timer, where you link the timer to
>the counter and there's an API that lets the timer know
>when the counter is enabled/disabled, changes frequency, etc.
>This needs changes to the timer and counter code in the CPU
>itself. (You can get away without doing this if you assume
>that the guest code never tries to actually disable the system
>counter or pick a non-default frequency, which is typically true:
>it will just want to set it up to a single standard config and
>then leave it alone.)

I have this implementation which puts the focus more on waking up the CPUs via 
the IRQ: 

 The IRQ fires but doesn't wake any CPU in case of direct kernel boot since the 
IRQ is masked in EL1 (rich OS mode) and EL3 isn't set up.

>
>> >(Incidentally the memory mapped system counter is a stock Arm
>> >IP block and I have about 60% of a model of it, I just haven't
>> >needed to upstream it yet because in practice no guest software
>> >really tries to mess with it. If we turn out to need it for
>> >this SoC that would be a reason for me to clean it up and
>> >send it out. But I suspect we don't need it in practice.)
>>
>> The ugly workar

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell]

On Mon, 10 Feb 2025 at 22:44, Bernhard Beschow  wrote:
>
>
>
> Am 10. Februar 2025 17:30:01 UTC schrieb Peter Maydell 
> :
> >On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  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 
> >> ---
> >
> >
> >
> >> --- /dev/null
> >> +++ b/docs/system/arm/imx8mp-evk.rst
> >> @@ -0,0 +1,56 @@
> >> +NXP i.MX 8M Plus Evaluation Kit (``imx8mp-evk``)
> >> +
> >> +
> >> +The QEMU i.MX 8M Plus EVK board emulation is intended to emulate a plain 
> >> i.MX 8M
> >> +Plus system on chip (SoC). All peripherals the real board has such as 
> >> flash and
> >> +I2C devices are intended to be added via configuration, e.g. command line.
> >
> >Why do this? If they're on the real hardware board, we should
> >be creating them by default in the machine model in QEMU.
> >Command line options are for devices that are optional and
> >pluggable by the user on the hardware.
>
> My goal is to be able to model a custom, proprietary machine which runs in a 
> CI. In order to avoid peripherals getting in the way, the idea is to have a 
> machine which essentially exposes the plain SoC, and allow users to 
> "decorate" it themselves. Is an EVK machine the wrong approach for this? Are 
> there any other ways to achieve this, e.g. with -nodefaults?

That's not really a usage model QEMU currently supports. We generally
speaking model what the actual hardware does. In the same way,
if you were running CI on the real EVK board your CI would have
to deal with those devices being present.

> >> +/*
> >> + * Magic value from Linux output: "arch_timer: cp15 timer(s) 
> >> running at
> >> + * 8.00MHz (phys)".
> >> + */
> >> +object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 800,
> >> +&error_abort);
> >
> >This is the CNTFRQ value in Hz. The datasheet actually tells you
> >this, so we don't need to call it a magic number (section 4.11.4.1.6.4
> >of the i.MX 8M Plus Applications Processor Reference Manual says the
> >base frequency of the system counter is 8MHz).
>
> Ah, so then it's the "nxp,sysctr-timer"-compatible counter in
> the device tree? I've stared my own implementation but didn't
> see the relation to the cp15 timer. Thanks for clarifying this.

Yeah; I haven't actually checked carefully whether there
are NXP specifics here, as that device-tree compat string
suggests. But it's almost certainly the same general principle:
there's a memory-mapped system counter which is the global
source of the system count, and each CPU consumes that count
and uses it to feed its generic timers and counters. So if
you mess with the enable on the memory mapped system counter
then this causes the CPU's timers/counters to stop ticking.

I modelled this similarly to the in-tree M-profile
sse-counter and sse-timer, where you link the timer to
the counter and there's an API that lets the timer know
when the counter is enabled/disabled, changes frequency, etc.
This needs changes to the timer and counter code in the CPU
itself. (You can get away without doing this if you assume
that the guest code never tries to actually disable the system
counter or pick a non-default frequency, which is typically true:
it will just want to set it up to a single standard config and
then leave it alone.)

> >(Incidentally the memory mapped system counter is a stock Arm
> >IP block and I have about 60% of a model of it, I just haven't
> >needed to upstream it yet because in practice no guest software
> >really tries to mess with it. If we turn out to need it for
> >this SoC that would be a reason for me to clean it up and
> >send it out. But I suspect we don't need it in practice.)
>
> The ugly workaround for the cpu-idle-states above is actually related to this 
> counter, IIUC. I suppose that QEMU doesn't support these idle states, it only 
> seems to support WFI. But if it did, then this counter would wake up the CPUs 
> (if I'm not mistaken, I'm no expert here). It would be nice to be able to 
> boot the machine via U-Boot, and I wonder if that could fix the idle states 
> when there are PSCI handlers in the secure world. Again, this probably also 
> requires the counter implementation. Looking into U-Boot, TF-A, and OP-TEE is 
> on my plan, but right now I'm focusing on direct kernel boot.

Hmm, if the system counter can generate interrupts then
it might be different from the Arm IP block in that
respect. I'd have to check the datasheet. Anyway, as
you suggest this is all something we can postpone until
we need to care about running EL3 firmware in the guest.

thanks
-- PMM

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Bernhard Beschow]

Am 10. Februar 2025 17:30:01 UTC schrieb Peter Maydell 
:
>On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  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 
>> ---
>
>
>
>> --- /dev/null
>> +++ b/docs/system/arm/imx8mp-evk.rst
>> @@ -0,0 +1,56 @@
>> +NXP i.MX 8M Plus Evaluation Kit (``imx8mp-evk``)
>> +
>> +
>> +The QEMU i.MX 8M Plus EVK board emulation is intended to emulate a plain 
>> i.MX 8M
>> +Plus system on chip (SoC). All peripherals the real board has such as flash 
>> and
>> +I2C devices are intended to be added via configuration, e.g. command line.
>
>Why do this? If they're on the real hardware board, we should
>be creating them by default in the machine model in QEMU.
>Command line options are for devices that are optional and
>pluggable by the user on the hardware.

My goal is to be able to model a custom, proprietary machine which runs in a 
CI. In order to avoid peripherals getting in the way, the idea is to have a 
machine which essentially exposes the plain SoC, and allow users to "decorate" 
it themselves. Is an EVK machine the wrong approach for this? Are there any 
other ways to achieve this, e.g. with -nodefaults?

>
>> +Supported devices
>> +-
>> +
>> +The ``imx8mp-evk`` machine implements the following devices:
>> +
>> + * Up to 4 Cortex-A53 Cores
>
>"cores"

Fixed in v3.

>
>> + * Generic Interrupt Controller (GICv3)
>> + * 4 UARTs
>> +
>> +Boot options
>> +
>> +
>> +The ``imx8mp-evk`` machine can start using the standard -kernel 
>> functionality
>> +for loading a Linux kernel.
>> +
>> +Direct Linux Kernel Boot
>> +
>> +
>> +Probably the easiest way to get started with a whole Linux system on the 
>> machine
>> +is to generate an image with Buildroot. Version 2024.11.1 is tested at the 
>> time
>> +of writing and involves two steps. First run the following commands in the
>> +toplevel directory of the Buildroot source tree:
>> +
>> +.. code-block:: bash
>> +
>> +  $ echo "BR2_TARGET_ROOTFS_CPIO=y" >> configs/freescale_imx8mpevk_defconfig
>> +  $ make freescale_imx8mpevk_defconfig
>> +  $ make
>> +
>> +Once finished successfully there is an ``output/image`` subfolder. Navigate 
>> into
>> +it patch the device tree needs to be patched with the following commands 
>> which
>> +will remove the ``cpu-idle-states`` properties from CPU nodes:
>
>This sentence seems to be confused in the middle -- cut-and-paste
>error ?

Yeah, probably. Fixed in v3.

>
>> +
>> +.. code-block:: bash
>> +
>> +  $ dtc imx8mp-evk.dtb | sed '/cpu-idle-states/d' > imx8mp-evk-patched.dts
>> +  $ dtc imx8mp-evk-patched.dts -o imx8mp-evk-patched.dtb
>> +
>> +Now that everything is prepared the newly built image can be run in the QEMU
>> +``imx8mp-evk`` machine:
>
>> +#define NAME_SIZE 20
>> +
>> +static void fsl_imx8mp_init(Object *obj)
>> +{
>> +MachineState *ms = MACHINE(qdev_get_machine());
>> +FslImx8mpState *s = FSL_IMX8MP(obj);
>> +char name[NAME_SIZE];
>
>Better than fixed sized char arrays for object names
>is to use
>   g_autofree char *name = g_strdup_printf("cpu%d", i);
>inside the block of each for() loop etc.

Will fix for v3, touching subsequent patches.

>
>> +int i;
>> +
>> +for (i = 0; i < MIN(ms->smp.cpus, FSL_IMX8MP_NUM_CPUS); i++) {
>> +snprintf(name, NAME_SIZE, "cpu%d", i);
>> +object_initialize_child(obj, name, &s->cpu[i],
>> +ARM_CPU_TYPE_NAME("cortex-a53"));
>> +}
>> +
>> +object_initialize_child(obj, "gic", &s->gic, TYPE_ARM_GICV3);
>> +
>> +for (i = 0; i < FSL_IMX8MP_NUM_UARTS; i++) {
>> +snprintf(name, NAME_SIZE, "uart%d", i + 1);
>> +object_initialize_child(obj, name, &s->uart[i], TYPE_IMX_SERIAL);
>> +}
>> +}
>> +
>> +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);
>> +}
>> +
>> +/*
>> +

Re: [PATCH v2 04/18] hw/arm: Add i.MX 8M Plus EVK board

[Peter Maydell]

On Tue, 4 Feb 2025 at 09:21, Bernhard Beschow  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 
> ---



> --- /dev/null
> +++ b/docs/system/arm/imx8mp-evk.rst
> @@ -0,0 +1,56 @@
> +NXP i.MX 8M Plus Evaluation Kit (``imx8mp-evk``)
> +
> +
> +The QEMU i.MX 8M Plus EVK board emulation is intended to emulate a plain 
> i.MX 8M
> +Plus system on chip (SoC). All peripherals the real board has such as flash 
> and
> +I2C devices are intended to be added via configuration, e.g. command line.

Why do this? If they're on the real hardware board, we should
be creating them by default in the machine model in QEMU.
Command line options are for devices that are optional and
pluggable by the user on the hardware.

> +Supported devices
> +-
> +
> +The ``imx8mp-evk`` machine implements the following devices:
> +
> + * Up to 4 Cortex-A53 Cores

"cores"

> + * Generic Interrupt Controller (GICv3)
> + * 4 UARTs
> +
> +Boot options
> +
> +
> +The ``imx8mp-evk`` machine can start using the standard -kernel functionality
> +for loading a Linux kernel.
> +
> +Direct Linux Kernel Boot
> +
> +
> +Probably the easiest way to get started with a whole Linux system on the 
> machine
> +is to generate an image with Buildroot. Version 2024.11.1 is tested at the 
> time
> +of writing and involves two steps. First run the following commands in the
> +toplevel directory of the Buildroot source tree:
> +
> +.. code-block:: bash
> +
> +  $ echo "BR2_TARGET_ROOTFS_CPIO=y" >> configs/freescale_imx8mpevk_defconfig
> +  $ make freescale_imx8mpevk_defconfig
> +  $ make
> +
> +Once finished successfully there is an ``output/image`` subfolder. Navigate 
> into
> +it patch the device tree needs to be patched with the following commands 
> which
> +will remove the ``cpu-idle-states`` properties from CPU nodes:

This sentence seems to be confused in the middle -- cut-and-paste
error ?

> +
> +.. code-block:: bash
> +
> +  $ dtc imx8mp-evk.dtb | sed '/cpu-idle-states/d' > imx8mp-evk-patched.dts
> +  $ dtc imx8mp-evk-patched.dts -o imx8mp-evk-patched.dtb
> +
> +Now that everything is prepared the newly built image can be run in the QEMU
> +``imx8mp-evk`` machine:

> +#define NAME_SIZE 20
> +
> +static void fsl_imx8mp_init(Object *obj)
> +{
> +MachineState *ms = MACHINE(qdev_get_machine());
> +FslImx8mpState *s = FSL_IMX8MP(obj);
> +char name[NAME_SIZE];

Better than fixed sized char arrays for object names
is to use
   g_autofree char *name = g_strdup_printf("cpu%d", i);
inside the block of each for() loop etc.

> +int i;
> +
> +for (i = 0; i < MIN(ms->smp.cpus, FSL_IMX8MP_NUM_CPUS); i++) {
> +snprintf(name, NAME_SIZE, "cpu%d", i);
> +object_initialize_child(obj, name, &s->cpu[i],
> +ARM_CPU_TYPE_NAME("cortex-a53"));
> +}
> +
> +object_initialize_child(obj, "gic", &s->gic, TYPE_ARM_GICV3);
> +
> +for (i = 0; i < FSL_IMX8MP_NUM_UARTS; i++) {
> +snprintf(name, NAME_SIZE, "uart%d", i + 1);
> +object_initialize_child(obj, name, &s->uart[i], TYPE_IMX_SERIAL);
> +}
> +}
> +
> +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 from Linux output: "arch_timer: cp15 timer(s) running 
> at
> + * 8.00MHz (phys)".
> + */
> +object_property_set_int(OBJECT(&s->cpu[i]), "cntfrq", 800,
> +&error_abort);

This is the CNTFRQ value in Hz. The datasheet actually tells you
this, so we don't need to call it a magic number (section 4.11.4.1.6.4
of the i.MX 8M Plus Applications Processor Reference Manual says the
base frequency of the system counter is 8MHz).

(Incidentally the memory mapped system counter is a stock Arm
IP block and I have about 60% of a model of it, I just haven't
needed to upstream it yet b