On Tue, 16 May 2023 17:53:14 +0100, Eric Auger <eauger@xxxxxxxxxx> wrote: > > Hi Marc, > > On 5/15/23 19:30, Marc Zyngier wrote: > > This is the 4th drop of NV support on arm64 for this year. > > > > For the previous episodes, see [1]. > > > > What's changed: > > > > - New framework to track system register traps that are reinjected in > > guest EL2. It is expected to replace the discrete handling we have > > enjoyed so far, which didn't scale at all. This has already fixed a > > number of bugs that were hidden (a bunch of traps were never > > forwarded...). Still a work in progress, but this is going in the > > right direction. > > > > - Allow the L1 hypervisor to have a S2 that has an input larger than > > the L0 IPA space. This fixes a number of subtle issues, depending on > > how the initial guest was created. > > > > - Consequently, the patch series has gone longer again. Boo. But > > hopefully some of it is easier to review... > > > > [1] https://lore.kernel.org/r/20230405154008.3552854-1-maz@xxxxxxxxxx > > I have started testing this and when booting my fedora guest I get > > [ 151.796544] kvm [7617]: Unsupported guest sys_reg access at: > 23f425fd0 [80000209] > [ 151.796544] { Op0( 3), Op1( 3), CRn(14), CRm( 3), Op2( 1), func_write }, > > as soon as the host has kvm-arm.mode=nested > > This seems to be triggered very early by EDK2 > (ArmPkg/Drivers/TimerDxe/TimerDxe.c). > > If I am not wrong this CNTV_CTL_EL0. Do you have any idea? So here's my current analysis: I assume you are running EDK2 as the L1 guest in a nested configuration. I also assume that you are not running on an Apple CPU. If these assumptions are correct, then EDK2 runs at vEL2, and is in nVHE mode. Finally, I'm going to assume that your implementation has FEAT_ECV and FEAT_NV2, because I can't see how it could fail otherwise. In these precise conditions, KVM sets the CNTHCTL_EL2.EL1TVT bit so that we can trap the EL0 virtual timer and faithfully emulate it (it is otherwise written to memory, which isn't very helpful). As it turns out, we don't handle these traps. I didn't spot it because my test machines are all Apple boxes that don't have a nVHE mode, so nothing on the nVHE path is getting *ANY* coverage. Hint: having access to such a machine would help (shipping address on request!). Otherwise, I'll eventually kill the nVHE support altogether. I have written the following patch, which compiles, but that I cannot test with my current setup. Could you please give it a go? Thanks again, M. >From feb03b57de0bcb83254a2d6a3ce320f5e39434b6 Mon Sep 17 00:00:00 2001 From: Marc Zyngier <maz@xxxxxxxxxx> Date: Tue, 16 May 2023 21:06:20 +0100 Subject: [PATCH] KVM: arm64: Handle virtual timer traps when CNTHCTL_EL2.EL1TVT is set Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx> --- arch/arm64/include/asm/sysreg.h | 1 + arch/arm64/kvm/sys_regs.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 72ff6df5d75b..77a61179ea37 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -436,6 +436,7 @@ #define SYS_CNTP_CTL_EL0 sys_reg(3, 3, 14, 2, 1) #define SYS_CNTP_CVAL_EL0 sys_reg(3, 3, 14, 2, 2) +#define SYS_CNTV_TVAL_EL0 sys_reg(3, 3, 14, 3, 0) #define SYS_CNTV_CTL_EL0 sys_reg(3, 3, 14, 3, 1) #define SYS_CNTV_CVAL_EL0 sys_reg(3, 3, 14, 3, 2) diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 27a29dcbfcd2..9aa9c4e4b4d6 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1328,6 +1328,14 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu, treg = TIMER_REG_TVAL; break; + case SYS_CNTV_TVAL_EL0: + if (is_hyp_ctxt(vcpu) && vcpu_el2_e2h_is_set(vcpu)) + tmr = TIMER_HVTIMER; + else + tmr = TIMER_VTIMER; + treg = TIMER_REG_TVAL; + break; + case SYS_AARCH32_CNTP_TVAL: case SYS_CNTP_TVAL_EL02: tmr = TIMER_PTIMER; @@ -1357,6 +1365,14 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu, treg = TIMER_REG_CTL; break; + case SYS_CNTV_CTL_EL0: + if (is_hyp_ctxt(vcpu) && vcpu_el2_e2h_is_set(vcpu)) + tmr = TIMER_HVTIMER; + else + tmr = TIMER_VTIMER; + treg = TIMER_REG_CTL; + break; + case SYS_AARCH32_CNTP_CTL: case SYS_CNTP_CTL_EL02: tmr = TIMER_PTIMER; @@ -1386,6 +1402,14 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu, treg = TIMER_REG_CVAL; break; + case SYS_CNTV_CVAL_EL0: + if (is_hyp_ctxt(vcpu) && vcpu_el2_e2h_is_set(vcpu)) + tmr = TIMER_HVTIMER; + else + tmr = TIMER_VTIMER; + treg = TIMER_REG_CVAL; + break; + case SYS_AARCH32_CNTP_CVAL: case SYS_CNTP_CVAL_EL02: tmr = TIMER_PTIMER; @@ -2510,6 +2534,10 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_CNTP_CTL_EL0), access_arch_timer }, { SYS_DESC(SYS_CNTP_CVAL_EL0), access_arch_timer }, + { SYS_DESC(SYS_CNTV_TVAL_EL0), access_arch_timer }, + { SYS_DESC(SYS_CNTV_CTL_EL0), access_arch_timer }, + { SYS_DESC(SYS_CNTV_CVAL_EL0), access_arch_timer }, + /* PMEVCNTRn_EL0 */ PMU_PMEVCNTR_EL0(0), PMU_PMEVCNTR_EL0(1), -- 2.39.2 -- Without deviation from the norm, progress is not possible.
