Hi Marc, On 5/16/23 22:28, Marc Zyngier wrote: > 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. all the above is correct. > > 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). indeed > > 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? with the patch below, my guest boots nicely. You did it great on the 1st shot!!! So this fixes my issue. I will continue testing the v10. Thank you again! Eric > > 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),
