Re: [PATCH] target/arm: Allow user-mode code to write CPSR.E via MSR

[Philippe Mathieu-Daudé]

On 5/15/20 11:26 PM, Richard Henderson wrote:
> On 5/15/20 11:50 AM, Peter Maydell wrote:
> > Using the MSR instruction to write to CPSR.E is deprecated, but it is
> > required to work from any mode including unprivileged code.  We were
> > incorrectly forbidding usermode code from writing it because
> > CPSR_USER did not include the CPSR_E bit.
> >
> > We use CPSR_USER in only three places:
> >   * as the mask of what to allow userspace MSR to write to CPSR
> >   * when deciding what bits a linux-user signal-return should be
> >     able to write from the sigcontext structure
> >   * in target_user_copy_regs() when we set up the initial
> >     registers for the linux-user process
> >
> > In the first two cases not being able to update CPSR.E is a
> > bug, and in the third case it doesn't matter because CPSR.E
> > is always 0 there. So we can fix both bugs by adding CPSR_E
> > to CPSR_EXEC.
>
> Wrong variable in description here.

Indeed CPSR_EXEC -> CPSR_USER typo.

> Otherwise,
> Reviewed-by: Richard Henderson <address@hidden>
>
>
> r~
>
> > (The recommended way to change CPSR.E is to use the 'SETEND'
> > instruction, which we do correctly allow from usermode code.)
> >
> > Signed-off-by: Peter Maydell <address@hidden>
> > ---
> > Bug reported on IRC.

Similar to commit a1ecb4381829d7:

Reported-by: Randy Yates <address@hidden>

> Quick-and-dirty test case at:
> >   https://people.linaro.org/~peter.maydell/msr-setend.c
> >
> >   target/arm/cpu.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> > index 5d995368d4f..677584e5da0 100644
> > --- a/target/arm/cpu.h
> > +++ b/target/arm/cpu.h
> > @@ -1230,7 +1230,7 @@ void pmu_init(ARMCPU *cpu);
> >   #define CACHED_CPSR_BITS (CPSR_T | CPSR_AIF | CPSR_GE | CPSR_IT | CPSR_Q \
> >       | CPSR_NZCV)
> >   /* Bits writable in user mode.  */
> > -#define CPSR_USER (CPSR_NZCV | CPSR_Q | CPSR_GE)
> > +#define CPSR_USER (CPSR_NZCV | CPSR_Q | CPSR_GE | CPSR_E)

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> >   /* Execution state bits.  MRS read as zero, MSR writes ignored.  */
> >   #define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J | CPSR_IL)
> >