Re: [Qemu-devel] [PATCH] arm: Add ARMv6-M programmer's model support

[Peter Maydell]

On 13 July 2018 at 11:30, Julia Suvorova <address@hidden> wrote:
> Forbid stack alignment change. (CCR)
> Reserve FAULTMASK, BASEPRI registers.
> Report any fault as HardFault. Disable MemManage, BusFault and
> UsageFault, so they always escalated to HardFault. (SHCSR)
>
> Signed-off-by: Julia Suvorova <address@hidden>
> ---
> This is the last cortex-m0 patch.
>
>  hw/intc/armv7m_nvic.c | 10 ++++++++++
>  target/arm/cpu.c      | 10 ++++++++++
>  target/arm/helper.c   | 13 +++++++++++--
>  3 files changed, 31 insertions(+), 2 deletions(-)

Most of this looks good; I have some comments on the reset value of CCR.

> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index a914ce4e8c..3788cb773d 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -220,6 +220,11 @@ static void arm_cpu_reset(CPUState *s)
>              env->v7m.aircr = R_V7M_AIRCR_BFHFNMINS_MASK;
>          }
>
> +        if (!arm_feature(env, ARM_FEATURE_V7)) {
> +            env->v7m.ccr[M_REG_NS] = 0x3f8;
> +            env->v7m.ccr[M_REG_S] = 0x3f8;

This code will have no effect, because just below we already have an
assignment to these fields:
        env->v7m.ccr[M_REG_NS] = R_V7M_CCR_STKALIGN_MASK;
        env->v7m.ccr[M_REG_S] = R_V7M_CCR_STKALIGN_MASK;

> +        }
> +
>          /* In v7M the reset value of this bit is IMPDEF, but ARM recommends
>           * that it resets to 1, so QEMU always does that rather than making
>           * it dependent on CPU model. In v8M it is RES1.
> @@ -230,6 +235,11 @@ static void arm_cpu_reset(CPUState *s)
>              /* in v8M the NONBASETHRDENA bit [0] is RES1 */
>              env->v7m.ccr[M_REG_NS] |= R_V7M_CCR_NONBASETHRDENA_MASK;
>              env->v7m.ccr[M_REG_S] |= R_V7M_CCR_NONBASETHRDENA_MASK;
> +
> +            if (!arm_feature(env, ARM_FEATURE_M_MAIN)) {
> +                env->v7m.ccr[M_REG_NS] |= R_V7M_CCR_UNALIGN_TRP_MASK;
> +                env->v7m.ccr[M_REG_S] |= R_V7M_CCR_UNALIGN_TRP_MASK;
> +            }

This should be outside the "if v8" if(), because you also want it for v6M
(giving you the v6M CCR value of STKALIGN and UNALIGN_TRP set and all
other bits clear).

thanks
-- PMM