KVM: arm64: Add NV timer support
Marc Zyngier
maintainercover KVM: arm64: Add NV timer support
Here's another batch of NV-related patches, this time bringing in most of the timer support for EL2 as well as nested guests.
The code is pretty convoluted for a bunch of reasons:
- FEAT_NV2 breaks the timer semantics by redirecting HW controls to memory, meaning that a guest could setup a timer and never see it firing until the next exit
- We go try hard to reflect the timer state in memory, but that's not great.
- With FEAT_ECV, we can finally correctly emulate the virtual timer, but this emulation is pretty costly
- As a way to make things suck less, we handle timer reads as early as possible, and only defer writes to the normal trap handling
- Finally, some implementations are badly broken, and require some hand-holding, irrespective of NV support. So we try and reuse the NV infrastructure to make them usable. This could be further optimised, but I'm running out of patience for this sort of HW.
What this is not implementing is support for CNTPOFF_EL2. It appears that the architecture doesn't let you correctly emulate it, so I guess this will be trap/emulate for the foreseeable future.
This series is on top of v6.13-rc1, and has been tested on my usual M2 setup, but also on a Snapdragon X1 Elite devkit. I would like to thank Qualcomm for the free hardware with no strings (nor support) attached!
If you are feeling brave, you can run the whole thing from [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-next
Marc Zyngier (11):
KVM: arm64: nv: Add handling of EL2-specific timer registers
KVM: arm64: nv: Sync nested timer state with FEAT_NV2
KVM: arm64: nv: Publish emulated timer interrupt state in the
in-memory state
KVM: arm64: nv: Use FEAT_ECV to trap access to EL0 timers
KVM: arm64: nv: Accelerate EL0 timer read accesses when FEAT_ECV in
use
KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor
context
KVM: arm64: Handle counter access early in non-HYP context
KVM: arm64: nv: Add trap routing for
CNTHCTL_EL2.EL1{NVPCT,NVVCT,TVT,TVCT} KVM: arm64: nv: Propagate CNTHCTL_EL2.EL1NV{P,V}CT bits
KVM: arm64: nv: Sanitise CNTHCTL_EL2
KVM: arm64: Work around x1e's CNTVOFF_EL2 bogosity
arch/arm64/include/asm/cputype.h | 2 + arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/include/asm/sysreg.h | 4 + arch/arm64/kernel/cpu_errata.c | 8 ++ arch/arm64/kernel/image-vars.h | 3 + arch/arm64/kvm/arch_timer.c | 177 +++++++++++++++++++++--- arch/arm64/kvm/arm.c | 3 + arch/arm64/kvm/emulate-nested.c | 58 +++++++- arch/arm64/kvm/hyp/include/hyp/switch.h | 39 ++++-- arch/arm64/kvm/hyp/nvhe/timer-sr.c | 16 ++- arch/arm64/kvm/hyp/vhe/switch.c | 84 +++++++++++ arch/arm64/kvm/nested.c | 15 ++ arch/arm64/kvm/sys_regs.c | 146 ++++++++++++++++++- arch/arm64/tools/cpucaps | 1 + include/clocksource/arm_arch_timer.h | 6 + include/kvm/arm_arch_timer.h | 23 +++ 16 files changed, 546 insertions(+), 41 deletions(-)
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 1/11 KVM: arm64: nv: Add handling of EL2-specific timer registers
Add the required handling for EL2 and EL02 registers, as well as EL1 registers used in the E2H context. This includes handling the virtual timer accesses when CNTHCTL_EL2.EL1TVT or CNTHCTL_EL2.EL1TVCT are set.
--- arch/arm64/include/asm/sysreg.h | 4 + arch/arm64/kvm/sys_regs.c | 143 +++++++++++++++++++++++++++ include/clocksource/arm_arch_timer.h | 2 + 3 files changed, 149 insertions(+)
arch/arm64/include/asm/sysreg.h +4 -0
@@ -477,6 +477,7 @@#define SYS_CNTFRQ_EL0 sys_reg(3, 3, 14, 0, 0)#define SYS_CNTPCT_EL0 sys_reg(3, 3, 14, 0, 1) +#define SYS_CNTVCT_EL0 sys_reg(3, 3, 14, 0, 2)#define SYS_CNTPCTSS_EL0 sys_reg(3, 3, 14, 0, 5)#define SYS_CNTVCTSS_EL0 sys_reg(3, 3, 14, 0, 6)
@@ -484,14 +485,17 @@#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)#define SYS_AARCH32_CNTP_TVAL sys_reg(0, 0, 14, 2, 0)#define SYS_AARCH32_CNTP_CTL sys_reg(0, 0, 14, 2, 1)#define SYS_AARCH32_CNTPCT sys_reg(0, 0, 0, 14, 0) +#define SYS_AARCH32_CNTVCT sys_reg(0, 1, 0, 14, 0)#define SYS_AARCH32_CNTP_CVAL sys_reg(0, 2, 0, 14, 0)#define SYS_AARCH32_CNTPCTSS sys_reg(0, 8, 0, 14, 0) +#define SYS_AARCH32_CNTVCTSS sys_reg(0, 9, 0, 14, 0)#define __PMEV_op2 (n) ((n) & 0x7)#define __CNTR_CRm (n) (0x8 | (((n) >> 3) & 0x3))
arch/arm64/kvm/sys_regs.c +143 -0
198 lines not on this page · raw message
include/clocksource/arm_arch_timer.h +2 -0
@@ -22,6 +22,8 @@#define CNTHCTL_EVNTDIR (1 << 3)#define CNTHCTL_EVNTI (0xF << 4)#define CNTHCTL_ECV (1 << 12) +#define CNTHCTL_EL1TVT (1 << 13) +#define CNTHCTL_EL1TVCT (1 << 14)enum arch_timer_reg {ARCH_TIMER_REG_CTRL ,
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 2/11 KVM: arm64: nv: Sync nested timer state with FEAT_NV2
In reply to Marc ZyngierEmulating the timers with FEAT_NV2 is a bit odd, as the timers can be reconfigured behind our back without the hypervisor even noticing. In the VHE case, that's an actual regression in the architecture...
--- arch/arm64/kvm/arch_timer.c | 44 ++++++++++++++++++++++++++++++++++++ arch/arm64/kvm/arm.c | 3 +++ include/kvm/arm_arch_timer.h | 1 + 3 files changed, 48 insertions(+)
arch/arm64/kvm/arch_timer.c +44 -0
@@ -905,6 +905,50 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)kvm_timer_blocking (vcpu); } +void kvm_timer_sync_nested (struct kvm_vcpu *vcpu) +{ +/* +* When NV2 is on, guest hypervisors have their EL0 timer register +* accesses redirected to the VNCR page. Any guest action taken on +* the timer is postponed until the next exit, leading to a very +* poor quality of emulation. +*/ +if (!is_hyp_ctxt (vcpu)) +return ; + +if (!vcpu_el2_e2h_is_set (vcpu)) { +/* +* A non-VHE guest hypervisor doesn't have any direct access +* to its timers: the EL2 registers trap (and the HW is +* fully emulated), while the EL0 registers access memory +* despite the access being notionally direct. Boo. +* +* We update the hardware timer registers with the +* latest value written by the guest to the VNCR page +* and let the hardware take care of the rest. +*/ +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTV_CTL_EL0 ),SYS_CNTV_CTL ); +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTV_CVAL_EL0 ),SYS_CNTV_CVAL ); +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTP_CTL_EL0 ),SYS_CNTP_CTL ); +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTP_CVAL_EL0 ),SYS_CNTP_CVAL ); + }else { +/* +* For a VHE guest hypervisor, the EL2 state is directly +* stored in the host EL0 timers, while the emulated EL0 +* state is stored in the VNCR page. The latter could have +* been updated behind our back, and we must reset the +* emulation of the timers. +*/ +struct timer_map map; +get_timer_map (vcpu, &map); + +soft_timer_cancel (&map.emul_vtimer->hrtimer); +soft_timer_cancel (&map.emul_ptimer->hrtimer); +timer_emulate (map.emul_vtimer); +timer_emulate (map.emul_ptimer); + } +} +/* * With a userspace irqchip we have to check if the guest de-asserted the * timer and if so, unmask the timer irq signal on the host interrupt
arch/arm64/kvm/arm.c +3 -0
@@ -1228,6 +1228,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)if (unlikely (!irqchip_in_kernel (vcpu->kvm)))kvm_timer_sync_user (vcpu); +if (vcpu_has_nv (vcpu)) +kvm_timer_sync_nested (vcpu); +kvm_arch_vcpu_ctxsync_fp (vcpu);/*
include/kvm/arm_arch_timer.h +1 -0
@@ -98,6 +98,7 @@ int __init kvm_timer_hyp_init(bool has_gic);int kvm_timer_enable (struct kvm_vcpu *vcpu);void kvm_timer_vcpu_reset (struct kvm_vcpu *vcpu);void kvm_timer_vcpu_init (struct kvm_vcpu *vcpu); +void kvm_timer_sync_nested (struct kvm_vcpu *vcpu);void kvm_timer_sync_user (struct kvm_vcpu *vcpu);bool kvm_timer_should_notify_user (struct kvm_vcpu *vcpu);void kvm_timer_update_run (struct kvm_vcpu *vcpu);
Signature · 1 line
2.39.2
Re: [PATCH 02/11] KVM: arm64: nv: Sync nested timer state with FEAT_NV2
Quoted 46 lines · Marc Zyngier
Emulating the timers with
FEAT_NV2is a bit odd, as the timers can be reconfigured behind our back without the hypervisor even noticing. In the VHE case, that's an actual regression in the architecture...Co-developed-by Christoffer Dall <[email protected]>Signed-off-by Christoffer Dall <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/arch_timer.c | 44 ++++++++++++++++++++++++++++++++++++ arch/arm64/kvm/arm.c | 3 +++ include/kvm/arm_arch_timer.h | 1 + 3 files changed, 48 insertions(+)arch/arm64/kvm/arch_timer.c +24
-0@@ -905,6 +905,50 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu)kvm_timer_blocking (vcpu); } +void kvm_timer_sync_nested (struct kvm_vcpu *vcpu) +{ +/* +* When NV2 is on, guest hypervisors have their EL0 timer register +* accesses redirected to the VNCR page. Any guest action taken on +* the timer is postponed until the next exit, leading to a very +* poor quality of emulation. +*/ +if (!is_hyp_ctxt (vcpu)) +return ; + +if (!vcpu_el2_e2h_is_set (vcpu)) { +/* +* A non-VHE guest hypervisor doesn't have any direct access +* to its timers: the EL2 registers trap (and the HW is +* fully emulated), while the EL0 registers access memory +* despite the access being notionally direct. Boo. +* +* We update the hardware timer registers with the +* latest value written by the guest to the VNCR page +* and let the hardware take care of the rest. +*/ +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTV_CTL_EL0 ),SYS_CNTV_CTL ); +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTV_CVAL_EL0 ),SYS_CNTV_CVAL );
arch/arm64/kvm/arch_timer.c void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu) · line 941
Quoted 10 lines
arch/arm64/kvm/arch_timer.c +10
-0+write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTP_CTL_EL0 ),SYS_CNTP_CTL ); +write_sysreg_el0 (__vcpu_sys_reg (vcpu,CNTP_CVAL_EL0 ),SYS_CNTP_CVAL ); + }else { +/* +* For a VHE guest hypervisor, the EL2 state is directly +* stored in the host EL0 timers, while the emulated EL0 +* state is stored in the VNCR page. The latter could have +* been updated behind our back, and we must reset the +* emulation of the timers. +*/
nitpick: s/host EL0/EL1/
At least in the way the architecture terms it there's no such thing as an EL0 timer, and "host EL0" might lead one to think of ELIsInHost(EL0)
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerPATCH 3/11 KVM: arm64: nv: Publish emulated timer interrupt state in the in-memory state
In reply to Marc ZyngierWith FEAT_NV2, the EL0 timer state is entirely stored in memory, meaning that the hypervisor can only provide a very poor emulation.
The only thing we can really do is to publish the interrupt state in the guest view of CNT{P,V}_CTL_EL0, and defer everything else to the next exit.
Only FEAT_ECV will allow us to fix it, at the cost of extra trapping.
--- arch/arm64/kvm/arch_timer.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
arch/arm64/kvm/arch_timer.c +19 -0
@@ -446,6 +446,25 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level, {int ret; +/* +* Paper over NV2 brokenness by publishing the interrupt status +* bit. This still results in a poor quality of emulation (guest +* writes will have no effect until the next exit). +* +* But hey, it's fast, right? +*/ +if (is_hyp_ctxt (vcpu) && + (timer_ctx ==vcpu_vtimer (vcpu) || timer_ctx ==vcpu_ptimer (vcpu))) { +u32 ctl =timer_get_ctl (timer_ctx); + +if (new_level) + ctl |=ARCH_TIMER_CTRL_IT_STAT ; +else + ctl &= ~ARCH_TIMER_CTRL_IT_STAT ; + +timer_set_ctl (timer_ctx, ctl); + } + timer_ctx->irq.level = new_level;trace_kvm_timer_update_irq (vcpu->vcpu_id,timer_irq (timer_ctx), timer_ctx->irq.level);
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 4/11 KVM: arm64: nv: Use FEAT_ECV to trap access to EL0 timers
In reply to Marc ZyngierAlthough FEAT_NV2 makes most things fast, it also makes it impossible to correctly emulate the timers, as the sysreg accesses are redirected to memory.
FEAT_ECV addresses this by giving a hypervisor the ability to trap the EL02 sysregs as well as the virtual timer.
Add the required trap setting to make use of the feature, allowing us to elide the ugly resync in the middle of the run loop.
--- arch/arm64/kvm/arch_timer.c | 36 +++++++++++++++++++++++++--- include/clocksource/arm_arch_timer.h | 2 ++ 2 files changed, 35 insertions(+), 3 deletions(-)
arch/arm64/kvm/arch_timer.c +33 -3
@@ -781,7 +781,7 @@ static void kvm_timer_vcpu_load_nested_switch(struct kvm_vcpu *vcpu,static void timer_set_traps (struct kvm_vcpu *vcpu,struct timer_map *map) { -bool tpt, tpc; +bool tvt, tpt, tvc, tpc, tvt02, tpt02;u64 clr, set;/*
@@ -796,7 +796,29 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)* within this function, reality kicks in and we start adding * traps based on emulation requirements. */ - tpt = tpc = false; + tvt = tpt = tvc = tpc = false; + tvt02 = tpt02 = false; + +/* +* NV2 badly breaks the timer semantics by redirecting accesses to +* the EL0 timer state to memory, so let's call ECV to the rescue if +* available: we trap all CNT{P,V}_{CTL,CVAL,TVAL}_EL0 accesses. +* +* The treatment slightly varies depending whether we run a nVHE or +* VHE guest: nVHE will use the _EL0 registers directly, while VHE +* will use the _EL02 accessors. This translates in different trap +* bits. +* +* None of the trapping is required when running in non-HYP context, +* unless required by the L1 hypervisor settings once we advertise +* ECV+NV in the guest, or that we need trapping for other reasons. +*/ +if (cpus_have_final_cap (ARM64_HAS_ECV ) &&is_hyp_ctxt (vcpu)) { +if (vcpu_el2_e2h_is_set (vcpu)) + tvt02 = tpt02 = true; +else + tvt = tpt = true; + }/* * We have two possibility to deal with a physical offset:
@@ -836,6 +858,10 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)assign_clear_set_bit (tpt,CNTHCTL_EL1PCEN <<10 , set, clr);assign_clear_set_bit (tpc,CNTHCTL_EL1PCTEN <<10 , set, clr); +assign_clear_set_bit (tvt,CNTHCTL_EL1TVT , clr, set); +assign_clear_set_bit (tvc,CNTHCTL_EL1TVCT , clr, set); +assign_clear_set_bit (tvt02,CNTHCTL_EL1NVVCT , clr, set); +assign_clear_set_bit (tpt02,CNTHCTL_EL1NVPCT , clr, set);/* This only happens on VHE, so use the CNTHCTL_EL2 accessor. */ sysreg_clear_set (cnthctl_el2, clr, set);
@@ -931,8 +957,12 @@ void kvm_timer_sync_nested(struct kvm_vcpu *vcpu)* accesses redirected to the VNCR page. Any guest action taken on * the timer is postponed until the next exit, leading to a very * poor quality of emulation. +* +* This is an unmitigated disaster, only papered over by FEAT_ECV, +* which allows trapping of the timer registers even with NV2. +* Still, this is still worse than FEAT_NV on its own. Meh. */ -if (!is_hyp_ctxt (vcpu)) +if (cpus_have_final_cap (ARM64_HAS_ECV ) || !is_hyp_ctxt (vcpu))return ;if (!vcpu_el2_e2h_is_set (vcpu)) {
include/clocksource/arm_arch_timer.h +2 -0
@@ -24,6 +24,8 @@#define CNTHCTL_ECV (1 << 12)#define CNTHCTL_EL1TVT (1 << 13)#define CNTHCTL_EL1TVCT (1 << 14) +#define CNTHCTL_EL1NVPCT (1 << 15) +#define CNTHCTL_EL1NVVCT (1 << 16)enum arch_timer_reg {ARCH_TIMER_REG_CTRL ,
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 5/11 KVM: arm64: nv: Accelerate EL0 timer read accesses when FEAT_ECV in use
In reply to Marc ZyngierAlthough FEAT_ECV allows us to correctly emulate the timers, it also reduces performances pretty badly.
Mitigate this by emulating the CTL/CVAL register reads in the inner run loop, without returning to the general kernel.
--- arch/arm64/kvm/arch_timer.c | 15 ------- arch/arm64/kvm/hyp/vhe/switch.c | 71 +++++++++++++++++++++++++++++++++ include/kvm/arm_arch_timer.h | 15 +++++++ 3 files changed, 86 insertions(+), 15 deletions(-)
arch/arm64/kvm/arch_timer.c +0 -15
@@ -101,21 +101,6 @@ u64 timer_get_cval(struct arch_timer_context *ctxt) } } -static u64 timer_get_offset (struct arch_timer_context *ctxt) -{ -u64 offset =0 ; - -if (!ctxt) -return 0 ; - -if (ctxt->offset.vm_offset) - offset += *ctxt->offset.vm_offset; -if (ctxt->offset.vcpu_offset) - offset += *ctxt->offset.vcpu_offset; - -return offset; -} -static void timer_set_ctl (struct arch_timer_context *ctxt,u32 ctl) {struct kvm_vcpu *vcpu = ctxt->vcpu;
arch/arm64/kvm/hyp/vhe/switch.c +71 -0
83 lines not on this page · raw message
include/kvm/arm_arch_timer.h +15 -0
@@ -156,4 +156,19 @@ static inline bool has_cntpoff(void) return (has_vhe() && cpus_have_final_cap(ARM64_HAS_ECV_CNTPOFF)); } +static inline u64 timer_get_offset (struct arch_timer_context *ctxt) +{ +u64 offset =0 ; + +if (!ctxt) +return 0 ; + +if (ctxt->offset.vm_offset) + offset += *ctxt->offset.vm_offset; +if (ctxt->offset.vcpu_offset) + offset += *ctxt->offset.vcpu_offset; + +return offset; +} +#endif
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 6/11 KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor context
In reply to Marc ZyngierSimilarly to handling the physical timer accesses early when FEAT_ECV causes a trap, we try to handle the physical counter without returning to the general sysreg handling.
More surprisingly, we introduce something similar for the virtual counter. Although this isn't necessary yet, it will prove useful on systems that have a broken CNTVOFF_EL2 implementation. Yes, they exist.
Special care is taken to offset reads of the counter with the host's CNTPOFF_EL2, as we perform this with TGE clear.
--- arch/arm64/kvm/hyp/include/hyp/switch.h | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 13 +++++++++++++ 2 files changed, 18 insertions(+)
arch/arm64/kvm/hyp/include/hyp/switch.h +5 -0
@@ -501,6 +501,11 @@ static inline bool handle_tx2_tvm(struct kvm_vcpu *vcpu)return true; } +static inline u64 compute_counter_value (struct arch_timer_context *ctxt) +{ +return arch_timer_read_cntpct_el0 () -timer_get_offset (ctxt); +} +static bool kvm_hyp_handle_cntpct (struct kvm_vcpu *vcpu) {struct arch_timer_context *ctxt;
arch/arm64/kvm/hyp/vhe/switch.c +13 -0
@@ -296,6 +296,13 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) val =__vcpu_sys_reg (vcpu,CNTP_CVAL_EL0 ); }break ; +case SYS_CNTPCT_EL0 : +case SYS_CNTPCTSS_EL0 : +/* If !ELIsInHost(EL0), the guest's CNTPOFF_EL2 applies */ + val =compute_counter_value (!(vcpu_el2_e2h_is_set (vcpu) && +vcpu_el2_tge_is_set (vcpu)) ? +vcpu_ptimer (vcpu) :vcpu_hptimer (vcpu)); +break ;case SYS_CNTV_CTL_EL02 : val =__vcpu_sys_reg (vcpu,CNTV_CTL_EL0 );break ;
@@ -314,6 +321,12 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code)else val =__vcpu_sys_reg (vcpu,CNTV_CVAL_EL0 );break ; +case SYS_CNTVCT_EL0 : +case SYS_CNTVCTSS_EL0 : +/* If !ELIsInHost(EL2), the guest's CNTVOFF_EL2 applies */ + val =compute_counter_value (!vcpu_el2_e2h_is_set (vcpu) ? +vcpu_vtimer (vcpu) :vcpu_hvtimer (vcpu)); +break ;default :return false; }
Signature · 1 line
2.39.2
Re: [PATCH 06/11] KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor context
typo: accelerate
Quoted 52 lines · Marc Zyngier
Similarly to handling the physical timer accesses early when
FEAT_ECVcauses a trap, we try to handle the physical counter without returning to the general sysreg handling.More surprisingly, we introduce something similar for the virtual counter. Although this isn't necessary yet, it will prove useful on systems that have a broken
CNTVOFF_EL2implementation. Yes, they exist.Special care is taken to offset reads of the counter with the host's
CNTPOFF_EL2, as we perform this with TGE clear.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/include/hyp/switch.h | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 13 +++++++++++++ 2 files changed, 18 insertions(+)arch/arm64/kvm/hyp/include/hyp/switch.h +5
-0@@ -501,6 +501,11 @@ static inline bool handle_tx2_tvm(struct kvm_vcpu *vcpu)return true; } +static inline u64 compute_counter_value (struct arch_timer_context *ctxt) +{ +return arch_timer_read_cntpct_el0 () -timer_get_offset (ctxt); +} +static bool kvm_hyp_handle_cntpct (struct kvm_vcpu *vcpu) {struct arch_timer_context *ctxt;arch/arm64/kvm/hyp/vhe/switch.c +7
-0@@ -296,6 +296,13 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) val =__vcpu_sys_reg (vcpu,CNTP_CVAL_EL0 ); }break ; +case SYS_CNTPCT_EL0 : +case SYS_CNTPCTSS_EL0 : +/* If !ELIsInHost(EL0), the guest's CNTPOFF_EL2 applies */ + val =compute_counter_value (!(vcpu_el2_e2h_is_set (vcpu) && +vcpu_el2_tge_is_set (vcpu)) ? +vcpu_ptimer (vcpu) :vcpu_hptimer (vcpu)); +break ;case SYS_CNTV_CTL_EL02 : val =__vcpu_sys_reg (vcpu,CNTV_CTL_EL0 );break ;
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) · line 326
Quoted 6 lines
arch/arm64/kvm/hyp/vhe/switch.c +3
-0@@ -314,6 +321,12 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code)else val =__vcpu_sys_reg (vcpu,CNTV_CVAL_EL0 );break ; +case SYS_CNTVCT_EL0 : +case SYS_CNTVCTSS_EL0 : +/* If !ELIsInHost(EL2), the guest's CNTVOFF_EL2 applies */
!ELIsInHost(EL0)
Quoted 9 lines
arch/arm64/kvm/hyp/vhe/switch.c +3
-0+ val =compute_counter_value (!vcpu_el2_e2h_is_set (vcpu) ? +vcpu_vtimer (vcpu) :vcpu_hvtimer (vcpu)); +break ;default :return false; }Signature · 1 line
2.39.2
Marc Zyngier
maintainerRe: [PATCH 06/11] KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor context
Quoted 2 lines · Oliver Upton
typo: accelerate
Huh, thanks!
Quoted 2 lines · Marc Zyngier
[...]
Quoted 5 lines
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) · line 326
Quoted 3 lines
arch/arm64/kvm/hyp/vhe/switch.c +3
-0+case SYS_CNTVCT_EL0 : +case SYS_CNTVCTSS_EL0 : +/* If !ELIsInHost(EL2), the guest's CNTVOFF_EL2 applies */ !ELIsInHost(EL0)
No, and that's the whole point. CNTVOFF_EL2 applies at all times when HCR_EL2==0 and that we're at EL2. From the pseudocode for CNTVCT_EL0:
<quote>
[...]
elsif PSTATE.EL == EL2 then
if !ELIsInHost(EL2) then X[t, 64] = PhysicalCountInt() - CNTVOFF_EL2; else X[t, 64] = PhysicalCountInt();
[...]
</quote>
Which is why we only check E2H, and not E2H+TGE.
It is CNTPOFF_EL2 that applies when !ELIsInHost(EL0), and this is why it cannot be reliably emulated as we don't (and cannot) track changes to HCR_EL2.TGE. Yes, this is nonsense.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [PATCH 06/11] KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor context
Quoted 12 lines · Marc Zyngier
Quoted 2 lines · Oliver Upton
typo: accelerate
Huh, thanks!
Quoted 2 lines · Marc Zyngier
[...]
Quoted 8 lines
Quoted 5 lines
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) · line 326
Quoted 3 lines
arch/arm64/kvm/hyp/vhe/switch.c +3
-0+case SYS_CNTVCT_EL0 : +case SYS_CNTVCTSS_EL0 : +/* If !ELIsInHost(EL2), the guest's CNTVOFF_EL2 applies */ !ELIsInHost(EL0)
No, and that's the whole point.
CNTVOFF_EL2applies at all times when HCR_EL2==0 and that we're at EL2. From the pseudocode forCNTVCT_EL0:
Dammit, I don't know how many times I misread "CNTPCT" here rather than CNTVCT. Sorry for the noise.
Signature · 2 lines
Thanks, Oliver
Joey Gouly
reviewerRe: [PATCH 06/11] KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor context
In reply to Marc ZyngierQuoted 11 lines · Marc Zyngier
Similarly to handling the physical timer accesses early when
FEAT_ECVcauses a trap, we try to handle the physical counter without returning to the general sysreg handling.More surprisingly, we introduce something similar for the virtual counter. Although this isn't necessary yet, it will prove useful on systems that have a broken
CNTVOFF_EL2implementation. Yes, they exist.Special care is taken to offset reads of the counter with the host's
CNTPOFF_EL2, as we perform this with TGE clear.
Can you explain this part a bit more? I'm assuming it's somehow related to the arch_timer_read_cntpct_el0() call.
However I think we're at EL2 inside kvm_hyp_handle_timer(), so reading CNTPCT_EL0 won't involve CNTPOFF_EL2.
What am I missing/misunderstanding?
Quoted 44 lines
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/include/hyp/switch.h | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 13 +++++++++++++ 2 files changed, 18 insertions(+)arch/arm64/kvm/hyp/include/hyp/switch.h +5
-0@@ -501,6 +501,11 @@ static inline bool handle_tx2_tvm(struct kvm_vcpu *vcpu)return true; } +static inline u64 compute_counter_value (struct arch_timer_context *ctxt) +{ +return arch_timer_read_cntpct_el0 () -timer_get_offset (ctxt); +} +static bool kvm_hyp_handle_cntpct (struct kvm_vcpu *vcpu) {struct arch_timer_context *ctxt;arch/arm64/kvm/hyp/vhe/switch.c +7
-0@@ -296,6 +296,13 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) val =__vcpu_sys_reg (vcpu,CNTP_CVAL_EL0 ); }break ; +case SYS_CNTPCT_EL0 : +case SYS_CNTPCTSS_EL0 : +/* If !ELIsInHost(EL0), the guest's CNTPOFF_EL2 applies */ + val =compute_counter_value (!(vcpu_el2_e2h_is_set (vcpu) && +vcpu_el2_tge_is_set (vcpu)) ? +vcpu_ptimer (vcpu) :vcpu_hptimer (vcpu)); +break ;case SYS_CNTV_CTL_EL02 : val =__vcpu_sys_reg (vcpu,CNTV_CTL_EL0 );break ;@@ -314,6 +321,12 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code)else val =__vcpu_sys_reg (vcpu,CNTV_CVAL_EL0 );
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) · line 332
Quoted 10 lines
arch/arm64/kvm/hyp/vhe/switch.c +6
-0break ; +case SYS_CNTVCT_EL0 : +case SYS_CNTVCTSS_EL0 : +/* If !ELIsInHost(EL2), the guest's CNTVOFF_EL2 applies */ + val =compute_counter_value (!vcpu_el2_e2h_is_set (vcpu) ? +vcpu_vtimer (vcpu) :vcpu_hvtimer (vcpu)); +break ;default :return false; }
Thanks,
Joey
Marc Zyngier
maintainerRe: [PATCH 06/11] KVM: arm64: nv: Acceletate EL0 counter accesses from hypervisor context
Quoted 14 lines · Joey Gouly
Quoted 11 lines · Marc Zyngier
Similarly to handling the physical timer accesses early when
FEAT_ECVcauses a trap, we try to handle the physical counter without returning to the general sysreg handling.More surprisingly, we introduce something similar for the virtual counter. Although this isn't necessary yet, it will prove useful on systems that have a broken
CNTVOFF_EL2implementation. Yes, they exist.Special care is taken to offset reads of the counter with the host's
CNTPOFF_EL2, as we perform this with TGE clear.
Quoted 7 lines
Can you explain this part a bit more? I'm assuming it's somehow related to the
arch_timer_read_cntpct_el0()call.However I think we're at EL2 inside
kvm_hyp_handle_timer(), so readingCNTPCT_EL0won't involveCNTPOFF_EL2.What am I missing/misunderstanding?
I think the wording above is particularly misleading, and leads to some bad confusion.
Let's go back to basics: we're at host EL2, and handling a trap from guest EL2. Although you are right that CNTPOFF_EL2 doesn't affect a direct read of CNTPCT_EL0 from EL2, we are emulating it as if it was executed from EL1 (guest EL2 being EL1).
So any offsetting that would be applied if we were not trapping CNTPCT_EL0 must still be applied. In the case of a read from hypervisor context, this is the VM-wide offset.
But this makes me realise that...
Quoted 30 lines
Quoted 30 lines
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/hyp/include/hyp/switch.h | 5 +++++ arch/arm64/kvm/hyp/vhe/switch.c | 13 +++++++++++++ 2 files changed, 18 insertions(+)arch/arm64/kvm/hyp/include/hyp/switch.h +5
-0@@ -501,6 +501,11 @@ static inline bool handle_tx2_tvm(struct kvm_vcpu *vcpu)return true; } +static inline u64 compute_counter_value (struct arch_timer_context *ctxt) +{ +return arch_timer_read_cntpct_el0 () -timer_get_offset (ctxt); +} +static bool kvm_hyp_handle_cntpct (struct kvm_vcpu *vcpu) {struct arch_timer_context *ctxt;arch/arm64/kvm/hyp/vhe/switch.c +0
-0@@ -296,6 +296,13 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) val =__vcpu_sys_reg (vcpu,CNTP_CVAL_EL0 );
Quoted 8 lines
arch/arm64/kvm/hyp/vhe/switch.c static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code) · line 304
Quoted 8 lines
arch/arm64/kvm/hyp/vhe/switch.c +6
-0}break ; +case SYS_CNTPCT_EL0 : +case SYS_CNTPCTSS_EL0 : +/* If !ELIsInHost(EL0), the guest's CNTPOFF_EL2 applies */ + val =compute_counter_value (!(vcpu_el2_e2h_is_set (vcpu) && +vcpu_el2_tge_is_set (vcpu)) ? +vcpu_ptimer (vcpu) :vcpu_hptimer (vcpu));
... this should be simplified, because there is no case where we can be in HYP context *and* not be using the hptimer() context (everything else would be handled by kvm_handle_cntxct()).
I'm minded to change this to:
arch/arm64/kvm/hyp/vhe/switch.c +1 -4
@@ -305,10 +305,7 @@ static bool kvm_hyp_handle_timer(struct kvm_vcpu *vcpu, u64 *exit_code)break ;case SYS_CNTPCT_EL0 :case SYS_CNTPCTSS_EL0 : -/* If !ELIsInHost(EL0), the guest's CNTPOFF_EL2 applies */ - val =compute_counter_value (!(vcpu_el2_e2h_is_set (vcpu) && -vcpu_el2_tge_is_set (vcpu)) ? -vcpu_ptimer (vcpu) :vcpu_hptimer (vcpu)); + val =compute_counter_value (vcpu_hptimer (vcpu));break ;case SYS_CNTV_CTL_EL02 : val =__vcpu_sys_reg (vcpu,CNTV_CTL_EL0 );
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH 7/11 KVM: arm64: Handle counter access early in non-HYP context
In reply to Marc ZyngierWe already deal with CNTPCT_EL0 accesses in non-HYP context. Let's add CNTVCT_EL0 as a good measure.
This is also an opportunity to simplify things and make it plain that this code is only for non-HYP context handling.
--- arch/arm64/kvm/hyp/include/hyp/switch.h | 34 +++++++++++++++---------- 1 file changed, 21 insertions(+), 13 deletions(-)
arch/arm64/kvm/hyp/include/hyp/switch.h +21 -13
@@ -506,7 +506,7 @@ static inline u64 compute_counter_value(struct arch_timer_context *ctxt)return arch_timer_read_cntpct_el0 () -timer_get_offset (ctxt); } -static bool kvm_hyp_handle_cntpct (struct kvm_vcpu *vcpu) +static bool kvm_handle_cntxct (struct kvm_vcpu *vcpu) {struct arch_timer_context *ctxt;u32 sysreg;
@@ -516,18 +516,19 @@ static bool kvm_hyp_handle_cntpct(struct kvm_vcpu *vcpu)* We only get here for 64bit guests, 32bit guests will hit * the long and winding road all the way to the standard * handling. Yes, it sucks to be irrelevant. +* +* Also, we only deal with non-hypervisor context here (either +* an EL1 guest, or a non-HYP context of an EL2 guest). */ +if (is_hyp_ctxt (vcpu)) +return false; + sysreg =esr_sys64_to_sysreg (kvm_vcpu_get_esr (vcpu));switch (sysreg) {case SYS_CNTPCT_EL0 :case SYS_CNTPCTSS_EL0 :if (vcpu_has_nv (vcpu)) { -if (is_hyp_ctxt (vcpu)) { - ctxt =vcpu_hptimer (vcpu); -break ; - } -/* Check for guest hypervisor trapping */ val =__vcpu_sys_reg (vcpu,CNTHCTL_EL2 );if (!vcpu_el2_e2h_is_set (vcpu))
@@ -539,16 +540,23 @@ static bool kvm_hyp_handle_cntpct(struct kvm_vcpu *vcpu) ctxt =vcpu_ptimer (vcpu);break ; +case SYS_CNTVCT_EL0 : +case SYS_CNTVCTSS_EL0 : +if (vcpu_has_nv (vcpu)) { +/* Check for guest hypervisor trapping */ + val =__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ); + +if (val &CNTHCTL_EL1TVCT ) +return false; + } + + ctxt =vcpu_vtimer (vcpu); +break ;default :return false; } - val =arch_timer_read_cntpct_el0 (); - -if (ctxt->offset.vm_offset) - val -= *kern_hyp_va (ctxt->offset.vm_offset); -if (ctxt->offset.vcpu_offset) - val -= *kern_hyp_va (ctxt->offset.vcpu_offset); + val =compute_counter_value (ctxt);vcpu_set_reg (vcpu,kvm_vcpu_sys_get_rt (vcpu), val);__kvm_skip_instr (vcpu);
@@ -593,7 +601,7 @@ static bool kvm_hyp_handle_sysreg(struct kvm_vcpu *vcpu, u64 *exit_code) __vgic_v3_perform_cpuif_access(vcpu) == 1)return true; -if (kvm_hyp_handle_cntpct (vcpu)) +if (kvm_handle_cntxct (vcpu))return true;return false;
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 8/11 KVM: arm64: nv: Add trap routing for CNTHCTL_EL2.EL1{NVPCT,NVVCT,TVT,TVCT}
In reply to Marc ZyngierFor completeness, fun, and cerebral meltdown, add the virtualisation related traps to the counter and timers.
--- arch/arm64/kvm/emulate-nested.c | 58 +++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-)
arch/arm64/kvm/emulate-nested.c +56 -2
@@ -89,6 +89,9 @@ enum cgt_group_id { CGT_HCRX_EnFPM, CGT_HCRX_TCR2En, +CGT_CNTHCTL_EL1TVT , +CGT_CNTHCTL_EL1TVCT , +CGT_ICH_HCR_TC ,CGT_ICH_HCR_TALL0 , CGT_ICH_HCR_TALL1,
@@ -124,6 +127,8 @@ enum cgt_group_id { __COMPLEX_CONDITIONS__,CGT_CNTHCTL_EL1PCTEN = __COMPLEX_CONDITIONS__,CGT_CNTHCTL_EL1PTEN , +CGT_CNTHCTL_EL1NVPCT , +CGT_CNTHCTL_EL1NVVCT ,CGT_CPTR_TTA , CGT_MDCR_HPMN,
@@ -393,6 +398,18 @@ static const struct trap_bits coarse_trap_bits[] = { .mask = HCRX_EL2_TCR2En, .behaviour = BEHAVE_FORWARD_RW, }, + [CGT_CNTHCTL_EL1TVT] = { + .index = CNTHCTL_EL2, + .value = CNTHCTL_EL1TVT, + .mask = CNTHCTL_EL1TVT, + .behaviour = BEHAVE_FORWARD_RW, + }, + [CGT_CNTHCTL_EL1TVCT] = { + .index = CNTHCTL_EL2, + .value = CNTHCTL_EL1TVCT, + .mask = CNTHCTL_EL1TVCT, + .behaviour = BEHAVE_FORWARD_READ, + }, [CGT_ICH_HCR_TC] = { .index = ICH_HCR_EL2, .value = ICH_HCR_TC,
@@ -487,6 +504,32 @@ static enum trap_behaviour check_cnthctl_el1pten(struct kvm_vcpu *vcpu)return BEHAVE_FORWARD_RW ; } +static bool is_nested_nv2_guest (struct kvm_vcpu *vcpu) +{ +u64 val; + + val =__vcpu_sys_reg (vcpu,HCR_EL2 ); +return ((val & (HCR_E2H |HCR_TGE |HCR_NV2 |HCR_NV1 |HCR_NV )) == (HCR_E2H |HCR_NV2 |HCR_NV )); +} + +static enum trap_behaviour check_cnthctl_el1nvpct (struct kvm_vcpu *vcpu) +{ +if (!is_nested_nv2_guest (vcpu) || + !(__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ) &CNTHCTL_EL1NVPCT )) +return BEHAVE_HANDLE_LOCALLY ; + +return BEHAVE_FORWARD_RW ; +} + +static enum trap_behaviour check_cnthctl_el1nvvct (struct kvm_vcpu *vcpu) +{ +if (!is_nested_nv2_guest (vcpu) || + !(__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ) &CNTHCTL_EL1NVVCT )) +return BEHAVE_HANDLE_LOCALLY ; + +return BEHAVE_FORWARD_RW ; +} +static enum trap_behaviour check_cptr_tta (struct kvm_vcpu *vcpu) {u64 val =__vcpu_sys_reg (vcpu,CPTR_EL2 );
@@ -534,6 +577,8 @@ static enum trap_behaviour check_mdcr_hpmn(struct kvm_vcpu *vcpu)static const complex_condition_check ccc[] = {CCC (CGT_CNTHCTL_EL1PCTEN , check_cnthctl_el1pcten),CCC (CGT_CNTHCTL_EL1PTEN , check_cnthctl_el1pten), +CCC (CGT_CNTHCTL_EL1NVPCT , check_cnthctl_el1nvpct), +CCC (CGT_CNTHCTL_EL1NVVCT , check_cnthctl_el1nvvct),CCC (CGT_CPTR_TTA , check_cptr_tta),CCC (CGT_MDCR_HPMN , check_mdcr_hpmn), };
@@ -850,11 +895,15 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = { SYS_CNTHP_CVAL_EL2, CGT_HCR_NV),SR_RANGE_TRAP (SYS_CNTHV_TVAL_EL2 ,SYS_CNTHV_CVAL_EL2 ,CGT_HCR_NV ), -/* All _EL02, _EL12 registers */ +/* All _EL02, _EL12 registers up to CNTKCTL_EL12*/ SR_RANGE_TRAP (sys_reg (3 ,5 ,0 ,0 ,0 ),sys_reg (3 ,5 ,10 ,15 ,7 ),CGT_HCR_NV ),SR_RANGE_TRAP (sys_reg (3 ,5 ,12 ,0 ,0 ), -sys_reg (3 ,5 ,14 ,15 ,7 ),CGT_HCR_NV ), +sys_reg (3 ,5 ,14 ,1 ,0 ),CGT_HCR_NV ), +SR_TRAP (SYS_CNTP_CTL_EL02 ,CGT_CNTHCTL_EL1NVPCT ), +SR_TRAP (SYS_CNTP_CVAL_EL02 ,CGT_CNTHCTL_EL1NVPCT ), +SR_TRAP (SYS_CNTV_CTL_EL02 ,CGT_CNTHCTL_EL1NVVCT ), +SR_TRAP (SYS_CNTV_CVAL_EL02 ,CGT_CNTHCTL_EL1NVVCT ),SR_TRAP (OP_AT_S1E2R ,CGT_HCR_NV ),SR_TRAP (OP_AT_S1E2W ,CGT_HCR_NV ), SR_TRAP(OP_AT_S12E1R, CGT_HCR_NV),
@@ -1184,6 +1233,11 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {SR_TRAP (SYS_CNTP_CTL_EL0 ,CGT_CNTHCTL_EL1PTEN ),SR_TRAP (SYS_CNTPCT_EL0 ,CGT_CNTHCTL_EL1PCTEN ),SR_TRAP (SYS_CNTPCTSS_EL0 ,CGT_CNTHCTL_EL1PCTEN ), +SR_TRAP (SYS_CNTV_TVAL_EL0 ,CGT_CNTHCTL_EL1TVT ), +SR_TRAP (SYS_CNTV_CVAL_EL0 ,CGT_CNTHCTL_EL1TVT ), +SR_TRAP (SYS_CNTV_CTL_EL0 ,CGT_CNTHCTL_EL1TVT ), +SR_TRAP (SYS_CNTVCT_EL0 ,CGT_CNTHCTL_EL1TVCT ), +SR_TRAP (SYS_CNTVCTSS_EL0 ,CGT_CNTHCTL_EL1TVCT ),SR_TRAP (SYS_FPMR , CGT_HCRX_EnFPM),/* * IMPDEF choice:
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 9/11 KVM: arm64: nv: Propagate CNTHCTL_EL2.EL1NV{P,V}CT bits
In reply to Marc ZyngierAllow a guest hypervisor to trap accesses to CNT{P,V}CT_EL02 by propagating these trap bits to the host trap configuration.
--- arch/arm64/kvm/arch_timer.c | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm64/kvm/arch_timer.c +7 -0
@@ -822,6 +822,10 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map)* Apply the enable bits that the guest hypervisor has requested for * its own guest. We can only add traps that wouldn't have been set * above. +* Implementation choices: we do not support NV when E2H=0 in the +* guest, and we don't support configuration where E2H is writable +* by the guest (either FEAT_VHE or FEAT_E2H0 is implemented, but +* not both). This simplifies the handling of the EL1NV* bits. */ if (vcpu_has_nv (vcpu) && !is_hyp_ctxt (vcpu)) {u64 val =__vcpu_sys_reg (vcpu,CNTHCTL_EL2 );
@@ -832,6 +836,9 @@ static void timer_set_traps(struct kvm_vcpu *vcpu, struct timer_map *map) tpt |= !(val & (CNTHCTL_EL1PCEN <<10 )); tpc |= !(val & (CNTHCTL_EL1PCTEN <<10 )); + + tpt02 |= (val &CNTHCTL_EL1NVPCT ); + tvt02 |= (val &CNTHCTL_EL1NVVCT ); }/*
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 10/11 KVM: arm64: nv: Sanitise CNTHCTL_EL2
In reply to Marc ZyngierInject some sanity in CNTHCTL_EL2, ensuring that we don't handle more than we advertise to the guest.
--- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/nested.c | 15 +++++++++++++++ include/clocksource/arm_arch_timer.h | 2 ++ 3 files changed, 18 insertions(+), 1 deletion(-)
arch/arm64/include/asm/kvm_host.h +1 -1
@@ -490,7 +490,6 @@ enum vcpu_sysreg {VBAR_EL2 ,/* Vector Base Address Register (EL2) */ RVBAR_EL2 ,/* Reset Vector Base Address Register */ CONTEXTIDR_EL2 ,/* Context ID Register (EL2) */ -CNTHCTL_EL2 ,/* Counter-timer Hypervisor Control register */ SP_EL2 ,/* EL2 Stack Pointer */ CNTHP_CTL_EL2 , CNTHP_CVAL_EL2,
@@ -501,6 +500,7 @@ enum vcpu_sysreg {MARKER (__SANITISED_REG_START__),TCR2_EL2 ,/* Extended Translation Control Register (EL2) */ MDCR_EL2 ,/* Monitor Debug Configuration Register (EL2) */ +CNTHCTL_EL2 ,/* Counter-timer Hypervisor Control register */ /* Any VNCR-capable reg goes after this point */ MARKER(__VNCR_START__),
arch/arm64/kvm/nested.c +15 -0
@@ -1271,6 +1271,21 @@ int kvm_init_nv_sysregs(struct kvm *kvm) res0 |= MDCR_EL2_EnSTEPOP;set_sysreg_masks (kvm,MDCR_EL2 , res0, res1); +/* CNTHCTL_EL2 */ + res0 =GENMASK (63 ,20 ); + res1 =0 ; +if (!kvm_has_feat (kvm,ID_AA64PFR0_EL1 ,RME ,IMP )) + res0 |=CNTHCTL_CNTPMASK |CNTHCTL_CNTVMASK ; +if (!kvm_has_feat (kvm,ID_AA64MMFR0_EL1 ,ECV ,CNTPOFF )) { + res0 |=CNTHCTL_ECV ; +if (!kvm_has_feat (kvm,ID_AA64MMFR0_EL1 ,ECV ,IMP )) + res0 |= (CNTHCTL_EL1TVT |CNTHCTL_EL1TVCT | +CNTHCTL_EL1NVPCT |CNTHCTL_EL1NVVCT ); + } +if (!kvm_has_feat (kvm,ID_AA64MMFR1_EL1 ,VH ,IMP )) + res0 |=GENMASK (11 ,8 ); +set_sysreg_masks (kvm,CNTHCTL_EL2 , res0, res1); +return 0 ; }
include/clocksource/arm_arch_timer.h +2 -0
@@ -26,6 +26,8 @@#define CNTHCTL_EL1TVCT (1 << 14)#define CNTHCTL_EL1NVPCT (1 << 15)#define CNTHCTL_EL1NVVCT (1 << 16) +#define CNTHCTL_CNTVMASK (1 << 18) +#define CNTHCTL_CNTPMASK (1 << 19)enum arch_timer_reg {ARCH_TIMER_REG_CTRL ,
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH 11/11 KVM: arm64: Work around x1e's CNTVOFF_EL2 bogosity
In reply to Marc ZyngierIt appears that on Qualcomm's x1e CPU, CNTVOFF_EL2 doesn't really work, specially with HCR_EL2.E2H=1.
A non-zero offset results in a screaming virtual timer interrupt, to the tune of a few 100k interrupts per second on a 4 vcpu VM. This is also evidenced by this CPU's inability to correctly run any of the timer selftests.
The only case this doesn't break is when this register is set to 0, which breaks VM migration.
When HCR_EL2.E2H=0, the timer seems to behave normally, and does not result in an interrupt storm.
As a workaround, use the fact that this CPU implements FEAT_ECV, and trap all accesses to the virtual timer and counter, keeping CNTVOFF_EL2 set to zero, and emulate accesses to CVAL/TVAL/CTL and the counter itself, fixing up the timer to account for the missing offset.
And if you think this is disgusting, you'd probably be right.
--- arch/arm64/include/asm/cputype.h | 2 ++ arch/arm64/kernel/cpu_errata.c | 8 +++++ arch/arm64/kernel/image-vars.h | 3 ++ arch/arm64/kvm/arch_timer.c | 58 ++++++++++++++++++++++++++++-- arch/arm64/kvm/hyp/nvhe/timer-sr.c | 16 ++++++--- arch/arm64/kvm/sys_regs.c | 3 +- arch/arm64/tools/cpucaps | 1 + include/kvm/arm_arch_timer.h | 7 ++++ 8 files changed, 90 insertions(+), 8 deletions(-)
arch/arm64/include/asm/cputype.h +2 -0
@@ -122,6 +122,7 @@#define QCOM_CPU_PART_KRYO_3XX_SILVER 0x803#define QCOM_CPU_PART_KRYO_4XX_GOLD 0x804#define QCOM_CPU_PART_KRYO_4XX_SILVER 0x805 +#define QCOM_CPU_PART_ORYON_X1 0x001#define NVIDIA_CPU_PART_DENVER 0x003#define NVIDIA_CPU_PART_CARMEL 0x004
@@ -198,6 +199,7 @@#define MIDR_QCOM_KRYO_3XX_SILVER MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_3XX_SILVER)#define MIDR_QCOM_KRYO_4XX_GOLD MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_4XX_GOLD)#define MIDR_QCOM_KRYO_4XX_SILVER MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_KRYO_4XX_SILVER) +#define MIDR_QCOM_ORYON_X1 MIDR_CPU_MODEL(ARM_CPU_IMP_QCOM, QCOM_CPU_PART_ORYON_X1)#define MIDR_NVIDIA_DENVER MIDR_CPU_MODEL(ARM_CPU_IMP_NVIDIA, NVIDIA_CPU_PART_DENVER)#define MIDR_NVIDIA_CARMEL MIDR_CPU_MODEL(ARM_CPU_IMP_NVIDIA, NVIDIA_CPU_PART_CARMEL)#define MIDR_FUJITSU_A64FX MIDR_CPU_MODEL(ARM_CPU_IMP_FUJITSU, FUJITSU_CPU_PART_A64FX)
arch/arm64/kernel/cpu_errata.c +8 -0
@@ -786,6 +786,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = { ERRATA_MIDR_RANGE_LIST(erratum_ac03_cpu_38_list), }, #endif + { + .desc = "Broken CNTVOFF_EL2", + .capability =ARM64_WORKAROUND_QCOM_ORYON_CNTVOFF , +ERRATA_MIDR_RANGE_LIST (((const struct midr_range []) { +MIDR_ALL_VERSIONS (MIDR_QCOM_ORYON_X1 ), + {} + })), + }, { } };
arch/arm64/kernel/image-vars.h +3 -0
@@ -105,6 +105,9 @@ KVM_NVHE_ALIAS(__hyp_stub_vectors);KVM_NVHE_ALIAS (vgic_v2_cpuif_trap);KVM_NVHE_ALIAS (vgic_v3_cpuif_trap); +/* Static key which is set if CNTVOFF_EL2 is unusable */ +KVM_NVHE_ALIAS (broken_cntvoff_key); +/* EL2 exception handling */ KVM_NVHE_ALIAS (__start___kvm_ex_table);KVM_NVHE_ALIAS (__stop___kvm_ex_table);
arch/arm64/kvm/arch_timer.c +55 -3
94 lines not on this page · raw message
arch/arm64/kvm/hyp/nvhe/timer-sr.c +12 -4
@@ -22,15 +22,16 @@ void __kvm_timer_set_cntvoff(u64 cntvoff)*/ void __timer_disable_traps (struct kvm_vcpu *vcpu) { -u64 val, shift =0 ; +u64 set, clr, shift =0 ;if (has_hvhe ()) shift =10 ;/* Allow physical timer/counter access for the host */ - val =read_sysreg (cnthctl_el2); - val |= (CNTHCTL_EL1PCTEN |CNTHCTL_EL1PCEN ) << shift; -write_sysreg (val, cnthctl_el2); + set = (CNTHCTL_EL1PCTEN |CNTHCTL_EL1PCEN ) << shift; + clr =CNTHCTL_EL1TVT |CNTHCTL_EL1TVCT ; + +sysreg_clear_set (cnthctl_el2, clr, set); }/*
@@ -58,5 +59,12 @@ void __timer_enable_traps(struct kvm_vcpu *vcpu) set <<=10 ; } +/* +* Trap the virtual counter/timer if we have a broken cntvoff +* implementation. +*/ +if (has_broken_cntvoff ()) + set |=CNTHCTL_EL1TVT |CNTHCTL_EL1TVCT ; +sysreg_clear_set (cnthctl_el2, clr, set); }
arch/arm64/kvm/sys_regs.c +2 -1
@@ -1721,7 +1721,8 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,if (!vcpu_has_ptrauth (vcpu)) val &= ~(ARM64_FEATURE_MASK (ID_AA64ISAR2_EL1_APA3 ) |ARM64_FEATURE_MASK (ID_AA64ISAR2_EL1_GPA3 )); -if (!cpus_have_final_cap (ARM64_HAS_WFXT )) +if (!cpus_have_final_cap (ARM64_HAS_WFXT ) || +has_broken_cntvoff ()) val &= ~ARM64_FEATURE_MASK (ID_AA64ISAR2_EL1_WFxT);break ;case SYS_ID_AA64MMFR2_EL1 :
arch/arm64/tools/cpucaps +1 -0
@@ -105,6 +105,7 @@ WORKAROUND_CLEAN_CACHE WORKAROUND_DEVICE_LOAD_ACQUIRE WORKAROUND_NVIDIA_CARMEL_CNP WORKAROUND_QCOM_FALKOR_E1003 +WORKAROUND_QCOM_ORYON_CNTVOFF WORKAROUND_REPEAT_TLBI WORKAROUND_SPECULATIVE_AT WORKAROUND_SPECULATIVE_SSBS
include/kvm/arm_arch_timer.h +7 -0
@@ -151,6 +151,13 @@ void kvm_timer_cpu_down(void);/* CNTKCTL_EL1 valid bits as of DDI0487J.a */ #define CNTKCTL_VALID_BITS (BIT(17) | GENMASK_ULL(9, 0)) +DECLARE_STATIC_KEY_FALSE (broken_cntvoff_key); + +static inline bool has_broken_cntvoff (void ) +{ +return static_branch_unlikely (&broken_cntvoff_key); +} +static inline bool has_cntpoff (void ) {return (has_vhe () &&cpus_have_final_cap (ARM64_HAS_ECV_CNTPOFF ));
Signature · 1 line
2.39.2
Quoted 23 lines · Marc Zyngier
Here's another batch of NV-related patches, this time bringing in most of the timer support for EL2 as well as nested guests.
The code is pretty convoluted for a bunch of reasons:
-
FEAT_NV2breaks the timer semantics by redirecting HW controls to memory, meaning that a guest could setup a timer and never see it firing until the next exit- We go try hard to reflect the timer state in memory, but that's not great.
- With
FEAT_ECV, we can finally correctly emulate the virtual timer, but this emulation is pretty costly- As a way to make things suck less, we handle timer reads as early as possible, and only defer writes to the normal trap handling
- Finally, some implementations are badly broken, and require some hand-holding, irrespective of NV support. So we try and reuse the NV infrastructure to make them usable. This could be further optimised, but I'm running out of patience for this sort of HW.
Quoted 7 lines
What this is not implementing is support for
CNTPOFF_EL2. It appears that the architecture doesn't let you correctly emulate it, so I guess this will be trap/emulate for the foreseeable future.This series is on top of v6.13-rc1, and has been tested on my usual M2 setup, but also on a Snapdragon X1 Elite devkit. I would like to thank Qualcomm for the free hardware with no strings (nor support) attached!
This series is looking pretty good to me, but I think it'd be good to remap "EL0 timer" -> "EL1 timer" throughout this series to match the architectural term.
Signature · 2 lines
Thanks, Oliver
Hi Marc,
Quoted 27 lines · Marc Zyngier
Here's another batch of NV-related patches, this time bringing in most of the timer support for EL2 as well as nested guests.
The code is pretty convoluted for a bunch of reasons:
-
FEAT_NV2breaks the timer semantics by redirecting HW controls to memory, meaning that a guest could setup a timer and never see it firing until the next exit- We go try hard to reflect the timer state in memory, but that's not great.
- With
FEAT_ECV, we can finally correctly emulate the virtual timer, but this emulation is pretty costly- As a way to make things suck less, we handle timer reads as early as possible, and only defer writes to the normal trap handling
- Finally, some implementations are badly broken, and require some hand-holding, irrespective of NV support. So we try and reuse the NV infrastructure to make them usable. This could be further optimised, but I'm running out of patience for this sort of HW.
What this is not implementing is support for
CNTPOFF_EL2. It appears that the architecture doesn't let you correctly emulate it, so I guess this will be trap/emulate for the foreseeable future.
Quoted 8 lines
This series is on top of v6.13-rc1, and has been tested on my usual M2 setup, but also on a Snapdragon X1 Elite devkit. I would like to thank Qualcomm for the free hardware with no strings (nor support) attached!
If you are feeling brave, you can run the whole thing from [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-next
I was feeling brave, and I think I see an issue in the cpufeature change
in the kvm-arm64/nv-e2h-select branch that's a part of
kvm-arm64/nv-next. In d75a4820a897 ("arm64: cpufeature: Handle NV_frac as a synonym of NV2"), I don't see NV_frac being added to the FTR bits. I believe that means it
will get sanitized out and consequently not seen by the NV feature
detection code. Does that commit also need:
arch/arm64/kernel/cpufeature.c +1 -0
@@ -480,6 +480,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr3[] = {static const struct arm64_ftr_bits ftr_id_aa64mmfr4[] = {S_ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR4_EL1_E2H0_SHIFT ,4 ,0 ), +S_ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE , ID_AA64MMFR4_EL1_NV_frac_SHIFT,4 ,0 ),ARM64_FTR_END , };
Thanks,
Chase
Marc Zyngier
maintainerHi Chase,
Quoted 23 lines · Chase Conklin
Hi Marc,
Quoted 4 lines · Marc Zyngier
If you are feeling brave, you can run the whole thing from [1].
[1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/nv-next
I was feeling brave, and I think I see an issue in the cpufeature change
in the kvm-arm64/nv-e2h-select branch that's a part of
kvm-arm64/nv-next. Ind75a4820a897("arm64: cpufeature: HandleNV_fracas a synonym of NV2"), I don't seeNV_fracbeing added to the FTR bits. I believe that means it
will get sanitized out and consequently not seen by the NV feature
detection code. Does that commit also need:arch/arm64/kernel/cpufeature.c +0
-0
arch/arm64/kernel/cpufeature.c static const struct arm64_ftr_bits ftr_id_aa64mmfr3[] = { · line 485
Quoted 6 lines
arch/arm64/kernel/cpufeature.c +1
-0@@ -480,6 +480,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr3[] = {static const struct arm64_ftr_bits ftr_id_aa64mmfr4[] = {S_ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE ,ID_AA64MMFR4_EL1_E2H0_SHIFT ,4 ,0 ), +S_ARM64_FTR_BITS (FTR_HIDDEN ,FTR_STRICT ,FTR_LOWER_SAFE , ID_AA64MMFR4_EL1_NV_frac_SHIFT,4 ,0 ),ARM64_FTR_END , };
Ah, I always get tripped by that one. You are absolutely correct, this is needed. I'll fold that in.
Thanks again,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.