On Fri, Dec 27, 2019 at 03:42:34PM +0000, Alexandru Elisei wrote:
Hi,
On 12/20/19 3:05 PM, Mark Rutland wrote:
When KVM injects an exception into a guest, it generates the CPSR value from scratch, configuring CPSR.{M,A,I,T,E}, and setting all other bits to zero.
This isn't correct, as the architecture specifies that some CPSR bits are (conditionally) cleared or set upon an exception, and others are unchanged from the original context.
This patch adds logic to match the architectural behaviour. To make this simple to follow/audit/extend, documentation references are provided, and bits are configured in order of their layout in SPSR_EL2. This
This patch applies equally well to 32bit KVM, where we have a SPSR register.
Indeed. The above wasn't intended to imply otherwise, but I'll add the following to make that clear:
| Note that this code is used by both arm and arm64, and is intended to | fuction with the SPSR_EL2 and SPSR_hyp layouts.
[...]
/* arm64 compatibility macros */ +#define PSR_AA32_MODE_FIQ FIQ_MODE +#define PSR_AA32_MODE_SVC SVC_MODE #define PSR_AA32_MODE_ABT ABT_MODE #define PSR_AA32_MODE_UND UND_MODE #define PSR_AA32_T_BIT PSR_T_BIT +#define PSR_AA32_F_BIT PSR_F_BIT #define PSR_AA32_I_BIT PSR_I_BIT #define PSR_AA32_A_BIT PSR_A_BIT #define PSR_AA32_E_BIT PSR_E_BIT #define PSR_AA32_IT_MASK PSR_IT_MASK +#define PSR_AA32_GE_MASK 0x000f0000 +#define PSR_AA32_PAN_BIT 0x00400000 +#define PSR_AA32_SSBS_BIT 0x00800000 +#define PSR_AA32_DIT_BIT 0x01000000
This is actually PSR_J_BIT on arm. Maybe it's worth defining it like that to make the overlap obvious.
Another bug! These are intended to match the SPSR_hyp view, where DIT is bit 21, and so this should be:
#define PSR_AA32_DIT_BIT 0x0x00200000
... placed immediately before the PAN bit.
We don't need a macro for the J bit as it was obsoleted and made RES0 by the ARMv7 virtualization extensions.
[...]
- // CPSR.PAN is unchanged unless overridden by SCTLR.SPAN
- // See ARM DDI 0487E.a, page G8-6246
- new |= (old & PSR_AA32_PAN_BIT);
- if (sctlr & BIT(23))
new |= PSR_AA32_PAN_BIT;PSTATE.PAN is unconditionally set when SCTLR.SPAN is clear.
Indeed, and this time the reference is explicit about that. :/
I've updated this to:
| // CPSR.PAN is unchanged unless SCTLR.SPAN == 0b0 | // SCTLR.SPAN is RES1 when ARMv8.1-PAN is not implemented | // See ARM DDI 0487E.a, page G8-6246 | new |= (old & PSR_AA32_PAN_BIT); | if (!(sctlr & BIT(23))) | new |= PSR_AA32_PAN_BIT;
[...]
I've also checked that the ARM documentation references match the code.
Thanks, your review is much appreciated!
Mark.