[PATCH 00/13] hw/arm: Implement new machine mps3-an536 (Cortex-R52 MPS3 AN536 FPGA image)

[PATCH 00/13] hw/arm: Implement new machine mps3-an536 (Cortex-R52 MPS3 AN536 FPGA image)

[Peter Maydell]

This patchset implements a new machine type, mps3-an536.  This is
similar to our existing mps2-* and mps3-* machine types in that it is
a model of an FPGA image for the Arm MPS3 development board.  It
differs from our current machine types in being based around an
R-profile CPU, the Cortex-R52, rather than an M-profile CPU.

This patchset implements the board support to a similar level as that
of our M-profile MPS machine types: most but not all devices are
supported.  Some minor missing features are documented in the docs
patch (and already in mps2.rst where they apply to all boards in this
family).  If there's demand (specifically from the Xen development
use case which is motivating this board model) we can look at adding
them.

I have tested with the "selftest" binary supplied with the FPGA
image, and also with a Linux kernel. (The kernel support for this
board is not upstream, but the patches can be found in this lakml thread
https://lore.kernel.org/all/20220630083641.21835-1-vladimir.murzin@arm.com/
or at the git branch
https://gitlab.arm.com/linux-arm/linux-vm/-/tree/staging/rclass-5.19 )
To get some of this guest code to boot I had to make some improvements
to our Cortex-R52 emulation; these are in the first four patches.

thanks
-- PMM

Peter Maydell (13):
  target/arm: Use new CBAR encoding for all v8 CPUs, not all aarch64
    CPUs
  target/arm: The Cortex-R52 has a read-only CBAR
  target/arm: Add Cortex-R52 IMPDEF sysregs
  target/arm: Allow access to SPSR_hyp from hyp mode
  hw/misc/mps2-scc: Fix condition for CFG3 register
  hw/misc/mps2-scc: Factor out which-board conditionals
  hw/misc/mps2-scc: Make changes needed for AN536 FPGA image
  hw/arm/mps3r: Initial skeleton for mps3-an536 board
  hw/arm/mps3r: Add CPUs, GIC, and per-CPU RAM
  hw/arm/mps3r: Add UARTs
  hw/arm/mps3r: Add GPIO, watchdog, dual-timer, I2C devices
  hw/arm/mps3r: Add remaining devices
  docs: Add documentation for the mps3-an536 board

 MAINTAINERS                             |   3 +-
 docs/system/arm/mps2.rst                |  37 +-
 configs/devices/arm-softmmu/default.mak |   1 +
 include/hw/misc/mps2-scc.h              |   1 +
 hw/arm/mps3r.c                          | 640 ++++++++++++++++++++++++
 hw/misc/mps2-scc.c                      | 138 ++++-
 target/arm/helper.c                     |   2 +-
 target/arm/tcg/cpu32.c                  | 109 ++++
 target/arm/tcg/op_helper.c              |  43 +-
 target/arm/tcg/translate.c              |  19 +-
 hw/arm/Kconfig                          |   5 +
 hw/arm/meson.build                      |   1 +
 12 files changed, 955 insertions(+), 44 deletions(-)
 create mode 100644 hw/arm/mps3r.c

-- 
2.34.1

[PATCH 01/13] target/arm: Use new CBAR encoding for all v8 CPUs, not all aarch64 CPUs

[Peter Maydell]

We support two different encodings for the AArch32 IMPDEF
CBAR register -- older cores like the Cortex A9, A7, A15
have this at 4, c15, c0, 0; newer cores like the
Cortex A35, A53, A57 and A72 have it at 1 c15 c0 0.

When we implemented this we picked which encoding to
use based on whether the CPU set ARM_FEATURE_AARCH64.
However this isn't right for three cases:
 * the qemu-system-arm 'max' CPU, which is supposed to be
   a variant on a Cortex-A57; it ought to use the same
   encoding the A57 does and which the AArch64 'max'
   exposes to AArch32 guest code
 * the Cortex-R52, which is AArch32-only but has the CBAR
   at the newer encoding (and where we incorrectly are
   not yet setting ARM_FEATURE_CBAR_RO anyway)
 * any possible future support for other v8 AArch32
   only CPUs, or for supporting "boot the CPU into
   AArch32 mode" on our existing cores like the A57 etc

Make the decision of the encoding be based on whether
the CPU implements the ARM_FEATURE_V8 flag instead.

This changes the behaviour only for the qemu-system-arm
'-cpu max'. We don't expect anybody to be relying on the
old behaviour because:
 * it's not what the real hardware Cortex-A57 does
   (and that's what our ID register claims we are)
 * we don't implement the memory-mapped GICv3 support
   which is the only thing that exists at the peripheral
   base address pointed to by the register

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 945d8571a61..2a2659aade2 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9519,7 +9519,7 @@ void register_cp_regs_for_features(ARMCPU *cpu)
          * AArch64 cores we might need to add a specific feature flag
          * to indicate cores with "flavour 2" CBAR.
          */
-        if (arm_feature(env, ARM_FEATURE_AARCH64)) {
+        if (arm_feature(env, ARM_FEATURE_V8)) {
             /* 32 bit view is [31:18] 0...0 [43:32]. */
             uint32_t cbar32 = (extract64(cpu->reset_cbar, 18, 14) << 18)
                 | extract64(cpu->reset_cbar, 32, 12);
-- 
2.34.1

Re: [PATCH 01/13] target/arm: Use new CBAR encoding for all v8 CPUs, not all aarch64 CPUs

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> We support two different encodings for the AArch32 IMPDEF
> CBAR register -- older cores like the Cortex A9, A7, A15
> have this at 4, c15, c0, 0; newer cores like the
> Cortex A35, A53, A57 and A72 have it at 1 c15 c0 0.
> 
> When we implemented this we picked which encoding to
> use based on whether the CPU set ARM_FEATURE_AARCH64.
> However this isn't right for three cases:
>   * the qemu-system-arm 'max' CPU, which is supposed to be
>     a variant on a Cortex-A57; it ought to use the same
>     encoding the A57 does and which the AArch64 'max'
>     exposes to AArch32 guest code
>   * the Cortex-R52, which is AArch32-only but has the CBAR
>     at the newer encoding (and where we incorrectly are
>     not yet setting ARM_FEATURE_CBAR_RO anyway)
>   * any possible future support for other v8 AArch32
>     only CPUs, or for supporting "boot the CPU into
>     AArch32 mode" on our existing cores like the A57 etc
> 
> Make the decision of the encoding be based on whether
> the CPU implements the ARM_FEATURE_V8 flag instead.
> 
> This changes the behaviour only for the qemu-system-arm
> '-cpu max'. We don't expect anybody to be relying on the
> old behaviour because:
>   * it's not what the real hardware Cortex-A57 does
>     (and that's what our ID register claims we are)

Not even that, because max resets MIDR.

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 01/13] target/arm: Use new CBAR encoding for all v8 CPUs, not all aarch64 CPUs

[Peter Maydell]

On Tue, 6 Feb 2024 at 20:34, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/6/24 23:29, Peter Maydell wrote:
> > We support two different encodings for the AArch32 IMPDEF
> > CBAR register -- older cores like the Cortex A9, A7, A15
> > have this at 4, c15, c0, 0; newer cores like the
> > Cortex A35, A53, A57 and A72 have it at 1 c15 c0 0.
> >
> > When we implemented this we picked which encoding to
> > use based on whether the CPU set ARM_FEATURE_AARCH64.
> > However this isn't right for three cases:
> >   * the qemu-system-arm 'max' CPU, which is supposed to be
> >     a variant on a Cortex-A57; it ought to use the same
> >     encoding the A57 does and which the AArch64 'max'
> >     exposes to AArch32 guest code
> >   * the Cortex-R52, which is AArch32-only but has the CBAR
> >     at the newer encoding (and where we incorrectly are
> >     not yet setting ARM_FEATURE_CBAR_RO anyway)
> >   * any possible future support for other v8 AArch32
> >     only CPUs, or for supporting "boot the CPU into
> >     AArch32 mode" on our existing cores like the A57 etc
> >
> > Make the decision of the encoding be based on whether
> > the CPU implements the ARM_FEATURE_V8 flag instead.
> >
> > This changes the behaviour only for the qemu-system-arm
> > '-cpu max'. We don't expect anybody to be relying on the
> > old behaviour because:
> >   * it's not what the real hardware Cortex-A57 does
> >     (and that's what our ID register claims we are)
>
> Not even that, because max resets MIDR.

qemu-system-aarch64 max does (in aarch64_max_tcg_initfn(),
yes; but qemu-system-arm max is set up in arm_max_initfn()
in cpu32.c, and that sets cpu->midr = 0x411fd070 (which is
the same as A57's MIDR)...

> Anyway,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

thanks
-- PMM

[PATCH 02/13] target/arm: The Cortex-R52 has a read-only CBAR

[Peter Maydell]

The Cortex-R52 implements the Configuration Base Address Register
(CBAR), as a read-only register.  Add ARM_FEATURE_CBAR_RO to this CPU
type, so that our implementation provides the register and the
associated qdev property.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/cpu32.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
index 11253051156..311d654cdce 100644
--- a/target/arm/tcg/cpu32.c
+++ b/target/arm/tcg/cpu32.c
@@ -809,6 +809,7 @@ static void cortex_r52_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_PMSA);
     set_feature(&cpu->env, ARM_FEATURE_NEON);
     set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
+    set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
     cpu->midr = 0x411fd133; /* r1p3 */
     cpu->revidr = 0x00000000;
     cpu->reset_fpsid = 0x41034023;
-- 
2.34.1

Re: [PATCH 02/13] target/arm: The Cortex-R52 has a read-only CBAR

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> The Cortex-R52 implements the Configuration Base Address Register
> (CBAR), as a read-only register.  Add ARM_FEATURE_CBAR_RO to this CPU
> type, so that our implementation provides the register and the
> associated qdev property.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/tcg/cpu32.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
> index 11253051156..311d654cdce 100644
> --- a/target/arm/tcg/cpu32.c
> +++ b/target/arm/tcg/cpu32.c
> @@ -809,6 +809,7 @@ static void cortex_r52_initfn(Object *obj)
>       set_feature(&cpu->env, ARM_FEATURE_PMSA);
>       set_feature(&cpu->env, ARM_FEATURE_NEON);
>       set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> +    set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

I just noticed that arm_cpu_post_init can be simplified to not check CBAR_RO, now that we 
have arm_cpu_propagate_feature_implications.


r~

Re: [PATCH 02/13] target/arm: The Cortex-R52 has a read-only CBAR

[Peter Maydell]

On Tue, 6 Feb 2024 at 20:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 2/6/24 23:29, Peter Maydell wrote:
> > The Cortex-R52 implements the Configuration Base Address Register
> > (CBAR), as a read-only register.  Add ARM_FEATURE_CBAR_RO to this CPU
> > type, so that our implementation provides the register and the
> > associated qdev property.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   target/arm/tcg/cpu32.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
> > index 11253051156..311d654cdce 100644
> > --- a/target/arm/tcg/cpu32.c
> > +++ b/target/arm/tcg/cpu32.c
> > @@ -809,6 +809,7 @@ static void cortex_r52_initfn(Object *obj)
> >       set_feature(&cpu->env, ARM_FEATURE_PMSA);
> >       set_feature(&cpu->env, ARM_FEATURE_NEON);
> >       set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
> > +    set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> I just noticed that arm_cpu_post_init can be simplified to not check CBAR_RO, now that we
> have arm_cpu_propagate_feature_implications.

The other bit of CBAR cleanup I have is that cortex-a55, cortex-a76,
neoverse-n1, neoverse-v1, neoverse-v2 and cortex-a710 have all
cut-n-pasted the line that sets ARM_FEATURE_CBAR_RO, but none
of them actually have a CBAR according to their TRM. The only
reason I didn't throw in a patch fixing that is that I think
it would be a migration compat break and I didn't feel like
it was worth the effort to try to deal with that...

-- PMM

[PATCH 03/13] target/arm: Add Cortex-R52 IMPDEF sysregs

[Peter Maydell]

Add the Cortex-R52 IMPDEF sysregs, by defining them here and
also by enabling the AUXCR feature which defines the ACTLR
and HACTLR registers. As is our usual practice, we make these
simple reads-as-zero stubs for now.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/cpu32.c | 108 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/target/arm/tcg/cpu32.c b/target/arm/tcg/cpu32.c
index 311d654cdce..6eb08a41b01 100644
--- a/target/arm/tcg/cpu32.c
+++ b/target/arm/tcg/cpu32.c
@@ -800,6 +800,111 @@ static void cortex_r5_initfn(Object *obj)
     define_arm_cp_regs(cpu, cortexr5_cp_reginfo);
 }
 
+static const ARMCPRegInfo cortex_r52_cp_reginfo[] = {
+    { .name = "CPUACTLR", .cp = 15, .opc1 = 0, .crm = 15,
+      .access = PL1_RW, .type = ARM_CP_CONST | ARM_CP_64BIT, .resetvalue = 0 },
+    { .name = "IMP_ATCMREGIONR",
+      .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_BTCMREGIONR",
+      .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_CTCMREGIONR",
+      .cp = 15, .opc1 = 0, .crn = 9, .crm = 1, .opc2 = 2,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_CSCTLR",
+      .cp = 15, .opc1 = 1, .crn = 9, .crm = 1, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_BPCTLR",
+      .cp = 15, .opc1 = 1, .crn = 9, .crm = 1, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_MEMPROTCLR",
+      .cp = 15, .opc1 = 1, .crn = 9, .crm = 1, .opc2 = 2,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_SLAVEPCTLR",
+      .cp = 15, .opc1 = 0, .crn = 11, .crm = 0, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_PERIPHREGIONR",
+      .cp = 15, .opc1 = 0, .crn = 15, .crm = 0, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_FLASHIFREGIONR",
+      .cp = 15, .opc1 = 0, .crn = 15, .crm = 0, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_BUILDOPTR",
+      .cp = 15, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 0,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_PINOPTR",
+      .cp = 15, .opc1 = 0, .crn = 15, .crm = 2, .opc2 = 7,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_QOSR",
+      .cp = 15, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_BUSTIMEOUTR",
+      .cp = 15, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 2,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_INTMONR",
+      .cp = 15, .opc1 = 1, .crn = 15, .crm = 3, .opc2 = 4,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_ICERR0",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 0, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_ICERR1",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 0, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_DCERR0",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 1, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_DCERR1",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 1, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_TCMERR0",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 2, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_TCMERR1",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 2, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_TCMSYNDR0",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 2, .opc2 = 2,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_TCMSYNDR1",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 2, .opc2 = 3,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_FLASHERR0",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 3, .opc2 = 0,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_FLASHERR1",
+      .cp = 15, .opc1 = 2, .crn = 15, .crm = 3, .opc2 = 1,
+      .access = PL1_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_CDBGDR0",
+      .cp = 15, .opc1 = 3, .crn = 15, .crm = 0, .opc2 = 0,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_CBDGBR1",
+      .cp = 15, .opc1 = 3, .crn = 15, .crm = 0, .opc2 = 1,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_TESTR0",
+      .cp = 15, .opc1 = 4, .crn = 15, .crm = 0, .opc2 = 0,
+      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+    { .name = "IMP_TESTR1",
+      .cp = 15, .opc1 = 4, .crn = 15, .crm = 0, .opc2 = 1,
+      .access = PL1_W, .type = ARM_CP_NOP, .resetvalue = 0 },
+    { .name = "IMP_CDBGDCI",
+      .cp = 15, .opc1 = 0, .crn = 15, .crm = 15, .opc2 = 0,
+      .access = PL1_W, .type = ARM_CP_NOP, .resetvalue = 0 },
+    { .name = "IMP_CDBGDCT",
+      .cp = 15, .opc1 = 3, .crn = 15, .crm = 2, .opc2 = 0,
+      .access = PL1_W, .type = ARM_CP_NOP, .resetvalue = 0 },
+    { .name = "IMP_CDBGICT",
+      .cp = 15, .opc1 = 3, .crn = 15, .crm = 2, .opc2 = 1,
+      .access = PL1_W, .type = ARM_CP_NOP, .resetvalue = 0 },
+    { .name = "IMP_CDBGDCD",
+      .cp = 15, .opc1 = 3, .crn = 15, .crm = 4, .opc2 = 0,
+      .access = PL1_W, .type = ARM_CP_NOP, .resetvalue = 0 },
+    { .name = "IMP_CDBGICD",
+      .cp = 15, .opc1 = 3, .crn = 15, .crm = 4, .opc2 = 1,
+      .access = PL1_W, .type = ARM_CP_NOP, .resetvalue = 0 },
+};
+
+
 static void cortex_r52_initfn(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
@@ -810,6 +915,7 @@ static void cortex_r52_initfn(Object *obj)
     set_feature(&cpu->env, ARM_FEATURE_NEON);
     set_feature(&cpu->env, ARM_FEATURE_GENERIC_TIMER);
     set_feature(&cpu->env, ARM_FEATURE_CBAR_RO);
+    set_feature(&cpu->env, ARM_FEATURE_AUXCR);
     cpu->midr = 0x411fd133; /* r1p3 */
     cpu->revidr = 0x00000000;
     cpu->reset_fpsid = 0x41034023;
@@ -840,6 +946,8 @@ static void cortex_r52_initfn(Object *obj)
 
     cpu->pmsav7_dregion = 16;
     cpu->pmsav8r_hdregion = 16;
+
+    define_arm_cp_regs(cpu, cortex_r52_cp_reginfo);
 }
 
 static void cortex_r5f_initfn(Object *obj)
-- 
2.34.1

Re: [PATCH 03/13] target/arm: Add Cortex-R52 IMPDEF sysregs

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> Add the Cortex-R52 IMPDEF sysregs, by defining them here and
> also by enabling the AUXCR feature which defines the ACTLR
> and HACTLR registers. As is our usual practice, we make these
> simple reads-as-zero stubs for now.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/tcg/cpu32.c | 108 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 108 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 04/13] target/arm: Allow access to SPSR_hyp from hyp mode

[Peter Maydell]

Architecturally, the AArch32 MSR/MRS to/from banked register
instructions are UNPREDICTABLE for attempts to access a banked
register that the guest could access in a more direct way (e.g.
using this insn to access r8_fiq when already in FIQ mode).  QEMU has
chosen to UNDEF on all of these.

However, for the case of accessing SPSR_hyp from hyp mode, it turns
out that real hardware permits this, with the same effect as if the
guest had directly written to SPSR. Further, there is some
guest code out there that assumes it can do this, because it
happens to work on hardware: an example Cortex-R52 startup code
fragment uses this, and it got copied into various other places,
including Zephyr. Zephyr was fixed to not use this:
 https://github.com/zephyrproject-rtos/zephyr/issues/47330
but other examples are still out there, like the selftest
binary for the MPS3-AN536.

For convenience of being able to run guest code, permit
this UNPREDICTABLE access instead of UNDEFing it.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Last time this came up I preferred the "keep QEMU behaviour
as it is, try to get the guest code fixed" approach:
 https://www.mail-archive.com/qemu-devel@nongnu.org/msg899970.html
but as this is the second time I lean a bit more towards
behaving like the hardware.
---
 target/arm/tcg/op_helper.c | 43 ++++++++++++++++++++++++++------------
 target/arm/tcg/translate.c | 19 +++++++++++------
 2 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/target/arm/tcg/op_helper.c b/target/arm/tcg/op_helper.c
index b5ac26061c7..c199b69fbff 100644
--- a/target/arm/tcg/op_helper.c
+++ b/target/arm/tcg/op_helper.c
@@ -570,10 +570,24 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
      */
     int curmode = env->uncached_cpsr & CPSR_M;
 
-    if (regno == 17) {
-        /* ELR_Hyp: a special case because access from tgtmode is OK */
-        if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
-            goto undef;
+    if (tgtmode == ARM_CPU_MODE_HYP) {
+        /*
+         * Handle Hyp target regs first because some are special cases
+         * which don't want the usual "not accessible from tgtmode" check.
+         */
+        switch (regno) {
+        case 16 ... 17: /* ELR_Hyp, SPSR_Hyp */
+            if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
+                goto undef;
+            }
+            break;
+        case 13:
+            if (curmode != ARM_CPU_MODE_MON) {
+                goto undef;
+            }
+            break;
+        default:
+            g_assert_not_reached();
         }
         return;
     }
@@ -604,13 +618,6 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
         }
     }
 
-    if (tgtmode == ARM_CPU_MODE_HYP) {
-        /* SPSR_Hyp, r13_hyp: accessible from Monitor mode only */
-        if (curmode != ARM_CPU_MODE_MON) {
-            goto undef;
-        }
-    }
-
     return;
 
 undef:
@@ -625,7 +632,12 @@ void HELPER(msr_banked)(CPUARMState *env, uint32_t value, uint32_t tgtmode,
 
     switch (regno) {
     case 16: /* SPSRs */
-        env->banked_spsr[bank_number(tgtmode)] = value;
+        if (tgtmode == (env->uncached_cpsr & CPSR_M)) {
+            /* Only happens for SPSR_Hyp access in Hyp mode */
+            env->spsr = value;
+        } else {
+            env->banked_spsr[bank_number(tgtmode)] = value;
+        }
         break;
     case 17: /* ELR_Hyp */
         env->elr_el[2] = value;
@@ -659,7 +671,12 @@ uint32_t HELPER(mrs_banked)(CPUARMState *env, uint32_t tgtmode, uint32_t regno)
 
     switch (regno) {
     case 16: /* SPSRs */
-        return env->banked_spsr[bank_number(tgtmode)];
+        if (tgtmode == (env->uncached_cpsr & CPSR_M)) {
+            /* Only happens for SPSR_Hyp access in Hyp mode */
+            return env->spsr;
+        } else {
+            return env->banked_spsr[bank_number(tgtmode)];
+        }
     case 17: /* ELR_Hyp */
         return env->elr_el[2];
     case 13:
diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c
index 5fa82497238..f947c62c6be 100644
--- a/target/arm/tcg/translate.c
+++ b/target/arm/tcg/translate.c
@@ -2822,13 +2822,20 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
         break;
     case ARM_CPU_MODE_HYP:
         /*
-         * SPSR_hyp and r13_hyp can only be accessed from Monitor mode
-         * (and so we can forbid accesses from EL2 or below). elr_hyp
-         * can be accessed also from Hyp mode, so forbid accesses from
-         * EL0 or EL1.
+         * r13_hyp can only be accessed from Monitor mode, and so we
+         * can forbid accesses from EL2 or below.
+         * elr_hyp can be accessed also from Hyp mode, so forbid
+         * accesses from EL0 or EL1.
+         * SPSR_hyp is supposed to be in the same category as r13_hyp
+         * and UNPREDICTABLE if accessed from anything except Monitor
+         * mode. However there is some real-world code that will do
+         * it because at least some hardware happens to permit the
+         * access. (Notably a standard Cortex-R52 startup code fragment
+         * does this.) So we permit SPSR_hyp from Hyp mode also, to allow
+         * this (incorrect) guest code to run.
          */
-        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 2 ||
-            (s->current_el < 3 && *regno != 17)) {
+        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 2
+            || (s->current_el < 3 && *regno != 16 && *regno != 17)) {
             goto undef;
         }
         break;
-- 
2.34.1

Re: [PATCH 04/13] target/arm: Allow access to SPSR_hyp from hyp mode

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> Architecturally, the AArch32 MSR/MRS to/from banked register
> instructions are UNPREDICTABLE for attempts to access a banked
> register that the guest could access in a more direct way (e.g.
> using this insn to access r8_fiq when already in FIQ mode).  QEMU has
> chosen to UNDEF on all of these.
> 
> However, for the case of accessing SPSR_hyp from hyp mode, it turns
> out that real hardware permits this, with the same effect as if the
> guest had directly written to SPSR. Further, there is some
> guest code out there that assumes it can do this, because it
> happens to work on hardware: an example Cortex-R52 startup code
> fragment uses this, and it got copied into various other places,
> including Zephyr. Zephyr was fixed to not use this:
>   https://github.com/zephyrproject-rtos/zephyr/issues/47330
> but other examples are still out there, like the selftest
> binary for the MPS3-AN536.
> 
> For convenience of being able to run guest code, permit
> this UNPREDICTABLE access instead of UNDEFing it.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
> Last time this came up I preferred the "keep QEMU behaviour
> as it is, try to get the guest code fixed" approach:
>   https://www.mail-archive.com/qemu-devel@nongnu.org/msg899970.html
> but as this is the second time I lean a bit more towards
> behaving like the hardware.
> ---
>   target/arm/tcg/op_helper.c | 43 ++++++++++++++++++++++++++------------
>   target/arm/tcg/translate.c | 19 +++++++++++------
>   2 files changed, 43 insertions(+), 19 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 05/13] hw/misc/mps2-scc: Fix condition for CFG3 register

[Peter Maydell]

We currently guard the CFG3 register read with
 (scc_partno(s) == 0x524 && scc_partno(s) == 0x547)
which is clearly wrong as it is never true.

This register is present on all board types except AN524
and AN527; correct the condition.

Fixes: 6ac80818941829c0 ("hw/misc/mps2-scc: Implement changes for AN547")
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/mps2-scc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
index 6cfb5ff1086..6c1b1cd3795 100644
--- a/hw/misc/mps2-scc.c
+++ b/hw/misc/mps2-scc.c
@@ -118,7 +118,7 @@ static uint64_t mps2_scc_read(void *opaque, hwaddr offset, unsigned size)
         r = s->cfg2;
         break;
     case A_CFG3:
-        if (scc_partno(s) == 0x524 && scc_partno(s) == 0x547) {
+        if (scc_partno(s) == 0x524 || scc_partno(s) == 0x547) {
             /* CFG3 reserved on AN524 */
             goto bad_offset;
         }
-- 
2.34.1

Re: [PATCH 05/13] hw/misc/mps2-scc: Fix condition for CFG3 register

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> We currently guard the CFG3 register read with
>   (scc_partno(s) == 0x524 && scc_partno(s) == 0x547)
> which is clearly wrong as it is never true.
> 
> This register is present on all board types except AN524
> and AN527; correct the condition.
> 
> Fixes: 6ac80818941829c0 ("hw/misc/mps2-scc: Implement changes for AN547")

Oops.
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mps2-scc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Re: [PATCH 05/13] hw/misc/mps2-scc: Fix condition for CFG3 register

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> We currently guard the CFG3 register read with
>   (scc_partno(s) == 0x524 && scc_partno(s) == 0x547)
> which is clearly wrong as it is never true.
> 
> This register is present on all board types except AN524
> and AN527; correct the condition.
> 
> Fixes: 6ac80818941829c0 ("hw/misc/mps2-scc: Implement changes for AN547")
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/misc/mps2-scc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 06/13] hw/misc/mps2-scc: Factor out which-board conditionals

[Peter Maydell]

The MPS SCC device has a lot of different flavours for the various
different MPS FPGA images, which look mostly similar but have
differences in how particular registers are handled.  Currently we
deal with this with a lot of open-coded checks on scc_partno(), but
as we add more board types this is getting a bit hard to read.

Factor out the conditions into some functions which we can
give more descriptive names to.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/misc/mps2-scc.c | 45 +++++++++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
index 6c1b1cd3795..02a80bacd71 100644
--- a/hw/misc/mps2-scc.c
+++ b/hw/misc/mps2-scc.c
@@ -59,6 +59,30 @@ static int scc_partno(MPS2SCC *s)
     return extract32(s->id, 4, 8);
 }
 
+/* Is CFG_REG2 present? */
+static bool have_cfg2(MPS2SCC *s)
+{
+    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
+}
+
+/* Is CFG_REG3 present? */
+static bool have_cfg3(MPS2SCC *s)
+{
+    return scc_partno(s) != 0x524 && scc_partno(s) != 0x547;
+}
+
+/* Is CFG_REG5 present? */
+static bool have_cfg5(MPS2SCC *s)
+{
+    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
+}
+
+/* Is CFG_REG6 present? */
+static bool have_cfg6(MPS2SCC *s)
+{
+    return scc_partno(s) == 0x524;
+}
+
 /* Handle a write via the SYS_CFG channel to the specified function/device.
  * Return false on error (reported to guest via SYS_CFGCTRL ERROR bit).
  */
@@ -111,15 +135,13 @@ static uint64_t mps2_scc_read(void *opaque, hwaddr offset, unsigned size)
         r = s->cfg1;
         break;
     case A_CFG2:
-        if (scc_partno(s) != 0x524 && scc_partno(s) != 0x547) {
-            /* CFG2 reserved on other boards */
+        if (!have_cfg2(s)) {
             goto bad_offset;
         }
         r = s->cfg2;
         break;
     case A_CFG3:
-        if (scc_partno(s) == 0x524 || scc_partno(s) == 0x547) {
-            /* CFG3 reserved on AN524 */
+        if (!have_cfg3(s)) {
             goto bad_offset;
         }
         /* These are user-settable DIP switches on the board. We don't
@@ -131,15 +153,13 @@ static uint64_t mps2_scc_read(void *opaque, hwaddr offset, unsigned size)
         r = s->cfg4;
         break;
     case A_CFG5:
-        if (scc_partno(s) != 0x524 && scc_partno(s) != 0x547) {
-            /* CFG5 reserved on other boards */
+        if (!have_cfg5(s)) {
             goto bad_offset;
         }
         r = s->cfg5;
         break;
     case A_CFG6:
-        if (scc_partno(s) != 0x524) {
-            /* CFG6 reserved on other boards */
+        if (!have_cfg6(s)) {
             goto bad_offset;
         }
         r = s->cfg6;
@@ -202,24 +222,21 @@ static void mps2_scc_write(void *opaque, hwaddr offset, uint64_t value,
         }
         break;
     case A_CFG2:
-        if (scc_partno(s) != 0x524 && scc_partno(s) != 0x547) {
-            /* CFG2 reserved on other boards */
+        if (!have_cfg2(s)) {
             goto bad_offset;
         }
         /* AN524: QSPI Select signal */
         s->cfg2 = value;
         break;
     case A_CFG5:
-        if (scc_partno(s) != 0x524 && scc_partno(s) != 0x547) {
-            /* CFG5 reserved on other boards */
+        if (!have_cfg5(s)) {
             goto bad_offset;
         }
         /* AN524: ACLK frequency in Hz */
         s->cfg5 = value;
         break;
     case A_CFG6:
-        if (scc_partno(s) != 0x524) {
-            /* CFG6 reserved on other boards */
+        if (!have_cfg6(s)) {
             goto bad_offset;
         }
         /* AN524: Clock divider for BRAM */
-- 
2.34.1

Re: [PATCH 06/13] hw/misc/mps2-scc: Factor out which-board conditionals

[Philippe Mathieu-Daudé]

Hi Peter,

On 6/2/24 14:29, Peter Maydell wrote:
> The MPS SCC device has a lot of different flavours for the various
> different MPS FPGA images, which look mostly similar but have
> differences in how particular registers are handled.  Currently we
> deal with this with a lot of open-coded checks on scc_partno(), but
> as we add more board types this is getting a bit hard to read.
> 
> Factor out the conditions into some functions which we can
> give more descriptive names to.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mps2-scc.c | 45 +++++++++++++++++++++++++++++++--------------
>   1 file changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
> index 6c1b1cd3795..02a80bacd71 100644
> --- a/hw/misc/mps2-scc.c
> +++ b/hw/misc/mps2-scc.c
> @@ -59,6 +59,30 @@ static int scc_partno(MPS2SCC *s)
>       return extract32(s->id, 4, 8);
>   }
>   
> +/* Is CFG_REG2 present? */
> +static bool have_cfg2(MPS2SCC *s)
> +{
> +    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
> +}
> +
> +/* Is CFG_REG3 present? */
> +static bool have_cfg3(MPS2SCC *s)
> +{
> +    return scc_partno(s) != 0x524 && scc_partno(s) != 0x547;
> +}
> +
> +/* Is CFG_REG5 present? */
> +static bool have_cfg5(MPS2SCC *s)
> +{
> +    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
> +}
> +
> +/* Is CFG_REG6 present? */
> +static bool have_cfg6(MPS2SCC *s)
> +{
> +    return scc_partno(s) == 0x524;
> +}

I'd rather QOM-decline TYPE_MPS2_SCC per board and have
MPS2SCCClass::have_cfgX fields set in each class_init.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 06/13] hw/misc/mps2-scc: Factor out which-board conditionals

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> The MPS SCC device has a lot of different flavours for the various
> different MPS FPGA images, which look mostly similar but have
> differences in how particular registers are handled.  Currently we
> deal with this with a lot of open-coded checks on scc_partno(), but
> as we add more board types this is getting a bit hard to read.
> 
> Factor out the conditions into some functions which we can
> give more descriptive names to.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/misc/mps2-scc.c | 45 +++++++++++++++++++++++++++++++--------------
>   1 file changed, 31 insertions(+), 14 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 06/13] hw/misc/mps2-scc: Factor out which-board conditionals

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> The MPS SCC device has a lot of different flavours for the various
> different MPS FPGA images, which look mostly similar but have
> differences in how particular registers are handled.  Currently we
> deal with this with a lot of open-coded checks on scc_partno(), but
> as we add more board types this is getting a bit hard to read.
> 
> Factor out the conditions into some functions which we can
> give more descriptive names to.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/misc/mps2-scc.c | 45 +++++++++++++++++++++++++++++++--------------
>   1 file changed, 31 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
> index 6c1b1cd3795..02a80bacd71 100644
> --- a/hw/misc/mps2-scc.c
> +++ b/hw/misc/mps2-scc.c
> @@ -59,6 +59,30 @@ static int scc_partno(MPS2SCC *s)
>       return extract32(s->id, 4, 8);
>   }
>   
> +/* Is CFG_REG2 present? */
> +static bool have_cfg2(MPS2SCC *s)
> +{
> +    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
> +}
> +
> +/* Is CFG_REG3 present? */
> +static bool have_cfg3(MPS2SCC *s)
> +{
> +    return scc_partno(s) != 0x524 && scc_partno(s) != 0x547;
> +}

Looking at next patch and nitpicking, if we want to avoid
a switch() case use, this style is easier to review IMHO:

    static bool have_cfg2(MPS2SCC *s)
    {
        return    scc_partno(s) == 0x524
               || scc_partno(s) == 0x547;
    }

Anyway, already R-b.

[PATCH 07/13] hw/misc/mps2-scc: Make changes needed for AN536 FPGA image

[Peter Maydell]

The MPS2 SCC device is broadly the same for all FPGA images, but has
minor differences in the behaviour of the CFG registers depending on
the image. In many cases we don't really care about the functionality
controlled by these registers and a reads-as-written or similar
behaviour is sufficient for the moment.

For the AN536 the required behaviour is:

 * A_CFG0 has CPU reset and halt bits
    - implement as reads-as-written for the moment
 * A_CFG1 has flash or ATCM address 0 remap handling
    - QEMU doesn't model this; implement as reads-as-written
 * A_CFG2 has QSPI select (like AN524)
    - implemented (no behaviour, as with AN524)
 * A_CFG3 is MCC_MSB_ADDR "additional MCC addressing bits"
    - QEMU doesn't care about these, so use the existing
      RAZ behaviour for convenience
 * A_CFG4 is board rev (like all other images)
    - no change needed
 * A_CFG5 is ACLK frq in hz (like AN524)
    - implemented as reads-as-written, as for other boards
 * A_CFG6 is core 0 vector table base address
    - implemented as reads-as-written for the moment
 * A_CFG7 is core 1 vector table base address
    - implemented as reads-as-written for the moment

Make the changes necessary for this; leave TODO comments where
appropriate to indicate where we might want to come back and
implement things like CPU reset.

The other aspects of the device specific to this FPGA image (like the
values of the board ID and similar registers) will be set via the
device's qdev properties.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/misc/mps2-scc.h |   1 +
 hw/misc/mps2-scc.c         | 101 +++++++++++++++++++++++++++++++++----
 2 files changed, 92 insertions(+), 10 deletions(-)

diff --git a/include/hw/misc/mps2-scc.h b/include/hw/misc/mps2-scc.h
index 3b2d13ac9c3..8ff188c06b1 100644
--- a/include/hw/misc/mps2-scc.h
+++ b/include/hw/misc/mps2-scc.h
@@ -51,6 +51,7 @@ struct MPS2SCC {
     uint32_t cfg4;
     uint32_t cfg5;
     uint32_t cfg6;
+    uint32_t cfg7;
     uint32_t cfgdata_rtn;
     uint32_t cfgdata_out;
     uint32_t cfgctrl;
diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
index 02a80bacd71..18be74157ee 100644
--- a/hw/misc/mps2-scc.c
+++ b/hw/misc/mps2-scc.c
@@ -37,6 +37,7 @@ REG32(CFG3, 0xc)
 REG32(CFG4, 0x10)
 REG32(CFG5, 0x14)
 REG32(CFG6, 0x18)
+REG32(CFG7, 0x1c)
 REG32(CFGDATA_RTN, 0xa0)
 REG32(CFGDATA_OUT, 0xa4)
 REG32(CFGCTRL, 0xa8)
@@ -62,25 +63,46 @@ static int scc_partno(MPS2SCC *s)
 /* Is CFG_REG2 present? */
 static bool have_cfg2(MPS2SCC *s)
 {
-    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
+    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547 ||
+        scc_partno(s) == 0x536;
 }
 
 /* Is CFG_REG3 present? */
 static bool have_cfg3(MPS2SCC *s)
 {
-    return scc_partno(s) != 0x524 && scc_partno(s) != 0x547;
+    return scc_partno(s) != 0x524 && scc_partno(s) != 0x547 &&
+        scc_partno(s) != 0x536;
 }
 
 /* Is CFG_REG5 present? */
 static bool have_cfg5(MPS2SCC *s)
 {
-    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547;
+    return scc_partno(s) == 0x524 || scc_partno(s) == 0x547 ||
+        scc_partno(s) == 0x536;
 }
 
 /* Is CFG_REG6 present? */
 static bool have_cfg6(MPS2SCC *s)
 {
-    return scc_partno(s) == 0x524;
+    return scc_partno(s) == 0x524 || scc_partno(s) == 0x536;
+}
+
+/* Is CFG_REG7 present? */
+static bool have_cfg7(MPS2SCC *s)
+{
+    return scc_partno(s) == 0x536;
+}
+
+/* Does CFG_REG0 drive the 'remap' GPIO output? */
+static bool cfg0_is_remap(MPS2SCC *s)
+{
+    return scc_partno(s) != 0x536;
+}
+
+/* Is CFG_REG1 driving a set of LEDs? */
+static bool cfg1_is_leds(MPS2SCC *s)
+{
+    return scc_partno(s) != 0x536;
 }
 
 /* Handle a write via the SYS_CFG channel to the specified function/device.
@@ -144,8 +166,16 @@ static uint64_t mps2_scc_read(void *opaque, hwaddr offset, unsigned size)
         if (!have_cfg3(s)) {
             goto bad_offset;
         }
-        /* These are user-settable DIP switches on the board. We don't
+        /*
+         * These are user-settable DIP switches on the board. We don't
          * model that, so just return zeroes.
+         *
+         * TODO: for AN536 this is MCC_MSB_ADDR "additional MCC addressing
+         * bits". These change which part of the DDR4 the motherboard
+         * configuration controller can see in its memory map (see the
+         * appnote section 2.4). QEMU doesn't model the MCC at all, so these
+         * bits are not interesting to us; read-as-zero is as good as anything
+         * else.
          */
         r = 0;
         break;
@@ -164,6 +194,12 @@ static uint64_t mps2_scc_read(void *opaque, hwaddr offset, unsigned size)
         }
         r = s->cfg6;
         break;
+    case A_CFG7:
+        if (!have_cfg7(s)) {
+            goto bad_offset;
+        }
+        r = s->cfg7;
+        break;
     case A_CFGDATA_RTN:
         r = s->cfgdata_rtn;
         break;
@@ -211,28 +247,43 @@ static void mps2_scc_write(void *opaque, hwaddr offset, uint64_t value,
          * we always reflect bit 0 in the 'remap' GPIO output line,
          * and let the board wire it up or not as it chooses.
          * TODO on some boards bit 1 is CPU_WAIT.
+         *
+         * TODO: on the AN536 this register controls reset and halt
+         * for both CPUs. For the moment we don't implement this, so the
+         * register just reads as written.
          */
         s->cfg0 = value;
-        qemu_set_irq(s->remap, s->cfg0 & 1);
+        if (cfg0_is_remap(s)) {
+            qemu_set_irq(s->remap, s->cfg0 & 1);
+        }
         break;
     case A_CFG1:
         s->cfg1 = value;
-        for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
-            led_set_state(s->led[i], extract32(value, i, 1));
+        /*
+         * On most boards this register drives LEDs.
+         *
+         * TODO: for AN536 this controls whether flash and ATCM are
+         * enabled or disabled on reset. QEMU doesn't model this, and
+         * always wires up RAM in the ATCM area and ROM in the flash area.
+         */
+        if (cfg1_is_leds(s)) {
+            for (size_t i = 0; i < ARRAY_SIZE(s->led); i++) {
+                led_set_state(s->led[i], extract32(value, i, 1));
+            }
         }
         break;
     case A_CFG2:
         if (!have_cfg2(s)) {
             goto bad_offset;
         }
-        /* AN524: QSPI Select signal */
+        /* AN524, AN536: QSPI Select signal */
         s->cfg2 = value;
         break;
     case A_CFG5:
         if (!have_cfg5(s)) {
             goto bad_offset;
         }
-        /* AN524: ACLK frequency in Hz */
+        /* AN524, AN536: ACLK frequency in Hz */
         s->cfg5 = value;
         break;
     case A_CFG6:
@@ -240,6 +291,14 @@ static void mps2_scc_write(void *opaque, hwaddr offset, uint64_t value,
             goto bad_offset;
         }
         /* AN524: Clock divider for BRAM */
+        /* AN536: Core 0 vector table base address */
+        s->cfg6 = value;
+        break;
+    case A_CFG7:
+        if (!have_cfg7(s)) {
+            goto bad_offset;
+        }
+        /* AN536: Core 1 vector table base address */
         s->cfg6 = value;
         break;
     case A_CFGDATA_OUT:
@@ -353,6 +412,24 @@ static void mps2_scc_finalize(Object *obj)
     g_free(s->oscclk_reset);
 }
 
+static bool cfg7_needed(void *opaque)
+{
+    MPS2SCC *s = opaque;
+
+    return have_cfg7(s);
+}
+
+static const VMStateDescription vmstate_cfg7 = {
+    .name = "mps2-scc/cfg7",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = cfg7_needed,
+    .fields = (const VMStateField[]) {
+        VMSTATE_UINT32(cfg7, MPS2SCC),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const VMStateDescription mps2_scc_vmstate = {
     .name = "mps2-scc",
     .version_id = 3,
@@ -372,6 +449,10 @@ static const VMStateDescription mps2_scc_vmstate = {
         VMSTATE_VARRAY_UINT32(oscclk, MPS2SCC, num_oscclk,
                               0, vmstate_info_uint32, uint32_t),
         VMSTATE_END_OF_LIST()
+    },
+    .subsections = (const VMStateDescription * const []) {
+        &vmstate_cfg7,
+        NULL
     }
 };
 
-- 
2.34.1

Re: [PATCH 07/13] hw/misc/mps2-scc: Make changes needed for AN536 FPGA image

[Richard Henderson]

On 2/6/24 23:29, Peter Maydell wrote:
> The MPS2 SCC device is broadly the same for all FPGA images, but has
> minor differences in the behaviour of the CFG registers depending on
> the image. In many cases we don't really care about the functionality
> controlled by these registers and a reads-as-written or similar
> behaviour is sufficient for the moment.
> 
> For the AN536 the required behaviour is:
> 
>   * A_CFG0 has CPU reset and halt bits
>      - implement as reads-as-written for the moment
>   * A_CFG1 has flash or ATCM address 0 remap handling
>      - QEMU doesn't model this; implement as reads-as-written
>   * A_CFG2 has QSPI select (like AN524)
>      - implemented (no behaviour, as with AN524)
>   * A_CFG3 is MCC_MSB_ADDR "additional MCC addressing bits"
>      - QEMU doesn't care about these, so use the existing
>        RAZ behaviour for convenience
>   * A_CFG4 is board rev (like all other images)
>      - no change needed
>   * A_CFG5 is ACLK frq in hz (like AN524)
>      - implemented as reads-as-written, as for other boards
>   * A_CFG6 is core 0 vector table base address
>      - implemented as reads-as-written for the moment
>   * A_CFG7 is core 1 vector table base address
>      - implemented as reads-as-written for the moment
> 
> Make the changes necessary for this; leave TODO comments where
> appropriate to indicate where we might want to come back and
> implement things like CPU reset.
> 
> The other aspects of the device specific to this FPGA image (like the
> values of the board ID and similar registers) will be set via the
> device's qdev properties.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   include/hw/misc/mps2-scc.h |   1 +
>   hw/misc/mps2-scc.c         | 101 +++++++++++++++++++++++++++++++++----
>   2 files changed, 92 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 07/13] hw/misc/mps2-scc: Make changes needed for AN536 FPGA image

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> The MPS2 SCC device is broadly the same for all FPGA images, but has
> minor differences in the behaviour of the CFG registers depending on
> the image. In many cases we don't really care about the functionality
> controlled by these registers and a reads-as-written or similar
> behaviour is sufficient for the moment.
> 
> For the AN536 the required behaviour is:
> 
>   * A_CFG0 has CPU reset and halt bits
>      - implement as reads-as-written for the moment
>   * A_CFG1 has flash or ATCM address 0 remap handling
>      - QEMU doesn't model this; implement as reads-as-written
>   * A_CFG2 has QSPI select (like AN524)
>      - implemented (no behaviour, as with AN524)
>   * A_CFG3 is MCC_MSB_ADDR "additional MCC addressing bits"
>      - QEMU doesn't care about these, so use the existing
>        RAZ behaviour for convenience
>   * A_CFG4 is board rev (like all other images)
>      - no change needed
>   * A_CFG5 is ACLK frq in hz (like AN524)
>      - implemented as reads-as-written, as for other boards
>   * A_CFG6 is core 0 vector table base address
>      - implemented as reads-as-written for the moment
>   * A_CFG7 is core 1 vector table base address
>      - implemented as reads-as-written for the moment
> 
> Make the changes necessary for this; leave TODO comments where
> appropriate to indicate where we might want to come back and
> implement things like CPU reset.
> 
> The other aspects of the device specific to this FPGA image (like the
> values of the board ID and similar registers) will be set via the
> device's qdev properties.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/misc/mps2-scc.h |   1 +
>   hw/misc/mps2-scc.c         | 101 +++++++++++++++++++++++++++++++++----
>   2 files changed, 92 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Peter Maydell]

The AN536 is another FPGA image for the MPS3 development board. Unlike
the existing FPGA images we already model, this board uses a Cortex-R
family CPU, and it does not use any equivalent to the M-profile
"Subsystem for Embedded" SoC-equivalent that we model in hw/arm/armsse.c.
It's therefore more convenient for us to model it as a completely
separate C file.

This commit adds the basic skeleton of the board model, and the
code to create all the RAM and ROM. We assume that we're probably
going to want to add more images in future, so use the same
base class/subclass setup that mps2-tz.c uses, even though at
the moment there's only a single subclass.

Following commits will add the CPUs and the peripherals.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 MAINTAINERS                             |   3 +-
 configs/devices/arm-softmmu/default.mak |   1 +
 hw/arm/mps3r.c                          | 239 ++++++++++++++++++++++++
 hw/arm/Kconfig                          |   5 +
 hw/arm/meson.build                      |   1 +
 5 files changed, 248 insertions(+), 1 deletion(-)
 create mode 100644 hw/arm/mps3r.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f9741b898e..8219ed9068c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -819,12 +819,13 @@ F: include/hw/misc/imx7_*.h
 F: hw/pci-host/designware.c
 F: include/hw/pci-host/designware.h
 
-MPS2
+MPS2 / MPS3
 M: Peter Maydell <peter.maydell@linaro.org>
 L: qemu-arm@nongnu.org
 S: Maintained
 F: hw/arm/mps2.c
 F: hw/arm/mps2-tz.c
+F: hw/arm/mps3r.c
 F: hw/misc/mps2-*.c
 F: include/hw/misc/mps2-*.h
 F: hw/arm/armsse.c
diff --git a/configs/devices/arm-softmmu/default.mak b/configs/devices/arm-softmmu/default.mak
index 023faa2f750..6ee31bc1ab9 100644
--- a/configs/devices/arm-softmmu/default.mak
+++ b/configs/devices/arm-softmmu/default.mak
@@ -13,6 +13,7 @@ CONFIG_ARM_VIRT=y
 # CONFIG_INTEGRATOR=n
 # CONFIG_FSL_IMX31=n
 # CONFIG_MUSICPAL=n
+# CONFIG_MPS3R=n
 # CONFIG_MUSCA=n
 # CONFIG_CHEETAH=n
 # CONFIG_SX1=n
diff --git a/hw/arm/mps3r.c b/hw/arm/mps3r.c
new file mode 100644
index 00000000000..888a846d23c
--- /dev/null
+++ b/hw/arm/mps3r.c
@@ -0,0 +1,239 @@
+/*
+ * Arm MPS3 board emulation for Cortex-R-based FPGA images.
+ * (For M-profile images see mps2.c and mps2tz.c.)
+ *
+ * Copyright (c) 2017 Linaro Limited
+ * Written by Peter Maydell
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 or
+ *  (at your option) any later version.
+ */
+
+/*
+ * The MPS3 is an FPGA based dev board. This file handles FPGA images
+ * which use the Cortex-R CPUs. We model these separately from the
+ * M-profile images, because on M-profile the FPGA image is based on
+ * a "Subsystem for Embedded" which is similar to an SoC, whereas
+ * the R-profile FPGA images don't have that abstraction layer.
+ *
+ * We model the following FPGA images here:
+ *  "mps3-an536" -- dual Cortex-R52 as documented in Arm Application Note AN536
+ *
+ * Application Note AN536:
+ * https://developer.arm.com/documentation/dai0536/latest/
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qapi/error.h"
+#include "exec/address-spaces.h"
+#include "cpu.h"
+#include "hw/boards.h"
+#include "hw/arm/boot.h"
+
+/* Define the layout of RAM and ROM in a board */
+typedef struct RAMInfo {
+    const char *name;
+    hwaddr base;
+    hwaddr size;
+    int mrindex; /* index into rams[]; -1 for the system RAM block */
+    int flags;
+} RAMInfo;
+
+/*
+ * The MPS3 DDR is 3GiB, but on a 32-bit host QEMU doesn't permit
+ * emulation of that much guest RAM, so artificially make it smaller.
+ */
+#if HOST_LONG_BITS == 32
+#define MPS3_DDR_SIZE (1 * GiB)
+#else
+#define MPS3_DDR_SIZE (3 * GiB)
+#endif
+
+/*
+ * Flag values:
+ * IS_MAIN: this is the main machine RAM
+ * IS_ROM: this area is read-only
+ */
+#define IS_MAIN 1
+#define IS_ROM 2
+
+#define MPS3R_RAM_MAX 9
+
+typedef enum MPS3RFPGAType {
+    FPGA_AN536,
+} MPS3RFPGAType;
+
+struct MPS3RMachineClass {
+    MachineClass parent;
+    MPS3RFPGAType fpga_type;
+    const RAMInfo *raminfo;
+};
+
+struct MPS3RMachineState {
+    MachineState parent;
+    MemoryRegion ram[MPS3R_RAM_MAX];
+};
+
+#define TYPE_MPS3R_MACHINE "mps3r"
+#define TYPE_MPS3R_AN536_MACHINE MACHINE_TYPE_NAME("mps3-an536")
+
+OBJECT_DECLARE_TYPE(MPS3RMachineState, MPS3RMachineClass, MPS3R_MACHINE)
+
+static const RAMInfo an536_raminfo[] = {
+    {
+        .name = "ATCM",
+        .base = 0x00000000,
+        .size = 0x00008000,
+        .mrindex = 0,
+    }, {
+        /* We model the QSPI flash as simple ROM for now */
+        .name = "QSPI",
+        .base = 0x08000000,
+        .size = 0x00800000,
+        .flags = IS_ROM,
+        .mrindex = 1,
+    }, {
+        .name = "BRAM",
+        .base = 0x10000000,
+        .size = 0x00080000,
+        .mrindex = 2,
+    }, {
+        .name = "DDR",
+        .base = 0x20000000,
+        .size = MPS3_DDR_SIZE,
+        .mrindex = -1,
+    }, {
+        .name = "ATCM0",
+        .base = 0xee000000,
+        .size = 0x00008000,
+        .mrindex = 3,
+    }, {
+        .name = "BTCM0",
+        .base = 0xee100000,
+        .size = 0x00008000,
+        .mrindex = 4,
+    }, {
+        .name = "CTCM0",
+        .base = 0xee200000,
+        .size = 0x00008000,
+        .mrindex = 5,
+    }, {
+        .name = "ATCM1",
+        .base = 0xee400000,
+        .size = 0x00008000,
+        .mrindex = 6,
+    }, {
+        .name = "BTCM1",
+        .base = 0xee500000,
+        .size = 0x00008000,
+        .mrindex = 7,
+    }, {
+        .name = "CTCM1",
+        .base = 0xee600000,
+        .size = 0x00008000,
+        .mrindex = 8,
+    }, {
+        .name = NULL,
+    }
+};
+
+static MemoryRegion *mr_for_raminfo(MPS3RMachineState *mms,
+                                    const RAMInfo *raminfo)
+{
+    /* Return an initialized MemoryRegion for the RAMInfo. */
+    MemoryRegion *ram;
+
+    if (raminfo->mrindex < 0) {
+        /* Means this RAMInfo is for QEMU's "system memory" */
+        MachineState *machine = MACHINE(mms);
+        assert(!(raminfo->flags & IS_ROM));
+        return machine->ram;
+    }
+
+    assert(raminfo->mrindex < MPS3R_RAM_MAX);
+    ram = &mms->ram[raminfo->mrindex];
+
+    memory_region_init_ram(ram, NULL, raminfo->name,
+                           raminfo->size, &error_fatal);
+    if (raminfo->flags & IS_ROM) {
+        memory_region_set_readonly(ram, true);
+    }
+    return ram;
+}
+
+static void mps3r_common_init(MachineState *machine)
+{
+    MPS3RMachineState *mms = MPS3R_MACHINE(machine);
+    MPS3RMachineClass *mmc = MPS3R_MACHINE_GET_CLASS(mms);
+    MemoryRegion *sysmem = get_system_memory();
+
+    for (const RAMInfo *ri = mmc->raminfo; ri->name; ri++) {
+        MemoryRegion *mr = mr_for_raminfo(mms, ri);
+        memory_region_add_subregion(sysmem, ri->base, mr);
+    }
+}
+
+static void mps3r_set_default_ram_info(MPS3RMachineClass *mmc)
+{
+    /*
+     * Set mc->default_ram_size and default_ram_id from the
+     * information in mmc->raminfo.
+     */
+    MachineClass *mc = MACHINE_CLASS(mmc);
+    const RAMInfo *p;
+
+    for (p = mmc->raminfo; p->name; p++) {
+        if (p->mrindex < 0) {
+            /* Found the entry for "system memory" */
+            mc->default_ram_size = p->size;
+            mc->default_ram_id = p->name;
+            return;
+        }
+    }
+    g_assert_not_reached();
+}
+
+static void mps3r_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+
+    mc->init = mps3r_common_init;
+}
+
+static void mps3r_an536_class_init(ObjectClass *oc, void *data)
+{
+    MachineClass *mc = MACHINE_CLASS(oc);
+    MPS3RMachineClass *mmc = MPS3R_MACHINE_CLASS(oc);
+    static const char * const valid_cpu_types[] = {
+        ARM_CPU_TYPE_NAME("cortex-r52"),
+        NULL
+    };
+
+    mc->desc = "ARM MPS3 with AN536 FPGA image for Cortex-R52";
+    mc->default_cpus = 2;
+    mc->min_cpus = mc->default_cpus;
+    mc->max_cpus = mc->default_cpus;
+    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-r52");
+    mc->valid_cpu_types = valid_cpu_types;
+    mmc->raminfo = an536_raminfo;
+    mps3r_set_default_ram_info(mmc);
+}
+
+static const TypeInfo mps3r_machine_types[] = {
+    {
+        .name = TYPE_MPS3R_MACHINE,
+        .parent = TYPE_MACHINE,
+        .abstract = true,
+        .instance_size = sizeof(MPS3RMachineState),
+        .class_size = sizeof(MPS3RMachineClass),
+        .class_init = mps3r_class_init,
+    }, {
+        .name = TYPE_MPS3R_AN536_MACHINE,
+        .parent = TYPE_MPS3R_MACHINE,
+        .class_init = mps3r_an536_class_init,
+    },
+};
+
+DEFINE_TYPES(mps3r_machine_types);
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index db08a00a45b..8b45dc116ae 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -106,6 +106,11 @@ config MAINSTONE
     select PFLASH_CFI01
     select SMC91C111
 
+config MPS3R
+    bool
+    default y
+    depends on TCG && ARM
+
 config MUSCA
     bool
     default y
diff --git a/hw/arm/meson.build b/hw/arm/meson.build
index c4017790670..a16d3479055 100644
--- a/hw/arm/meson.build
+++ b/hw/arm/meson.build
@@ -8,6 +8,7 @@ arm_ss.add(when: 'CONFIG_HIGHBANK', if_true: files('highbank.c'))
 arm_ss.add(when: 'CONFIG_INTEGRATOR', if_true: files('integratorcp.c'))
 arm_ss.add(when: 'CONFIG_MAINSTONE', if_true: files('mainstone.c'))
 arm_ss.add(when: 'CONFIG_MICROBIT', if_true: files('microbit.c'))
+arm_ss.add(when: 'CONFIG_MPS3R', if_true: files('mps3r.c'))
 arm_ss.add(when: 'CONFIG_MUSICPAL', if_true: files('musicpal.c'))
 arm_ss.add(when: 'CONFIG_NETDUINOPLUS2', if_true: files('netduinoplus2.c'))
 arm_ss.add(when: 'CONFIG_OLIMEX_STM32_H405', if_true: files('olimex-stm32-h405.c'))
-- 
2.34.1

Re: [PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Philippe Mathieu-Daudé]

Hi Peter,

On 6/2/24 14:29, Peter Maydell wrote:
> The AN536 is another FPGA image for the MPS3 development board. Unlike
> the existing FPGA images we already model, this board uses a Cortex-R
> family CPU, and it does not use any equivalent to the M-profile
> "Subsystem for Embedded" SoC-equivalent that we model in hw/arm/armsse.c.
> It's therefore more convenient for us to model it as a completely
> separate C file.
> 
> This commit adds the basic skeleton of the board model, and the
> code to create all the RAM and ROM. We assume that we're probably
> going to want to add more images in future, so use the same
> base class/subclass setup that mps2-tz.c uses, even though at
> the moment there's only a single subclass.
> 
> Following commits will add the CPUs and the peripherals.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   MAINTAINERS                             |   3 +-
>   configs/devices/arm-softmmu/default.mak |   1 +
>   hw/arm/mps3r.c                          | 239 ++++++++++++++++++++++++
>   hw/arm/Kconfig                          |   5 +
>   hw/arm/meson.build                      |   1 +
>   5 files changed, 248 insertions(+), 1 deletion(-)
>   create mode 100644 hw/arm/mps3r.c


> +static MemoryRegion *mr_for_raminfo(MPS3RMachineState *mms,
> +                                    const RAMInfo *raminfo)
> +{
> +    /* Return an initialized MemoryRegion for the RAMInfo. */
> +    MemoryRegion *ram;
> +
> +    if (raminfo->mrindex < 0) {
> +        /* Means this RAMInfo is for QEMU's "system memory" */
> +        MachineState *machine = MACHINE(mms);
> +        assert(!(raminfo->flags & IS_ROM));
> +        return machine->ram;
> +    }
> +
> +    assert(raminfo->mrindex < MPS3R_RAM_MAX);
> +    ram = &mms->ram[raminfo->mrindex];
> +
> +    memory_region_init_ram(ram, NULL, raminfo->name,

You are not using the parent=mms, is that deliberate?
(as in: easier to migrate eventually?)

> +                           raminfo->size, &error_fatal);
> +    if (raminfo->flags & IS_ROM) {
> +        memory_region_set_readonly(ram, true);
> +    }
> +    return ram;
> +}

Re: [PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Peter Maydell]

On Tue, 6 Feb 2024 at 19:21, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Peter,
>
> On 6/2/24 14:29, Peter Maydell wrote:
> > The AN536 is another FPGA image for the MPS3 development board. Unlike
> > the existing FPGA images we already model, this board uses a Cortex-R
> > family CPU, and it does not use any equivalent to the M-profile
> > "Subsystem for Embedded" SoC-equivalent that we model in hw/arm/armsse.c.
> > It's therefore more convenient for us to model it as a completely
> > separate C file.
> >
> > This commit adds the basic skeleton of the board model, and the
> > code to create all the RAM and ROM. We assume that we're probably
> > going to want to add more images in future, so use the same
> > base class/subclass setup that mps2-tz.c uses, even though at
> > the moment there's only a single subclass.
> >
> > Following commits will add the CPUs and the peripherals.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   MAINTAINERS                             |   3 +-
> >   configs/devices/arm-softmmu/default.mak |   1 +
> >   hw/arm/mps3r.c                          | 239 ++++++++++++++++++++++++
> >   hw/arm/Kconfig                          |   5 +
> >   hw/arm/meson.build                      |   1 +
> >   5 files changed, 248 insertions(+), 1 deletion(-)
> >   create mode 100644 hw/arm/mps3r.c
>
>
> > +static MemoryRegion *mr_for_raminfo(MPS3RMachineState *mms,
> > +                                    const RAMInfo *raminfo)
> > +{
> > +    /* Return an initialized MemoryRegion for the RAMInfo. */
> > +    MemoryRegion *ram;
> > +
> > +    if (raminfo->mrindex < 0) {
> > +        /* Means this RAMInfo is for QEMU's "system memory" */
> > +        MachineState *machine = MACHINE(mms);
> > +        assert(!(raminfo->flags & IS_ROM));
> > +        return machine->ram;
> > +    }
> > +
> > +    assert(raminfo->mrindex < MPS3R_RAM_MAX);
> > +    ram = &mms->ram[raminfo->mrindex];
> > +
> > +    memory_region_init_ram(ram, NULL, raminfo->name,
>
> You are not using the parent=mms, is that deliberate?
> (as in: easier to migrate eventually?)

No, I didn't have a particular reason for not setting the parent;
I just copied this bit of code from mps2-tz.c, which also doesn't
set the parent pointer...

-- PMM

Re: [PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> The AN536 is another FPGA image for the MPS3 development board. Unlike
> the existing FPGA images we already model, this board uses a Cortex-R
> family CPU, and it does not use any equivalent to the M-profile
> "Subsystem for Embedded" SoC-equivalent that we model in hw/arm/armsse.c.
> It's therefore more convenient for us to model it as a completely
> separate C file.
> 
> This commit adds the basic skeleton of the board model, and the
> code to create all the RAM and ROM. We assume that we're probably
> going to want to add more images in future, so use the same
> base class/subclass setup that mps2-tz.c uses, even though at
> the moment there's only a single subclass.
> 
> Following commits will add the CPUs and the peripherals.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   MAINTAINERS                             |   3 +-
>   configs/devices/arm-softmmu/default.mak |   1 +
>   hw/arm/mps3r.c                          | 239 ++++++++++++++++++++++++
>   hw/arm/Kconfig                          |   5 +
>   hw/arm/meson.build                      |   1 +
>   5 files changed, 248 insertions(+), 1 deletion(-)
>   create mode 100644 hw/arm/mps3r.c


> +/*
> + * The MPS3 DDR is 3GiB, but on a 32-bit host QEMU doesn't permit
> + * emulation of that much guest RAM, so artificially make it smaller.
> + */
> +#if HOST_LONG_BITS == 32
> +#define MPS3_DDR_SIZE (1 * GiB)
> +#else
> +#define MPS3_DDR_SIZE (3 * GiB)
> +#endif

Generically, can we migrate a VM started on a 32-bit host to a 64-bit
one?

> +static void mps3r_set_default_ram_info(MPS3RMachineClass *mmc)
> +{
> +    /*
> +     * Set mc->default_ram_size and default_ram_id from the
> +     * information in mmc->raminfo.
> +     */
> +    MachineClass *mc = MACHINE_CLASS(mmc);
> +    const RAMInfo *p;
> +
> +    for (p = mmc->raminfo; p->name; p++) {
> +        if (p->mrindex < 0) {
> +            /* Found the entry for "system memory" */
> +            mc->default_ram_size = p->size;
> +            mc->default_ram_id = p->name;

Nice.

> +            return;
> +        }
> +    }
> +    g_assert_not_reached();
> +}

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Peter Maydell]

On Wed, 7 Feb 2024 at 09:02, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 6/2/24 14:29, Peter Maydell wrote:
> > The AN536 is another FPGA image for the MPS3 development board. Unlike
> > the existing FPGA images we already model, this board uses a Cortex-R
> > family CPU, and it does not use any equivalent to the M-profile
> > "Subsystem for Embedded" SoC-equivalent that we model in hw/arm/armsse.c.
> > It's therefore more convenient for us to model it as a completely
> > separate C file.
> >
> > This commit adds the basic skeleton of the board model, and the
> > code to create all the RAM and ROM. We assume that we're probably
> > going to want to add more images in future, so use the same
> > base class/subclass setup that mps2-tz.c uses, even though at
> > the moment there's only a single subclass.
> >
> > Following commits will add the CPUs and the peripherals.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >   MAINTAINERS                             |   3 +-
> >   configs/devices/arm-softmmu/default.mak |   1 +
> >   hw/arm/mps3r.c                          | 239 ++++++++++++++++++++++++
> >   hw/arm/Kconfig                          |   5 +
> >   hw/arm/meson.build                      |   1 +
> >   5 files changed, 248 insertions(+), 1 deletion(-)
> >   create mode 100644 hw/arm/mps3r.c
>
>
> > +/*
> > + * The MPS3 DDR is 3GiB, but on a 32-bit host QEMU doesn't permit
> > + * emulation of that much guest RAM, so artificially make it smaller.
> > + */
> > +#if HOST_LONG_BITS == 32
> > +#define MPS3_DDR_SIZE (1 * GiB)
> > +#else
> > +#define MPS3_DDR_SIZE (3 * GiB)
> > +#endif
>
> Generically, can we migrate a VM started on a 32-bit host to a 64-bit
> one?

I think it's one of those things that in theory is supposed
to be possible and in practice nobody tests so it might well
not work. At any rate, this is the same thing we do already
in mps2-tz.c for the 2GB DRAM those boards have.

> > +static void mps3r_set_default_ram_info(MPS3RMachineClass *mmc)
> > +{
> > +    /*
> > +     * Set mc->default_ram_size and default_ram_id from the
> > +     * information in mmc->raminfo.
> > +     */
> > +    MachineClass *mc = MACHINE_CLASS(mmc);
> > +    const RAMInfo *p;
> > +
> > +    for (p = mmc->raminfo; p->name; p++) {
> > +        if (p->mrindex < 0) {
> > +            /* Found the entry for "system memory" */
> > +            mc->default_ram_size = p->size;
> > +            mc->default_ram_id = p->name;
>
> Nice.
>
> > +            return;
> > +        }
> > +    }
> > +    g_assert_not_reached();
> > +}
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks.

I have remembered why we don't pass the MachineState as the
owner pointer for memory_region_init_ram(): in that function
we do a cast of the owner pointer to a DeviceState, so we
can hand it to vmstate_register_ram(). And a MachineState
inherits directly from Object, not from DeviceState.
There's a paragraph in the doc-comment for
memory_region_init_ram() explaining this: currently we
require the owner pointer to be either NULL or a DeviceState.

thanks
-- PMM

Re: [PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Cédric Le Goater]

>>> +/*
>>> + * The MPS3 DDR is 3GiB, but on a 32-bit host QEMU doesn't permit
>>> + * emulation of that much guest RAM, so artificially make it smaller.
>>> + */
>>> +#if HOST_LONG_BITS == 32
>>> +#define MPS3_DDR_SIZE (1 * GiB)
>>> +#else
>>> +#define MPS3_DDR_SIZE (3 * GiB)
>>> +#endif
>>
>> Generically, can we migrate a VM started on a 32-bit host to a 64-bit
>> one?
> 
> I think it's one of those things that in theory is supposed
> to be possible and in practice nobody tests so it might well
> not work. At any rate, this is the same thing we do already
> in mps2-tz.c for the 2GB DRAM those boards have.

We could have a common helper may be. Aspeed does:

   /* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
   #if HOST_LONG_BITS == 32
   #define ASPEED_RAM_SIZE(sz) MIN((sz), 1 * GiB)
   #else
   #define ASPEED_RAM_SIZE(sz) (sz)
   #endif

Thanks,

C.

Re: [PATCH 08/13] hw/arm/mps3r: Initial skeleton for mps3-an536 board

[Philippe Mathieu-Daudé]

On 8/2/24 18:07, Cédric Le Goater wrote:
> 
>>>> +/*
>>>> + * The MPS3 DDR is 3GiB, but on a 32-bit host QEMU doesn't permit
>>>> + * emulation of that much guest RAM, so artificially make it smaller.
>>>> + */
>>>> +#if HOST_LONG_BITS == 32
>>>> +#define MPS3_DDR_SIZE (1 * GiB)
>>>> +#else
>>>> +#define MPS3_DDR_SIZE (3 * GiB)
>>>> +#endif
>>>
>>> Generically, can we migrate a VM started on a 32-bit host to a 64-bit
>>> one?
>>
>> I think it's one of those things that in theory is supposed
>> to be possible and in practice nobody tests so it might well
>> not work. At any rate, this is the same thing we do already
>> in mps2-tz.c for the 2GB DRAM those boards have.
> 
> We could have a common helper may be. Aspeed does:
> 
>    /* On 32-bit hosts, lower RAM to 1G because of the 2047 MB limit */
>    #if HOST_LONG_BITS == 32
>    #define ASPEED_RAM_SIZE(sz) MIN((sz), 1 * GiB)
>    #else
>    #define ASPEED_RAM_SIZE(sz) (sz)
>    #endif

Or deprecate system emulation on 32-bit hosts.

[PATCH 09/13] hw/arm/mps3r: Add CPUs, GIC, and per-CPU RAM

[Peter Maydell]

Create the CPUs, the GIC, and the per-CPU RAM block for
the mps3-an536 board.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Some parts of this might need to end up parameterisable if/when
we add another machine type to this source file, but rather than
trying to guess which parts, I stuck with the simple code for now.

I thought about creating a "wrapper" device like our hw/cpu/a15mpcore.c
etc for the Cortex-R52 and GIC, since in hardware the GIC is part
of the CPU proper, not an external device. However given the need
for some of the devices in this system to be per-CPU and have
per-CPU interrupts, this seemed too awkward, so I have open
coded the creation of the CPUs and GIC here. We can always
consider refactoring later if we get another Cortex-R52 board.
---
 hw/arm/mps3r.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 177 insertions(+), 3 deletions(-)

diff --git a/hw/arm/mps3r.c b/hw/arm/mps3r.c
index 888a846d23c..6473f62d677 100644
--- a/hw/arm/mps3r.c
+++ b/hw/arm/mps3r.c
@@ -27,10 +27,14 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qlist.h"
 #include "exec/address-spaces.h"
 #include "cpu.h"
 #include "hw/boards.h"
+#include "hw/qdev-properties.h"
 #include "hw/arm/boot.h"
+#include "hw/arm/bsa.h"
+#include "hw/intc/arm_gicv3.h"
 
 /* Define the layout of RAM and ROM in a board */
 typedef struct RAMInfo {
@@ -60,6 +64,10 @@ typedef struct RAMInfo {
 #define IS_ROM 2
 
 #define MPS3R_RAM_MAX 9
+#define MPS3R_CPU_MAX 2
+
+#define PERIPHBASE 0xf0000000
+#define NUM_SPIS 96
 
 typedef enum MPS3RFPGAType {
     FPGA_AN536,
@@ -69,11 +77,18 @@ struct MPS3RMachineClass {
     MachineClass parent;
     MPS3RFPGAType fpga_type;
     const RAMInfo *raminfo;
+    hwaddr loader_start;
 };
 
 struct MPS3RMachineState {
     MachineState parent;
+    struct arm_boot_info bootinfo;
     MemoryRegion ram[MPS3R_RAM_MAX];
+    Object *cpu[MPS3R_CPU_MAX];
+    MemoryRegion cpu_sysmem[MPS3R_CPU_MAX];
+    MemoryRegion sysmem_alias[MPS3R_CPU_MAX];
+    MemoryRegion cpu_ram[MPS3R_CPU_MAX];
+    GICv3State gic;
 };
 
 #define TYPE_MPS3R_MACHINE "mps3r"
@@ -163,6 +178,107 @@ static MemoryRegion *mr_for_raminfo(MPS3RMachineState *mms,
     return ram;
 }
 
+/*
+ * There is no defined secondary boot protocol for Linux for the AN536,
+ * because real hardware has a restriction that atomic operations between
+ * the two CPUs do not function correctly, and so true SMP is not
+ * possible. Therefore for cases where the user is directly booting
+ * a kernel, we treat the system as essentially uniprocessor, and
+ * put the secondary CPU into power-off state (as if the user on the
+ * real hardware had configured the secondary to be halted via the
+ * SCC config registers).
+ *
+ * Note that the default secondary boot code would not work here anyway
+ * as it assumes a GICv2, and we have a GICv3.
+ */
+static void mps3r_write_secondary_boot(ARMCPU *cpu,
+                                       const struct arm_boot_info *info)
+{
+    /*
+     * Power the secondary CPU off. This means we don't need to write any
+     * boot code into guest memory. Note that the 'cpu' argument to this
+     * function is the primary CPU we passed to arm_load_kernel(), not
+     * the secondary. Loop around all the other CPUs, as the boot.c
+     * code does for the "disable secondaries if PSCI is enabled" case.
+     */
+    for (CPUState *cs = first_cpu; cs; cs = CPU_NEXT(cs)) {
+        if (cs != first_cpu) {
+            object_property_set_bool(OBJECT(cs), "start-powered-off", true,
+                                     &error_abort);
+        }
+    }
+}
+
+static void mps3r_secondary_cpu_reset(ARMCPU *cpu,
+                                      const struct arm_boot_info *info)
+{
+    /* We don't need to do anything here because the CPU will be off */
+}
+
+static void create_gic(MPS3RMachineState *mms, MemoryRegion *sysmem)
+{
+    MachineState *machine = MACHINE(mms);
+    DeviceState *gicdev;
+    QList *redist_region_count;
+
+    object_initialize_child(OBJECT(mms), "gic", &mms->gic, TYPE_ARM_GICV3);
+    gicdev = DEVICE(&mms->gic);
+    qdev_prop_set_uint32(gicdev, "num-cpu", machine->smp.cpus);
+    qdev_prop_set_uint32(gicdev, "num-irq", NUM_SPIS + GIC_INTERNAL);
+    redist_region_count = qlist_new();
+    qlist_append_int(redist_region_count, machine->smp.cpus);
+    qdev_prop_set_array(gicdev, "redist-region-count", redist_region_count);
+    object_property_set_link(OBJECT(&mms->gic), "sysmem",
+                             OBJECT(sysmem), &error_fatal);
+    sysbus_realize(SYS_BUS_DEVICE(&mms->gic), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->gic), 0, PERIPHBASE);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->gic), 1, PERIPHBASE + 0x100000);
+    /*
+     * Wire the outputs from each CPU's generic timer and the GICv3
+     * maintenance interrupt signal to the appropriate GIC PPI inputs,
+     * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.
+     */
+    for (int i = 0; i < machine->smp.cpus; i++) {
+        DeviceState *cpudev = DEVICE(mms->cpu[i]);
+        SysBusDevice *gicsbd = SYS_BUS_DEVICE(&mms->gic);
+        int intidbase = NUM_SPIS + i * GIC_INTERNAL;
+        int irq;
+        /*
+         * Mapping from the output timer irq lines from the CPU to the
+         * GIC PPI inputs used for this board. This isn't a BSA board,
+         * but it uses the standard convention for the PPI numbers.
+         */
+        const int timer_irq[] = {
+            [GTIMER_PHYS] = ARCH_TIMER_NS_EL1_IRQ,
+            [GTIMER_VIRT] = ARCH_TIMER_VIRT_IRQ,
+            [GTIMER_HYP]  = ARCH_TIMER_NS_EL2_IRQ,
+        };
+
+        for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {
+            qdev_connect_gpio_out(cpudev, irq,
+                                  qdev_get_gpio_in(gicdev,
+                                                   intidbase + timer_irq[irq]));
+        }
+
+        qdev_connect_gpio_out_named(cpudev, "gicv3-maintenance-interrupt", 0,
+                                    qdev_get_gpio_in(gicdev,
+                                                     intidbase + ARCH_GIC_MAINT_IRQ));
+
+        qdev_connect_gpio_out_named(cpudev, "pmu-interrupt", 0,
+                                    qdev_get_gpio_in(gicdev,
+                                                     intidbase + VIRTUAL_PMU_IRQ));
+
+        sysbus_connect_irq(gicsbd, i,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));
+        sysbus_connect_irq(gicsbd, i + machine->smp.cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
+        sysbus_connect_irq(gicsbd, i + 2 * machine->smp.cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));
+        sysbus_connect_irq(gicsbd, i + 3 * machine->smp.cpus,
+                           qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));
+    }
+}
+
 static void mps3r_common_init(MachineState *machine)
 {
     MPS3RMachineState *mms = MPS3R_MACHINE(machine);
@@ -173,6 +289,50 @@ static void mps3r_common_init(MachineState *machine)
         MemoryRegion *mr = mr_for_raminfo(mms, ri);
         memory_region_add_subregion(sysmem, ri->base, mr);
     }
+
+    assert(machine->smp.cpus <= MPS3R_CPU_MAX);
+    for (int i = 0; i < machine->smp.cpus; i++) {
+        g_autofree char *sysmem_name = g_strdup_printf("cpu-%d-memory", i);
+        g_autofree char *ramname = g_strdup_printf("cpu-%d-memory", i);
+        g_autofree char *alias_name = g_strdup_printf("sysmem-alias-%d", i);
+
+        /*
+         * Each CPU has some private RAM/peripherals, so create the container
+         * which will house those, with the whole-machine system memory being
+         * used where there's no CPU-specific device. Note that we need the
+         * sysmem_alias aliases because we can't put one MR (the original
+         * 'sysmem') into more than one other MR.
+         */
+        memory_region_init(&mms->cpu_sysmem[i], OBJECT(machine),
+                           sysmem_name, UINT64_MAX);
+        memory_region_init_alias(&mms->sysmem_alias[i], OBJECT(machine),
+                                 alias_name, sysmem, 0, UINT64_MAX);
+        memory_region_add_subregion_overlap(&mms->cpu_sysmem[i], 0,
+                                            &mms->sysmem_alias[i], -1);
+
+        mms->cpu[i] = object_new(machine->cpu_type);
+        object_property_set_link(mms->cpu[i], "memory",
+                                 OBJECT(&mms->cpu_sysmem[i]), &error_abort);
+        object_property_set_int(mms->cpu[i], "reset-cbar",
+                                PERIPHBASE, &error_abort);
+        qdev_realize(DEVICE(mms->cpu[i]), NULL, &error_fatal);
+        object_unref(mms->cpu[i]);
+
+        /* Per-CPU RAM */
+        memory_region_init_ram(&mms->cpu_ram[i], NULL, ramname,
+                               0x1000, &error_fatal);
+        memory_region_add_subregion(&mms->cpu_sysmem[i], 0xe7c01000,
+                                    &mms->cpu_ram[i]);
+    }
+
+    create_gic(mms, sysmem);
+
+    mms->bootinfo.ram_size = machine->ram_size;
+    mms->bootinfo.board_id = -1;
+    mms->bootinfo.loader_start = mmc->loader_start;
+    mms->bootinfo.write_secondary_boot = mps3r_write_secondary_boot;
+    mms->bootinfo.secondary_cpu_reset_hook = mps3r_secondary_cpu_reset;
+    arm_load_kernel(ARM_CPU(mms->cpu[0]), machine, &mms->bootinfo);
 }
 
 static void mps3r_set_default_ram_info(MPS3RMachineClass *mmc)
@@ -189,6 +349,7 @@ static void mps3r_set_default_ram_info(MPS3RMachineClass *mmc)
             /* Found the entry for "system memory" */
             mc->default_ram_size = p->size;
             mc->default_ram_id = p->name;
+            mmc->loader_start = p->base;
             return;
         }
     }
@@ -212,9 +373,22 @@ static void mps3r_an536_class_init(ObjectClass *oc, void *data)
     };
 
     mc->desc = "ARM MPS3 with AN536 FPGA image for Cortex-R52";
-    mc->default_cpus = 2;
-    mc->min_cpus = mc->default_cpus;
-    mc->max_cpus = mc->default_cpus;
+    /*
+     * In the real FPGA image there are always two cores, but the standard
+     * initial setting for the SCC SYSCON 0x000 register is 0x21, meaning
+     * that the second core is held in reset and halted. Many images built for
+     * the board do not expect the second core to run at startup (especially
+     * since on the real FPGA image it is not possible to use LDREX/STREX
+     * in RAM between the two cores, so a true SMP setup isn't supported).
+     *
+     * As QEMU's equivalent of this, we support both -smp 1 and -smp 2,
+     * with the default being -smp 1. This seems a more intuitive UI for
+     * QEMU users than, for instance, having a machine property to allow
+     * the user to set the initial value of the SYSCON 0x000 register.
+     */
+    mc->default_cpus = 1;
+    mc->min_cpus = 1;
+    mc->max_cpus = 2;
     mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-r52");
     mc->valid_cpu_types = valid_cpu_types;
     mmc->raminfo = an536_raminfo;
-- 
2.34.1

Re: [PATCH 09/13] hw/arm/mps3r: Add CPUs, GIC, and per-CPU RAM

[Alex Bennée]

Peter Maydell <peter.maydell@linaro.org> writes:

> Create the CPUs, the GIC, and the per-CPU RAM block for
> the mps3-an536 board.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

LGTM:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 10/13] hw/arm/mps3r: Add UARTs

[Peter Maydell]

This board has a lot of UARTs: there is one UART per CPU in the
per-CPU peripheral part of the address map, whose interrupts are
connected as per-CPU interrupt lines.  Then there are 4 UARTs in the
normal part of the peripheral space, whose interrupts are shared
peripheral interrupts.

Connect and wire them all up; this involves some OR gates where
multiple overflow interrupts are wired into one GIC input.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mps3r.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/hw/arm/mps3r.c b/hw/arm/mps3r.c
index 6473f62d677..8c790313790 100644
--- a/hw/arm/mps3r.c
+++ b/hw/arm/mps3r.c
@@ -30,10 +30,13 @@
 #include "qapi/qmp/qlist.h"
 #include "exec/address-spaces.h"
 #include "cpu.h"
+#include "sysemu/sysemu.h"
 #include "hw/boards.h"
+#include "hw/or-irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/bsa.h"
+#include "hw/char/cmsdk-apb-uart.h"
 #include "hw/intc/arm_gicv3.h"
 
 /* Define the layout of RAM and ROM in a board */
@@ -65,6 +68,7 @@ typedef struct RAMInfo {
 
 #define MPS3R_RAM_MAX 9
 #define MPS3R_CPU_MAX 2
+#define MPS3R_UART_MAX 4 /* shared UART count */
 
 #define PERIPHBASE 0xf0000000
 #define NUM_SPIS 96
@@ -89,6 +93,10 @@ struct MPS3RMachineState {
     MemoryRegion sysmem_alias[MPS3R_CPU_MAX];
     MemoryRegion cpu_ram[MPS3R_CPU_MAX];
     GICv3State gic;
+    /* per-CPU UARTs followed by the shared UARTs */
+    CMSDKAPBUART uart[MPS3R_CPU_MAX + MPS3R_UART_MAX];
+    OrIRQState cpu_uart_oflow[MPS3R_CPU_MAX];
+    OrIRQState uart_oflow;
 };
 
 #define TYPE_MPS3R_MACHINE "mps3r"
@@ -96,6 +104,13 @@ struct MPS3RMachineState {
 
 OBJECT_DECLARE_TYPE(MPS3RMachineState, MPS3RMachineClass, MPS3R_MACHINE)
 
+/*
+ * Main clock frequency CLK in Hz (50MHz). In the image there are also
+ * ACLK, MCLK, GPUCLK and PERIPHCLK at the same frequency; for our
+ * model we just roll them all into one.
+ */
+#define CLK_FRQ 50000000
+
 static const RAMInfo an536_raminfo[] = {
     {
         .name = "ATCM",
@@ -279,11 +294,40 @@ static void create_gic(MPS3RMachineState *mms, MemoryRegion *sysmem)
     }
 }
 
+/*
+ * Create UART uartno, and map it into the MemoryRegion mem at address baseaddr.
+ * The qemu_irq arguments are where we connect the various IRQs from the UART.
+ */
+static void create_uart(MPS3RMachineState *mms, int uartno, MemoryRegion *mem,
+                        hwaddr baseaddr, qemu_irq txirq, qemu_irq rxirq,
+                        qemu_irq txoverirq, qemu_irq rxoverirq,
+                        qemu_irq combirq)
+{
+    g_autofree char *s = g_strdup_printf("uart%d", uartno);
+    SysBusDevice *sbd;
+
+    assert(uartno < ARRAY_SIZE(mms->uart));
+    object_initialize_child(OBJECT(mms), s, &mms->uart[uartno],
+                            TYPE_CMSDK_APB_UART);
+    qdev_prop_set_uint32(DEVICE(&mms->uart[uartno]), "pclk-frq", CLK_FRQ);
+    qdev_prop_set_chr(DEVICE(&mms->uart[uartno]), "chardev", serial_hd(uartno));
+    sbd = SYS_BUS_DEVICE(&mms->uart[uartno]);
+    sysbus_realize(sbd, &error_fatal);
+    memory_region_add_subregion(mem, baseaddr,
+                                sysbus_mmio_get_region(sbd, 0));
+    sysbus_connect_irq(sbd, 0, txirq);
+    sysbus_connect_irq(sbd, 1, rxirq);
+    sysbus_connect_irq(sbd, 2, txoverirq);
+    sysbus_connect_irq(sbd, 3, rxoverirq);
+    sysbus_connect_irq(sbd, 4, combirq);
+}
+
 static void mps3r_common_init(MachineState *machine)
 {
     MPS3RMachineState *mms = MPS3R_MACHINE(machine);
     MPS3RMachineClass *mmc = MPS3R_MACHINE_GET_CLASS(mms);
     MemoryRegion *sysmem = get_system_memory();
+    DeviceState *gicdev;
 
     for (const RAMInfo *ri = mmc->raminfo; ri->name; ri++) {
         MemoryRegion *mr = mr_for_raminfo(mms, ri);
@@ -326,6 +370,56 @@ static void mps3r_common_init(MachineState *machine)
     }
 
     create_gic(mms, sysmem);
+    gicdev = DEVICE(&mms->gic);
+
+    /*
+     * UARTs 0 and 1 are per-CPU; their interrupts are wired to
+     * the relevant CPU's PPI 0..3, aka INTID 16..19
+     */
+    for (int i = 0; i < machine->smp.cpus; i++) {
+        int intidbase = NUM_SPIS + i * GIC_INTERNAL;
+        g_autofree char *s = g_strdup_printf("cpu-uart-oflow-orgate%d", i);
+        DeviceState *orgate;
+
+        /* The two overflow IRQs from the UART are ORed together into PPI 3 */
+        object_initialize_child(OBJECT(mms), s, &mms->cpu_uart_oflow[i],
+                                TYPE_OR_IRQ);
+        orgate = DEVICE(&mms->cpu_uart_oflow[i]);
+        qdev_prop_set_uint32(orgate, "num-lines", 2);
+        qdev_realize(orgate, NULL, &error_fatal);
+        qdev_connect_gpio_out(orgate, 0,
+                              qdev_get_gpio_in(gicdev, intidbase + 19));
+
+        create_uart(mms, i, &mms->cpu_sysmem[i], 0xe7c00000,
+                    qdev_get_gpio_in(gicdev, intidbase + 17), /* tx */
+                    qdev_get_gpio_in(gicdev, intidbase + 16), /* rx */
+                    qdev_get_gpio_in(orgate, 0), /* txover */
+                    qdev_get_gpio_in(orgate, 1), /* rxover */
+                    qdev_get_gpio_in(gicdev, intidbase + 18) /* combined */);
+    }
+    /*
+     * UARTs 2 to 5 are whole-system; all overflow IRQs are ORed
+     * together into IRQ 17
+     */
+    object_initialize_child(OBJECT(mms), "uart-oflow-orgate",
+                            &mms->uart_oflow, TYPE_OR_IRQ);
+    qdev_prop_set_uint32(DEVICE(&mms->uart_oflow), "num-lines",
+                         MPS3R_UART_MAX * 2);
+    qdev_realize(DEVICE(&mms->uart_oflow), NULL, &error_fatal);
+    qdev_connect_gpio_out(DEVICE(&mms->uart_oflow), 0,
+                          qdev_get_gpio_in(gicdev, 17));
+
+    for (int i = 0; i < MPS3R_UART_MAX; i++) {
+        hwaddr baseaddr = 0xe0205000 + i * 0x1000;
+        int rxirq = 5 + i * 2, txirq = 6 + i * 2, combirq = 13 + i;
+
+        create_uart(mms, i + MPS3R_CPU_MAX, sysmem, baseaddr,
+                    qdev_get_gpio_in(gicdev, txirq),
+                    qdev_get_gpio_in(gicdev, rxirq),
+                    qdev_get_gpio_in(DEVICE(&mms->uart_oflow), i * 2),
+                    qdev_get_gpio_in(DEVICE(&mms->uart_oflow), i * 2 + 1),
+                    qdev_get_gpio_in(gicdev, combirq));
+    }
 
     mms->bootinfo.ram_size = machine->ram_size;
     mms->bootinfo.board_id = -1;
-- 
2.34.1

Re: [PATCH 10/13] hw/arm/mps3r: Add UARTs

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> This board has a lot of UARTs: there is one UART per CPU in the
> per-CPU peripheral part of the address map, whose interrupts are
> connected as per-CPU interrupt lines.  Then there are 4 UARTs in the
> normal part of the peripheral space, whose interrupts are shared
> peripheral interrupts.
> 
> Connect and wire them all up; this involves some OR gates where
> multiple overflow interrupts are wired into one GIC input.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/mps3r.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 94 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 11/13] hw/arm/mps3r: Add GPIO, watchdog, dual-timer, I2C devices

[Peter Maydell]

Add the GPIO, watchdog, dual-timer and I2C devices to the mps3-an536
board.  These are all simple devices that just need to be created and
wired up.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mps3r.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/hw/arm/mps3r.c b/hw/arm/mps3r.c
index 8c790313790..803ed0ffb5c 100644
--- a/hw/arm/mps3r.c
+++ b/hw/arm/mps3r.c
@@ -33,11 +33,16 @@
 #include "sysemu/sysemu.h"
 #include "hw/boards.h"
 #include "hw/or-irq.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "hw/arm/boot.h"
 #include "hw/arm/bsa.h"
 #include "hw/char/cmsdk-apb-uart.h"
+#include "hw/i2c/arm_sbcon_i2c.h"
 #include "hw/intc/arm_gicv3.h"
+#include "hw/misc/unimp.h"
+#include "hw/timer/cmsdk-apb-dualtimer.h"
+#include "hw/watchdog/cmsdk-apb-watchdog.h"
 
 /* Define the layout of RAM and ROM in a board */
 typedef struct RAMInfo {
@@ -97,6 +102,10 @@ struct MPS3RMachineState {
     CMSDKAPBUART uart[MPS3R_CPU_MAX + MPS3R_UART_MAX];
     OrIRQState cpu_uart_oflow[MPS3R_CPU_MAX];
     OrIRQState uart_oflow;
+    CMSDKAPBWatchdog watchdog;
+    CMSDKAPBDualTimer dualtimer;
+    ArmSbconI2CState i2c[5];
+    Clock *clk;
 };
 
 #define TYPE_MPS3R_MACHINE "mps3r"
@@ -329,6 +338,9 @@ static void mps3r_common_init(MachineState *machine)
     MemoryRegion *sysmem = get_system_memory();
     DeviceState *gicdev;
 
+    mms->clk = clock_new(OBJECT(machine), "CLK");
+    clock_set_hz(mms->clk, CLK_FRQ);
+
     for (const RAMInfo *ri = mmc->raminfo; ri->name; ri++) {
         MemoryRegion *mr = mr_for_raminfo(mms, ri);
         memory_region_add_subregion(sysmem, ri->base, mr);
@@ -421,6 +433,53 @@ static void mps3r_common_init(MachineState *machine)
                     qdev_get_gpio_in(gicdev, combirq));
     }
 
+    for (int i = 0; i < 4; i++) {
+        /* CMSDK GPIO controllers */
+        g_autofree char *s = g_strdup_printf("gpio%d", i);
+        create_unimplemented_device(s, 0xe0000000 + i * 0x1000, 0x1000);
+    }
+
+    object_initialize_child(OBJECT(mms), "watchdog", &mms->watchdog,
+                            TYPE_CMSDK_APB_WATCHDOG);
+    qdev_connect_clock_in(DEVICE(&mms->watchdog), "WDOGCLK", mms->clk);
+    sysbus_realize(SYS_BUS_DEVICE(&mms->watchdog), &error_fatal);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&mms->watchdog), 0,
+                       qdev_get_gpio_in(gicdev, 0));
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->watchdog), 0, 0xe0100000);
+
+    object_initialize_child(OBJECT(mms), "dualtimer", &mms->dualtimer,
+                            TYPE_CMSDK_APB_DUALTIMER);
+    qdev_connect_clock_in(DEVICE(&mms->dualtimer), "TIMCLK", mms->clk);
+    sysbus_realize(SYS_BUS_DEVICE(&mms->dualtimer), &error_fatal);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&mms->dualtimer), 0,
+                       qdev_get_gpio_in(gicdev, 3));
+    sysbus_connect_irq(SYS_BUS_DEVICE(&mms->dualtimer), 1,
+                       qdev_get_gpio_in(gicdev, 1));
+    sysbus_connect_irq(SYS_BUS_DEVICE(&mms->dualtimer), 2,
+                       qdev_get_gpio_in(gicdev, 2));
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->dualtimer), 0, 0xe0101000);
+
+    for (int i = 0; i < ARRAY_SIZE(mms->i2c); i++) {
+        static const hwaddr i2cbase[] = {0xe0102000,    /* Touch */
+                                         0xe0103000,    /* Audio */
+                                         0xe0107000,    /* Shield0 */
+                                         0xe0108000,    /* Shield1 */
+                                         0xe0109000};   /* DDR4 EEPROM */
+        g_autofree char *s = g_strdup_printf("i2c%d", i);
+
+        object_initialize_child(OBJECT(mms), s, &mms->i2c[i],
+                                TYPE_ARM_SBCON_I2C);
+        sysbus_realize(SYS_BUS_DEVICE(&mms->i2c[i]), &error_fatal);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&mms->i2c[i]), 0, i2cbase[i]);
+        if (i != 2 && i != 3) {
+            /*
+             * internal-only bus: mark it full to avoid user-created
+             * i2c devices being plugged into it.
+             */
+            qbus_mark_full(qdev_get_child_bus(DEVICE(&mms->i2c[i]), "i2c"));
+        }
+    }
+
     mms->bootinfo.ram_size = machine->ram_size;
     mms->bootinfo.board_id = -1;
     mms->bootinfo.loader_start = mmc->loader_start;
-- 
2.34.1

Re: [PATCH 11/13] hw/arm/mps3r: Add GPIO, watchdog, dual-timer, I2C devices

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> Add the GPIO, watchdog, dual-timer and I2C devices to the mps3-an536
> board.  These are all simple devices that just need to be created and
> wired up.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/mps3r.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 59 insertions(+)


> +    for (int i = 0; i < ARRAY_SIZE(mms->i2c); i++) {

> +        if (i != 2 && i != 3) {

Maybe easier to read as:

           if (i < 2 || i > 3) {

> +            /*
> +             * internal-only bus: mark it full to avoid user-created
> +             * i2c devices being plugged into it.
> +             */
> +            qbus_mark_full(qdev_get_child_bus(DEVICE(&mms->i2c[i]), "i2c"));

TIL qbus_mark_full! Nice.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        }
> +    }

[PATCH 12/13] hw/arm/mps3r: Add remaining devices

[Peter Maydell]

Add the remaining devices (or unimplemented-device stubs) for
this board: SPI controllers, SCC, FPGAIO, I2S, RTC, the
QSPI write-config block, and ethernet.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/mps3r.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/hw/arm/mps3r.c b/hw/arm/mps3r.c
index 803ed0ffb5c..4d55a6564c6 100644
--- a/hw/arm/mps3r.c
+++ b/hw/arm/mps3r.c
@@ -40,7 +40,12 @@
 #include "hw/char/cmsdk-apb-uart.h"
 #include "hw/i2c/arm_sbcon_i2c.h"
 #include "hw/intc/arm_gicv3.h"
+#include "hw/misc/mps2-scc.h"
+#include "hw/misc/mps2-fpgaio.h"
 #include "hw/misc/unimp.h"
+#include "hw/net/lan9118.h"
+#include "hw/rtc/pl031.h"
+#include "hw/ssi/pl022.h"
 #include "hw/timer/cmsdk-apb-dualtimer.h"
 #include "hw/watchdog/cmsdk-apb-watchdog.h"
 
@@ -105,6 +110,11 @@ struct MPS3RMachineState {
     CMSDKAPBWatchdog watchdog;
     CMSDKAPBDualTimer dualtimer;
     ArmSbconI2CState i2c[5];
+    PL022State spi[3];
+    MPS2SCC scc;
+    MPS2FPGAIO fpgaio;
+    UnimplementedDeviceState i2s_audio;
+    PL031State rtc;
     Clock *clk;
 };
 
@@ -178,6 +188,16 @@ static const RAMInfo an536_raminfo[] = {
     }
 };
 
+static const int an536_oscclk[] = {
+    24000000, /* 24MHz reference for RTC and timers */
+    50000000, /* 50MHz ACLK */
+    50000000, /* 50MHz MCLK */
+    50000000, /* 50MHz GPUCLK */
+    24576000, /* 24.576MHz AUDCLK */
+    23750000, /* 23.75MHz HDLCDCLK */
+    100000000, /* 100MHz DDR4_REF_CLK */
+};
+
 static MemoryRegion *mr_for_raminfo(MPS3RMachineState *mms,
                                     const RAMInfo *raminfo)
 {
@@ -337,6 +357,7 @@ static void mps3r_common_init(MachineState *machine)
     MPS3RMachineClass *mmc = MPS3R_MACHINE_GET_CLASS(mms);
     MemoryRegion *sysmem = get_system_memory();
     DeviceState *gicdev;
+    QList *oscclk;
 
     mms->clk = clock_new(OBJECT(machine), "CLK");
     clock_set_hz(mms->clk, CLK_FRQ);
@@ -480,6 +501,59 @@ static void mps3r_common_init(MachineState *machine)
         }
     }
 
+    for (int i = 0; i < ARRAY_SIZE(mms->spi); i++) {
+        g_autofree char *s = g_strdup_printf("spi%d", i);
+        hwaddr baseaddr = 0xe0104000 + i * 0x1000;
+
+        object_initialize_child(OBJECT(mms), s, &mms->spi[i], TYPE_PL022);
+        sysbus_realize(SYS_BUS_DEVICE(&mms->spi[i]), &error_fatal);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&mms->spi[i]), 0, baseaddr);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&mms->spi[i]), 0,
+                           qdev_get_gpio_in(gicdev, 22 + i));
+    }
+
+    object_initialize_child(OBJECT(mms), "scc", &mms->scc, TYPE_MPS2_SCC);
+    qdev_prop_set_uint32(DEVICE(&mms->scc), "scc-cfg0", 0);
+    qdev_prop_set_uint32(DEVICE(&mms->scc), "scc-cfg4", 0x2);
+    qdev_prop_set_uint32(DEVICE(&mms->scc), "scc-aid", 0x00200008);
+    qdev_prop_set_uint32(DEVICE(&mms->scc), "scc-id", 0x41055360);
+    oscclk = qlist_new();
+    for (int i = 0; i < ARRAY_SIZE(an536_oscclk); i++) {
+        qlist_append_int(oscclk, an536_oscclk[i]);
+    }
+    qdev_prop_set_array(DEVICE(&mms->scc), "oscclk", oscclk);
+    sysbus_realize(SYS_BUS_DEVICE(&mms->scc), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->scc), 0, 0xe0200000);
+
+    create_unimplemented_device("i2s-audio", 0xe0201000, 0x1000);
+
+    object_initialize_child(OBJECT(mms), "fpgaio", &mms->fpgaio,
+                            TYPE_MPS2_FPGAIO);
+    qdev_prop_set_uint32(DEVICE(&mms->fpgaio), "prescale-clk", an536_oscclk[1]);
+    qdev_prop_set_uint32(DEVICE(&mms->fpgaio), "num-leds", 10);
+    qdev_prop_set_bit(DEVICE(&mms->fpgaio), "has-switches", true);
+    qdev_prop_set_bit(DEVICE(&mms->fpgaio), "has-dbgctrl", false);
+    sysbus_realize(SYS_BUS_DEVICE(&mms->fpgaio), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->fpgaio), 0, 0xe0202000);
+
+    create_unimplemented_device("clcd", 0xe0209000, 0x1000);
+
+    object_initialize_child(OBJECT(mms), "rtc", &mms->rtc, TYPE_PL031);
+    sysbus_realize(SYS_BUS_DEVICE(&mms->rtc), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(&mms->rtc), 0, 0xe020a000);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&mms->rtc), 0,
+                       qdev_get_gpio_in(gicdev, 4));
+
+    /*
+     * In hardware this is a LAN9220; the LAN9118 is software compatible
+     * except that it doesn't support the checksum-offload feature.
+     */
+    lan9118_init(0xe0300000,
+                 qdev_get_gpio_in(gicdev, 18));
+
+    create_unimplemented_device("usb", 0xe0301000, 0x1000);
+    create_unimplemented_device("qspi-write-config", 0xe0600000, 0x1000);
+
     mms->bootinfo.ram_size = machine->ram_size;
     mms->bootinfo.board_id = -1;
     mms->bootinfo.loader_start = mmc->loader_start;
-- 
2.34.1

Re: [PATCH 12/13] hw/arm/mps3r: Add remaining devices

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> Add the remaining devices (or unimplemented-device stubs) for
> this board: SPI controllers, SCC, FPGAIO, I2S, RTC, the
> QSPI write-config block, and ethernet.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   hw/arm/mps3r.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 74 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 13/13] docs: Add documentation for the mps3-an536 board

[Peter Maydell]

Add documentation for the mps3-an536 board type.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/system/arm/mps2.rst | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/docs/system/arm/mps2.rst b/docs/system/arm/mps2.rst
index 8a75beb3a08..a305935cc49 100644
--- a/docs/system/arm/mps2.rst
+++ b/docs/system/arm/mps2.rst
@@ -1,7 +1,7 @@
-Arm MPS2 and MPS3 boards (``mps2-an385``, ``mps2-an386``, ``mps2-an500``, ``mps2-an505``, ``mps2-an511``, ``mps2-an521``, ``mps3-an524``, ``mps3-an547``)
-=========================================================================================================================================================
+Arm MPS2 and MPS3 boards (``mps2-an385``, ``mps2-an386``, ``mps2-an500``, ``mps2-an505``, ``mps2-an511``, ``mps2-an521``, ``mps3-an524``, ``mps3-an536``, ``mps3-an547``)
+=========================================================================================================================================================================
 
-These board models all use Arm M-profile CPUs.
+These board models use Arm M-profile or R-profile CPUs.
 
 The Arm MPS2, MPS2+ and MPS3 dev boards are FPGA based (the 2+ has a
 bigger FPGA but is otherwise the same as the 2; the 3 has a bigger
@@ -13,6 +13,8 @@ FPGA image.
 
 QEMU models the following FPGA images:
 
+FPGA images using M-profile CPUs:
+
 ``mps2-an385``
   Cortex-M3 as documented in Arm Application Note AN385
 ``mps2-an386``
@@ -30,6 +32,11 @@ QEMU models the following FPGA images:
 ``mps3-an547``
   Cortex-M55 on an MPS3, as documented in Arm Application Note AN547
 
+FPGA images using R-profile CPUs:
+
+``mps3-an536``
+  Dual Cortex-R52 on an MPS3, as documented in Arm Application Note AN536
+
 Differences between QEMU and real hardware:
 
 - AN385/AN386 remapping of low 16K of memory to either ZBT SSRAM1 or to
@@ -45,6 +52,30 @@ Differences between QEMU and real hardware:
   flash, but only as simple ROM, so attempting to rewrite the flash
   from the guest will fail
 - QEMU does not model the USB controller in MPS3 boards
+- AN536 does not support runtime control of CPU reset and halt via
+  the SCC CFG_REG0 register.
+- AN536 does not support enabling or disabling the flash and ATCM
+  interfaces via the SCC CFG_REG1 register.
+- AN536 does not support setting of the initial vector table
+  base address via the SCC CFG_REG6 and CFG_REG7 register config,
+  and does not provide a mechanism for specifying these values at
+  startup, so all guest images must be built to start from TCM
+  (i.e. to expect the interrupt vector base at 0 from reset).
+- AN536 defaults to only creating a single CPU; this is the equivalent
+  of the way the real FPGA image usually runs with the second Cortex-R52
+  held in halt via the initial SCC CFG_REG0 register setting. You can
+  create the second CPU with ``-smp 2``; both CPUs will then start
+  execution immediately on startup.
+
+Note that for the AN536 the first UART is accessible only by
+CPU0, and the second UART is accessible only by CPU1. The
+first UART accessible shared between both CPUs is the third
+UART. Guest software might therefore be built to use either
+the first UART or the third UART; if you don't see any output
+from the UART you are looking at, try one of the others.
+(Even if the AN536 machine is started with a single CPU and so
+no "CPU1-only UART", the UART numbering remains the same,
+with the third UART being the first of the shared ones.)
 
 Machine-specific options
 """"""""""""""""""""""""
-- 
2.34.1

Re: [PATCH 13/13] docs: Add documentation for the mps3-an536 board

[Philippe Mathieu-Daudé]

On 6/2/24 14:29, Peter Maydell wrote:
> Add documentation for the mps3-an536 board type.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   docs/system/arm/mps2.rst | 37 ++++++++++++++++++++++++++++++++++---
>   1 file changed, 34 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>