On Sun, 6 Sept 2026 at 20:44, Gilles Grimaud <[email protected]> wrote: > > Cortex-M0+ implementations may provide an Armv6-M PMSA MPU with eight > regions. QEMU uses its historical v7m/PMSAv7 naming for M-profile code > shared by v6-M and v7-M, but currently exposes, allocates and migrates the > existing region model only when ARM_FEATURE_V7 is set. This prevents a > Cortex-M0+ board from enabling its optional MPU. > > Allow non-v8 M-profile CPUs to use the existing PMSAv7 state, migration > state and translation path. Restrict the shared register model to the v6-M > layout: do not expose the v7-M alias registers or TEX bits, and reject > region sizes below the v6-M minimum of 256 bytes. > > The dedicated PMSAv8 path remains selected first for v8-M CPUs. Expose the > pmsav7-dregion property and clear region state on reset.
I think I would split this into more than one patch; all these things are distinct changes: * don't expose RBAR and RASR alias registers when Main Extension not present * TEX bits are RES0 * region size minimum check is wrong * change if() conditions to include v6M when we are asking "is this PMSAv7?" > Signed-off-by: gilles grimaud <[email protected]> > --- > hw/intc/armv7m_nvic.c | 27 ++++++++++++++++++++++++++- > target/arm/cpu.c | 12 +++++++++--- > target/arm/machine.c | 3 ++- > target/arm/ptw.c | 10 +++++++--- > 4 files changed, 44 insertions(+), 8 deletions(-) > > diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c > index c1ceb7450f..a702bd0c38 100644 > --- a/hw/intc/armv7m_nvic.c > +++ b/hw/intc/armv7m_nvic.c > @@ -1405,6 +1405,11 @@ static uint32_t nvic_readl(NVICState *s, uint32_t > offset, MemTxAttrs attrs) > return cpu->env.pmsav8.rbar[attrs.secure][region]; > } > > + if (offset != 0xd9c && > + !arm_feature(&cpu->env, ARM_FEATURE_V7)) { > + goto bad_offset; > + } > + > if (region >= cpu->pmsav7_dregion) { > return 0; > } > @@ -1432,6 +1437,11 @@ static uint32_t nvic_readl(NVICState *s, uint32_t > offset, MemTxAttrs attrs) > return cpu->env.pmsav8.rlar[attrs.secure][region]; > } > > + if (offset != 0xda0 && > + !arm_feature(&cpu->env, ARM_FEATURE_V7)) { > + goto bad_offset; > + } > + > if (region >= cpu->pmsav7_dregion) { > return 0; > } > @@ -1919,6 +1929,11 @@ static void nvic_writel(NVICState *s, uint32_t offset, > uint32_t value, > return; > } > > + if (offset != 0xd9c && > + !arm_feature(&cpu->env, ARM_FEATURE_V7)) { > + goto bad_offset; > + } > + > if (value & (1 << 4)) { > /* VALID bit means use the region number specified in this > * value and also update MPU_RNR.REGION with that value. > @@ -1969,12 +1984,22 @@ static void nvic_writel(NVICState *s, uint32_t > offset, uint32_t value, > return; > } > > + if (offset != 0xda0 && > + !arm_feature(&cpu->env, ARM_FEATURE_V7)) { > + goto bad_offset; > + } These alias registers don't exist on v8M without the Main Extension either. We should model that by rearranging the case lines like this: case 0xda4: /* MPU_RBAR_A1 */ case 0xdac: /* MPU_RBAR_A2 */ case 0xdb4: /* MPU_RBAR_A3 */ if (!arm_feature(&cpu->env, ARM_FEATURE_M_MAIN)) { /* These aliases are not present for v6M or v8M without Main */ goto bad_offset; } /* fall through */ case 0xd9c: /* MPU_RBAR */ { [existing code here] (similarly for the other 3 places.) > + > if (region >= cpu->pmsav7_dregion) { > return; > } > > cpu->env.pmsav7.drsr[region] = value & 0xff3f; > - cpu->env.pmsav7.dracr[region] = (value >> 16) & 0x173f; > + if (arm_feature(&cpu->env, ARM_FEATURE_V7)) { > + cpu->env.pmsav7.dracr[region] = (value >> 16) & 0x173f; > + } else { > + /* Armv6-M has XN, AP, S, C and B, but no TEX field. */ > + cpu->env.pmsav7.dracr[region] = (value >> 16) & 0x1707; > + } > tlb_flush(CPU(cpu)); > break; > } This part is OK. > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 77aa78f00e..f58a1db843 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -617,7 +617,8 @@ static void arm_cpu_reset_hold(Object *obj, ResetType > type) > sizeof(*env->pmsav8.rlar[M_REG_S]) > * cpu->pmsav7_dregion); > } > - } else if (arm_feature(env, ARM_FEATURE_V7)) { > + } else if (arm_feature(env, ARM_FEATURE_V7) || > + arm_feature(env, ARM_FEATURE_M)) { > memset(env->pmsav7.drbar, 0, > sizeof(*env->pmsav7.drbar) * cpu->pmsav7_dregion); > memset(env->pmsav7.drsr, 0, > @@ -1656,7 +1657,11 @@ static void arm_cpu_post_init(Object *obj) > #ifndef CONFIG_USER_ONLY > if (arm_feature(&cpu->env, ARM_FEATURE_PMSA)) { > qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property); > - if (arm_feature(&cpu->env, ARM_FEATURE_V7)) { > + /* > + * QEMU's PMSAv7 state also models the Armv6-M MPU register layout. > + */ > + if (arm_feature(&cpu->env, ARM_FEATURE_V7) || > + arm_feature(&cpu->env, ARM_FEATURE_M)) { > qdev_property_add_static(DEVICE(obj), > &arm_cpu_pmsav7_dregion_property); > } > @@ -2332,7 +2337,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error > **errp) > } > > if (arm_feature(env, ARM_FEATURE_PMSA) && > - arm_feature(env, ARM_FEATURE_V7)) { > + (arm_feature(env, ARM_FEATURE_V7) || > + arm_feature(env, ARM_FEATURE_M))) { > uint32_t nr = cpu->pmsav7_dregion; > > if (nr > 0xff) { > diff --git a/target/arm/machine.c b/target/arm/machine.c > index 7005f4e5d7..53a2734d25 100644 > --- a/target/arm/machine.c > +++ b/target/arm/machine.c > @@ -584,7 +584,8 @@ static bool pmsav7_needed(void *opaque) > CPUARMState *env = &cpu->env; > > return arm_feature(env, ARM_FEATURE_PMSA) && > - arm_feature(env, ARM_FEATURE_V7) && > + (arm_feature(env, ARM_FEATURE_V7) || > + arm_feature(env, ARM_FEATURE_M)) && > !arm_feature(env, ARM_FEATURE_V8); > } > > diff --git a/target/arm/ptw.c b/target/arm/ptw.c > index a29de0385f..6cb8a8394a 100644 > --- a/target/arm/ptw.c > +++ b/target/arm/ptw.c > @@ -2748,9 +2748,12 @@ static bool get_phys_addr_pmsav7(CPUARMState *env, > continue; > } > > - if (!rsize) { > + if (!rsize || > + (arm_feature(env, ARM_FEATURE_M) && > + !arm_feature(env, ARM_FEATURE_V7) && rsize < 7)) { > qemu_log_mask(LOG_GUEST_ERROR, > - "DRSR[%d]: Rsize field cannot be 0\n", n); > + "DRSR[%d]: invalid Rsize field 0x%x\n", > + n, rsize); > continue; > } > rsize++; We don't get this "minimum size check" right for v7M either, because that wants a SIZE field of at least 4. The zero check is only correct for R-profile. It's not strictly necessary to fix this bug as part of adding cortex-m0+ support, because we already get this wrong for other M-profile CPUs, and a correctly programmed guest won't run into it. If we want to do it, then better as a separate patch that fixes both v6M and v7M, something like: /* minimum valid value for rsize varies between v7R, v6M, v7M */ uint32_t rsize_min = 1; if (arm_feature(env, ARM_FEATURE_M) { rsize_min = arm_feature(env, ARM_FEATURE_V7) ? 4 : 7; } for (n = ... ) { ... if (rsize < rsize_min) { log and continue; } > @@ -3901,7 +3904,8 @@ static bool get_phys_addr_nogpc(CPUARMState *env, > S1Translate *ptw, > /* PMSAv8 */ > ret = get_phys_addr_pmsav8(env, ptw, address, access_type, > result, fi); > - } else if (arm_feature(env, ARM_FEATURE_V7)) { > + } else if (arm_feature(env, ARM_FEATURE_V7) || > + arm_feature(env, ARM_FEATURE_M)) { > /* PMSAv7 */ > ret = get_phys_addr_pmsav7(env, ptw, address, access_type, > result, fi); I think these other if condition changes are all OK. thanks -- PMM
