arm/arm64: KVM: Handle traps of ICC_SRE_EL1 as RAZ/WI
Subsystems (3)
PATCH 1/1 arm/arm64: KVM: Handle traps of ICC_SRE_EL1 as RAZ/WI
From: Christoffer Dall <[email protected]>
When running on a system with a GICv3, we currenly don't allow the guest to access the system register interface of the GICv3. We do this by clearing the ICC_SRE_EL2.Enable, which causes all guest accesses to ICC_SRE_EL1 to trap to EL2 and causes all guest accesses to other ICC_ registers to cause an undefined exception in the guest.
However, we currently don't handle the trap of guest accesses to ICC_SRE_EL1 and will spill out a warning. The trap just needs to handle the access as RAZ/WI, and a guest that tries to prod this register and set ICC_SRE_EL1.SRE=1, must read back the value (which Linux already does) to see if it succeeded, and will thus observe that ICC_SRE_EL1.SRE was not set.
Add the simple trap handler in the sorted table of the system registers.
--
v2: added 32-bit counterpart, renaming pm_fake to trap_raz_wi in the process,
hence the additional changes on that side
--- arch/arm/kvm/coproc.c | 49 +++++++++++++++++++++++++---------------------- arch/arm64/kvm/sys_regs.c | 5 +++++ 2 files changed, 31 insertions(+), 23 deletions(-)
arch/arm/kvm/coproc.c +26 -23
@@ -268,6 +268,16 @@ bool access_sctlr(struct kvm_vcpu *vcpu,return true; } +static bool trap_raz_wi (struct kvm_vcpu *vcpu, +const struct coproc_params *p, +const struct coproc_reg *r) +{ +if (p->is_write) +return ignore_write (vcpu, p); +else +return read_zero (vcpu, p); +} +/* * We could trap ID_DFR0 and tell the guest we don't support performance * monitoring. Unfortunately the patch to make the kernel check ID_DFR0 was
@@ -277,29 +287,19 @@ bool access_sctlr(struct kvm_vcpu *vcpu,* must always support PMCCNTR (the cycle counter): we just RAZ/WI for * all PM registers, which doesn't crash the guest kernel at least. */ -static bool pm_fake (struct kvm_vcpu *vcpu, -const struct coproc_params *p, -const struct coproc_reg *r) -{ -if (p->is_write) -return ignore_write (vcpu, p); -else -return read_zero (vcpu, p); -} - -#define access_pmcr pm_fake -#define access_pmcntenset pm_fake -#define access_pmcntenclr pm_fake -#define access_pmovsr pm_fake -#define access_pmselr pm_fake -#define access_pmceid0 pm_fake -#define access_pmceid1 pm_fake -#define access_pmccntr pm_fake -#define access_pmxevtyper pm_fake -#define access_pmxevcntr pm_fake -#define access_pmuserenr pm_fake -#define access_pmintenset pm_fake -#define access_pmintenclr pm_fake +#define access_pmcr trap_raz_wi +#define access_pmcntenset trap_raz_wi +#define access_pmcntenclr trap_raz_wi +#define access_pmovsr trap_raz_wi +#define access_pmselr trap_raz_wi +#define access_pmceid0 trap_raz_wi +#define access_pmceid1 trap_raz_wi +#define access_pmccntr trap_raz_wi +#define access_pmxevtyper trap_raz_wi +#define access_pmxevcntr trap_raz_wi +#define access_pmuserenr trap_raz_wi +#define access_pmintenset trap_raz_wi +#define access_pmintenclr trap_raz_wi/* Architected CP15 registers. * CRn denotes the primary register number, but is copied to the CRm in the
@@ -405,6 +405,9 @@ static const struct coproc_reg cp15_regs[] = { {CRn (12 ),CRm (0 ),Op1 (0 ),Op2 (0 ), is32, NULL, reset_val, c12_VBAR, 0x00000000 }, +/* ICC_SRE */ + { CRn(12), CRm(12), Op1( 0), Op2( 5), is32, trap_raz_wi }, +/* CONTEXTIDR/TPIDRURW/TPIDRURO/TPIDRPRW: swapped by interrupt.S. */ { CRn(13), CRm( 0), Op1( 0), Op2( 1), is32, access_vm_reg, reset_val, c13_CID, 0x00000000 },
arch/arm64/kvm/sys_regs.c +5 -0
@@ -424,6 +424,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {/* VBAR_EL1 */ {Op0 (0b11 ),Op1 (0b000 ),CRn (0b1100 ),CRm (0b0000 ),Op2 (0b000 ), NULL, reset_val, VBAR_EL1, 0 }, + +/* ICC_SRE_EL1 */ + { Op0(0b11), Op1(0b000), CRn(0b1100), CRm(0b1100), Op2(0b101), + trap_raz_wi }, +/* CONTEXTIDR_EL1 */ { Op0(0b11), Op1(0b000), CRn(0b1101), CRm(0b0000), Op2(0b001), access_vm_reg, reset_val, CONTEXTIDR_EL1, 0 },
Signature · 1 line
1.8.3.2
PATCH 1/1 arm/arm64: KVM: Handle traps of ICC_SRE_EL1 as RAZ/WI
Quoted 20 lines · Ard Biesheuvel
From: Christoffer Dall <[email protected]>
When running on a system with a GICv3, we currenly don't allow the guest to access the system register interface of the GICv3. We do this by clearing the ICC_SRE_EL2.Enable, which causes all guest accesses to
ICC_SRE_EL1to trap to EL2 and causes all guest accesses to otherICC_registers to cause an undefined exception in the guest.However, we currently don't handle the trap of guest accesses to
ICC_SRE_EL1and will spill out a warning. The trap just needs to handle the access as RAZ/WI, and a guest that tries to prod this register and set ICC_SRE_EL1.SRE=1, must read back the value (which Linux already does) to see if it succeeded, and will thus observe that ICC_SRE_EL1.SRE was not set.Add the simple trap handler in the sorted table of the system registers.
Signed-off-by Christoffer Dall <[email protected]>[ardb: added 32-bit counterpart]Signed-off-by Ard Biesheuvel <[email protected]>
Quoted 8 lines
--
v2: added 32-bit counterpart, renamingpm_faketotrap_raz_wiin the process,
hence the additional changes on that side--- arch/arm/kvm/coproc.c | 49 +++++++++++++++++++++++++---------------------- arch/arm64/kvm/sys_regs.c | 5 +++++ 2 files changed, 31 insertions(+), 23 deletions(-)
Err... Not really. What I meant was to update the cp15_regs array in arch/arm64/kvm/sys_regs.c. The 32bit port of KVM is unlikely to ever grow support for GICv3, as these registers are *not* defined for ARMv7.
Thanks,
M.
Signature · 1 line
Jazz is not dead. It just smells funny.