Thank you Philippe and the QEMU team! --Randy
Philippe Mathieu-Daudé <f4...@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.hender...@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.mayd...@linaro.org> >>> --- >>> Bug reported on IRC. > > Similar to commit a1ecb4381829d7: > > Reported-by: Randy Yates <ya...@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é <f4...@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
