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

[Randy Yates <yates@digitalsignallabs.com>]

Thank you Philippe and the QEMU team!

--Randy

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> 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 <richard.henderson@linaro.org>
>>
>>
>> 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 <peter.maydell@linaro.org>
>>> ---
>>> Bug reported on IRC.
>
> Similar to commit a1ecb4381829d7:
>
> Reported-by: Randy Yates <yates@ieee.org>
>
>> 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é <f4bug@amsat.org>
>
>>>   /* Execution state bits.  MRS read as zero, MSR writes ignored.  */
>>>   #define CPSR_EXEC (CPSR_T | CPSR_IT | CPSR_J | CPSR_IL)
>>>   
>>>
>>
>>
>

-- 
Randy Yates, DSP/Embedded Firmware Developer
Digital Signal Labs
http://www.digitalsignallabs.com