Re: [PATCH for-4.2] target/arm: Support EL0 v7m msr/mrs for CONFIG_USER_ONLY

[Peter Maydell]

On Mon, 18 Nov 2019 at 19:49, Richard Henderson
<address@hidden> wrote:
>
> Simply moving the non-stub helper_v7m_mrs/msr outside of
> !CONFIG_USER_ONLY is not an option, because of all of the
> other system-mode helpers that are called.
>
> But we can split out a few subroutines to handle the few
> EL0 accessible registers without duplicating code.
>
> Reported-by: Christophe Lyon <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/cpu.h      |   2 +
>  target/arm/m_helper.c | 110 ++++++++++++++++++++++++++----------------
>  2 files changed, 70 insertions(+), 42 deletions(-)
>
>  /* These should probably raise undefined insn exceptions.  */

This comment is now wrong -- all the 'dummy version for user
mode' helpers below it are either "known to never be called" or
have a "works for user-mode" implementation, so we can just delete it.

> -void HELPER(v7m_msr)(CPUARMState *env, uint32_t reg, uint32_t val)
> +void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
>  {
> -    ARMCPU *cpu = env_archcpu(env);
> +    uint32_t mask = extract32(maskreg, 8, 4);
> +    uint32_t reg = extract32(maskreg, 0, 8);
>
> -    cpu_abort(CPU(cpu), "v7m_msr %d\n", reg);
> +    switch (reg) {
> +    case 0 ... 7: /* xPSR sub-fields */
> +        v7m_msr_xpsr(env, mask, reg, val);
> +        break;
> +    case 20: /* CONTROL */
> +        /* There are no sub-fields that are actually writable from EL0. */
> +        break;

I feel like somebody's static analysis is probably going to complain
if we don't have a default case here:
   default:
        /* Writes to all other registers from EL0 are ignored */
        break;

Looks good otherwise, so I'll apply it to target-arm.next with those fixups.

thanks
-- PMM