AArch32 CP1x registers are overlayed on their AArch64 counterparts in the vcpu struct. This leads to an interesting problem as they are stored in their CPU-local format, and thus a CP1x register doesn't "hit" the lower 32bit portion of the AArch64 register on a BE host.
To workaround this unfortunate situation, introduce a bias trick in the vcpu_cp1x() accessors which picks the correct half of the 64bit register.
Cc: stable@vger.kernel.org Reported-by: James Morse james.morse@arm.com Signed-off-by: Marc Zyngier maz@kernel.org --- arch/arm64/include/asm/kvm_host.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 59029e90b557..e80c0e06f235 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -404,8 +404,14 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg); * CP14 and CP15 live in the same array, as they are backed by the * same system registers. */ -#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r)]) -#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r)]) +#ifdef CPU_BIG_ENDIAN +#define CPx_OFFSET 1 +#else +#define CPx_OFFSET 0 +#endif + +#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r) ^ CPx_OFFSET]) +#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r) ^ CPx_OFFSET])
struct kvm_vm_stat { ulong remote_tlb_flush;
On 2020-06-09 09:49, Marc Zyngier wrote:
AArch32 CP1x registers are overlayed on their AArch64 counterparts in the vcpu struct. This leads to an interesting problem as they are stored in their CPU-local format, and thus a CP1x register doesn't "hit" the lower 32bit portion of the AArch64 register on a BE host.
To workaround this unfortunate situation, introduce a bias trick in the vcpu_cp1x() accessors which picks the correct half of the 64bit register.
Cc: stable@vger.kernel.org Reported-by: James Morse james.morse@arm.com Signed-off-by: Marc Zyngier maz@kernel.org
arch/arm64/include/asm/kvm_host.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 59029e90b557..e80c0e06f235 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -404,8 +404,14 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg);
- CP14 and CP15 live in the same array, as they are backed by the
- same system registers.
*/ -#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r)]) -#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r)]) +#ifdef CPU_BIG_ENDIAN
Ahem... I think you're missing a "CONFIG_" there ;)
Bonus trickery - for a 0 or 1 value you can simply use IS_ENABLED().
Robin.
+#define CPx_OFFSET 1 +#else +#define CPx_OFFSET 0 +#endif
+#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r) ^ CPx_OFFSET]) +#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r) ^ CPx_OFFSET]) struct kvm_vm_stat { ulong remote_tlb_flush;
Hi Robin,
On 2020-06-09 12:41, Robin Murphy wrote:
On 2020-06-09 09:49, Marc Zyngier wrote:
AArch32 CP1x registers are overlayed on their AArch64 counterparts in the vcpu struct. This leads to an interesting problem as they are stored in their CPU-local format, and thus a CP1x register doesn't "hit" the lower 32bit portion of the AArch64 register on a BE host.
To workaround this unfortunate situation, introduce a bias trick in the vcpu_cp1x() accessors which picks the correct half of the 64bit register.
Cc: stable@vger.kernel.org Reported-by: James Morse james.morse@arm.com Signed-off-by: Marc Zyngier maz@kernel.org
arch/arm64/include/asm/kvm_host.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 59029e90b557..e80c0e06f235 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -404,8 +404,14 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg);
- CP14 and CP15 live in the same array, as they are backed by the
- same system registers.
*/ -#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r)]) -#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r)]) +#ifdef CPU_BIG_ENDIAN
Ahem... I think you're missing a "CONFIG_" there ;)
Duh! As I said, I didn't test the thing at all! ;-)
Bonus trickery - for a 0 or 1 value you can simply use IS_ENABLED().
Beautiful! Definitely a must! :D
Thanks,
M.
Hi Marc, Robin,
On 09/06/2020 12:48, Marc Zyngier wrote:
On 2020-06-09 12:41, Robin Murphy wrote:
On 2020-06-09 09:49, Marc Zyngier wrote:
AArch32 CP1x registers are overlayed on their AArch64 counterparts in the vcpu struct. This leads to an interesting problem as they are stored in their CPU-local format, and thus a CP1x register doesn't "hit" the lower 32bit portion of the AArch64 register on a BE host.
To workaround this unfortunate situation, introduce a bias trick in the vcpu_cp1x() accessors which picks the correct half of the 64bit register.
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 59029e90b557..e80c0e06f235 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -404,8 +404,14 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg); * CP14 and CP15 live in the same array, as they are backed by the * same system registers. */ -#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r)]) -#define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r)]) +#ifdef CPU_BIG_ENDIAN
Ahem... I think you're missing a "CONFIG_" there ;)
Duh! As I said, I didn't test the thing at all! ;-)
Bonus trickery - for a 0 or 1 value you can simply use IS_ENABLED().
Beautiful! Definitely a must! :D
With Robin's suggestion of: ---------------%<--------------- diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 2a935457712b..54e9c7eb3596 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -405,11 +405,7 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg); * CP14 and CP15 live in the same array, as they are backed by the * same system registers. */ -#ifdef CPU_BIG_ENDIAN -#define CPx_OFFSET 1 -#else -#define CPx_OFFSET 0 -#endif +#define CPx_OFFSET IS_ENABLED(CONFIG_CPU_BIG_ENDIAN)
#define vcpu_cp14(v,r) ((v)->arch.ctxt.copro[(r) ^ CPx_OFFSET]) #define vcpu_cp15(v,r) ((v)->arch.ctxt.copro[(r) ^ CPx_OFFSET]) ---------------%<---------------
Tested-by: James Morse james.morse@arm.com Acked-by: James Morse james.morse@arm.com
Thanks,
James
----- Before this patch, an aarch32 guest of a BE host reading sysregs KVM is trap-and-undeffing gets: | Bad mode in prefetch abort handler detected | Internal error: Oops - bad mode: 0 [#1] SMP THUMB2 | Modules linked in: | CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0+ #260 | Hardware name: Generic DT based system | PC is at 0x4 | LR is at smp_cpus_done+0x85/0x98 | pc : [<00000004>] lr : [<808035cb>] psr: 6000009b | sp : 9f4a1f08 ip : 00000003 fp : 00000000 | r10: 00000000 r9 : 00000000 r8 : 00000000 | r7 : 80904ea8 r6 : 80904f6c r5 : 00000002 r4 : 000f4240 | r3 : bc605c12 r2 : bc605c12 r1 : 1f38c000 r0 : 0000c348 | Flags: nZCv IRQs off FIQs on Mode UND_32 ISA ARM Segment none | Control: 50c5383d Table: 8000406a DAC: bc605c12 | Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [...] | [<808035cb>] (smp_cpus_done) from [<00000002>] (0x2) | Code: bad PC value | ---[ end trace b37275bf489ca225 ]---
instead of the undef it so richly deserved: | Internal error: Oops - undefined instruction: 0 [#1] SMP THUMB2 | Modules linked in: | CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.7.0+ #260 | Hardware name: Generic DT based system | PC is at smp_cpus_done+0x88/0x98 | LR is at smp_cpus_done+0x85/0x98 | pc : [<808035ce>] lr : [<808035cb>] psr: 60000073 | sp : 9f495f50 ip : 00000001 fp : 00000000 | r10: 00000000 r9 : 00000000 r8 : 00000000 | r7 : 80904ea8 r6 : 80904f6c r5 : 00000001 r4 : 0007a120 | r3 : 7f3828d2 r2 : 7f3828d2 r1 : 1f39f000 r0 : 0000c348 | Flags: nZCv IRQs on FIQs off Mode SVC_32 ISA Thumb Segment none | Control: 50c5383d Table: 8000406a DAC: 00000051 | Process swapper/0 (pid: 1, stack limit = 0x(ptrval)) [...] | [<808035ce>] (smp_cpus_done) from [<80800f73>] (kernel_init_freeable+0xdf/0x204) | [<80800f73>] (kernel_init_freeable) from [<805aa2a7>] (kernel_init+0x7/0xc8) | [<805aa2a7>] (kernel_init) from [<80100159>] (ret_from_fork+0x11/0x38) | Code: f7ff f8b9 f24c 3048 (ee11) 1f30 | ---[ end trace 4c78dcd8460e6041 ]---
linux-stable-mirror@lists.linaro.org
-
James Morse -
Marc Zyngier -
Robin Murphy