KVM: arm64: Rework timer offsetting for fun and profit
Subsystems (5)
Marc Zyngier
maintainerv3 cover KVM: arm64: Rework timer offsetting for fun and profit
This series aims at satisfying multiple goals:
- allow a VMM to atomically restore a timer offset for a whole VM instead of updating the offset each time a vcpu get its counter written
- allow a VMM to save/restore the physical timer context, something that we cannot do at the moment due to the lack of offsetting
- provide a framework that is suitable for NV support, where we get both global and per timer, per vcpu offsetting, and manage interrupts in a less braindead way.
We fix a couple of issues along the way, both from a stylistic and correctness perspective. This results in a new per VM KVM API that allows a global offset to be set at any point in time, overriding both of the timer counter writebacks.
We also take this opportunity to rework the way IRQs are associated with timers, something that was always a bit dodgy. This relies on a new lock, which should disappear once Oliver's lock ordering series is merged (we can reuse the config_lock for this).
This has been tested with nVHE, VHE and NV. I do not have access to CNTPOFF-aware HW, but Colton managed to give it a go. Note that the NV patches in this series are here to give a perspective on how this gets used.
I've updated the arch_timer selftest to allow an offset to be provided from the command line, and fixed a couple of glaring issues along the way.
Note that this is at best 6.4 material. I have a branch stashed at [0] and based on 6.3-rc3, as well as a minimal example of the use of the API at [3] based on kvmtool.
Simon: I'd appreciate some feedback as whether this change fits your requirements, given that you brought this up the first place.
Thanks,
M.
* From v2 [2]:
- Fixed 32bit handling of the physical counter when the offset is non-zero
- Dropped unused -O option from the selftest
- Added lockdep_assert_held() to (un)lock_all_vcpus()
- Reordered the last two patches
- Added Colton's RBs, with thanks
- Dropped the initial patch which has been merged
- Rebased on 6.3-rc3
* From v1 [1]:
- Switched from a dual offset to a single one which gets applied to both virtual and physical counters. Which means that NV doesn't behave oddly anymore by ignoring the virtual offset.
- Some cosmetic repainting of the UAPI symbols
- Added patches to rework the IRQ mapping to timers
- Patch #1 on its way to Paolo
- Rebased on 6.3-rc1
[0] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/timer-vm-offsets
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/commit/?h=zero-offset&id=3b1253073ee57c0d92baf7b214362829b487b8d5
Marc Zyngier (18):
KVM: arm64: timers: Use a per-vcpu, per-timer accumulator for
fractional ns
arm64: Add CNTPOFF_EL2 register definition
arm64: Add HAS_ECV_CNTPOFF capability
KVM: arm64: timers: Use CNTPOFF_EL2 to offset the physical timer
KVM: arm64: timers: Allow physical offset without CNTPOFF_EL2
KVM: arm64: Expose {un,}lock_all_vcpus() to the rest of KVM
KVM: arm64: timers: Allow userspace to set the global counter offset
KVM: arm64: timers: Allow save/restoring of the physical timer
KVM: arm64: timers: Rationalise per-vcpu timer init
KVM: arm64: timers: Abstract per-timer IRQ access
KVM: arm64: timers: Move the timer IRQs into arch_timer_vm_data
KVM: arm64: Abstract the number of valid timers per vcpu
KVM: arm64: Document KVM_ARM_SET_CNT_OFFSETS and co
KVM: arm64: nv: timers: Add a per-timer, per-vcpu offset
KVM: arm64: nv: timers: Support hyp timer emulation
KVM: arm64: selftests: Add physical timer registers to the sysreg list
KVM: arm64: selftests: Deal with spurious timer interrupts
KVM: arm64: selftests: Augment existing timer test to handle variable
offset
Documentation/virt/kvm/api.rst | 38 ++ arch/arm64/include/asm/kvm_host.h | 13 + arch/arm64/include/asm/sysreg.h | 2 + arch/arm64/include/uapi/asm/kvm.h | 11 + arch/arm64/kernel/cpufeature.c | 11 + arch/arm64/kvm/arch_timer.c | 539 ++++++++++++++---- arch/arm64/kvm/arm.c | 53 ++ arch/arm64/kvm/guest.c | 29 +- arch/arm64/kvm/hyp/nvhe/timer-sr.c | 18 +- arch/arm64/kvm/hypercalls.c | 2 +- arch/arm64/kvm/sys_regs.c | 9 + arch/arm64/kvm/trace_arm.h | 6 +- arch/arm64/kvm/vgic/vgic-kvm-device.c | 38 -- arch/arm64/kvm/vgic/vgic.c | 15 + arch/arm64/kvm/vgic/vgic.h | 3 - arch/arm64/tools/cpucaps | 1 + arch/arm64/tools/sysreg | 4 + include/clocksource/arm_arch_timer.h | 1 + include/kvm/arm_arch_timer.h | 36 +- include/kvm/arm_vgic.h | 1 + include/uapi/linux/kvm.h | 3 + .../selftests/kvm/aarch64/arch_timer.c | 56 +- .../selftests/kvm/aarch64/get-reg-list.c | 5 +- 23 files changed, 690 insertions(+), 204 deletions(-)
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 1/18 KVM: arm64: timers: Use a per-vcpu, per-timer accumulator for fractional ns
Instead of accumulating the fractional ns value generated every time we compute a ns delta in a global variable, use a per-vcpu, per-timer variable. This keeps the fractional ns local to the timer instead of contributing to any odd, unrelated timer.
--- arch/arm64/kvm/arch_timer.c | 2 +- include/kvm/arm_arch_timer.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kvm/arch_timer.c +1 -1
@@ -212,7 +212,7 @@ static u64 kvm_counter_compute_delta(struct arch_timer_context *timer_ctx, ns =cyclecounter_cyc2ns (timecounter->cc, val - now, timecounter->mask, - &timecounter->frac); + &timer_ctx->ns_frac);return ns; }
include/kvm/arm_arch_timer.h +1 -0
@@ -44,6 +44,7 @@ struct arch_timer_context {/* Emulated Timer (may be unused) */ struct hrtimer hrtimer; +u64 ns_frac;/* Offset for this counter/timer */ struct arch_timer_offset offset;
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 2/18 arm64: Add CNTPOFF_EL2 register definition
In reply to Marc ZyngierAdd the definition for CNTPOFF_EL2 in the description file.
--- arch/arm64/tools/sysreg | 4 ++++ 1 file changed, 4 insertions(+)
arch/arm64/tools/sysreg +4 -0
@@ -1952,6 +1952,10 @@ Sysreg CONTEXTIDR_EL2 3 4 13 0 1 Fields CONTEXTIDR_ELx EndSysreg +Sysreg CNTPOFF_EL2 3 4 14 0 6 +Field 63:0 PhysicalOffset +EndSysreg + Sysreg CPACR_EL12 3 5 1 0 2 Fields CPACR_ELx EndSysreg
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 3/18 arm64: Add HAS_ECV_CNTPOFF capability
In reply to Marc ZyngierAdd the probing code for the FEAT_ECV variant that implements CNTPOFF_EL2. Why it is optional is a mystery, but let's try and detect it.
--- arch/arm64/kernel/cpufeature.c | 11 +++++++++++ arch/arm64/tools/cpucaps | 1 + 2 files changed, 12 insertions(+)
arch/arm64/kernel/cpufeature.c +11 -0
@@ -2223,6 +2223,17 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .sign = FTR_UNSIGNED, .min_field_value = 1, }, + { + .desc = "Enhanced Counter Virtualization (CNTPOFF)", + .capability = ARM64_HAS_ECV_CNTPOFF, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_cpuid_feature, + .sys_reg = SYS_ID_AA64MMFR0_EL1, + .field_pos = ID_AA64MMFR0_EL1_ECV_SHIFT, + .field_width = 4, + .sign = FTR_UNSIGNED, + .min_field_value = ID_AA64MMFR0_EL1_ECV_CNTPOFF, + }, #ifdef CONFIG_ARM64_PAN { .desc = "Privileged Access Never",
arch/arm64/tools/cpucaps +1 -0
@@ -23,6 +23,7 @@ HAS_DCPOP HAS_DIT HAS_E0PD HAS_ECV +HAS_ECV_CNTPOFF HAS_EPAN HAS_GENERIC_AUTH HAS_GENERIC_AUTH_ARCH_QARMA3
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 4/18 KVM: arm64: timers: Use CNTPOFF_EL2 to offset the physical timer
In reply to Marc ZyngierWith ECV and CNTPOFF_EL2, it is very easy to offer an offset for the physical timer. So let's do just that.
Nothing can set the offset yet, so this should have no effect whatsoever (famous last words...).
--- arch/arm64/kvm/arch_timer.c | 18 +++++++++++++++++- arch/arm64/kvm/hypercalls.c | 2 +- include/clocksource/arm_arch_timer.h | 1 + include/kvm/arm_arch_timer.h | 2 ++ 4 files changed, 21 insertions(+), 2 deletions(-)
arch/arm64/kvm/arch_timer.c +17 -1
@@ -52,6 +52,11 @@ static u64 kvm_arm_timer_read(struct kvm_vcpu *vcpu,struct arch_timer_context *timer, enum kvm_arch_timer_regs treg); +static bool has_cntpoff (void ) +{ +return (has_vhe () &&cpus_have_final_cap (ARM64_HAS_ECV_CNTPOFF )); +} +u32 timer_get_ctl (struct arch_timer_context *ctxt) {struct kvm_vcpu *vcpu = ctxt->vcpu;
@@ -84,7 +89,7 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)static u64 timer_get_offset (struct arch_timer_context *ctxt) { -if (ctxt->offset.vm_offset) +if (ctxt && ctxt->offset.vm_offset)return *ctxt->offset.vm_offset;return 0 ;
@@ -432,6 +437,12 @@ static void set_cntvoff(u64 cntvoff)kvm_call_hyp (__kvm_timer_set_cntvoff, cntvoff); } +static void set_cntpoff (u64 cntpoff) +{ +if (has_cntpoff ()) +write_sysreg_s (cntpoff,SYS_CNTPOFF_EL2 ); +} +static void timer_save_state (struct arch_timer_context *ctx) {struct arch_timer_cpu *timer =vcpu_timer (ctx->vcpu);
@@ -480,6 +491,7 @@ static void timer_save_state(struct arch_timer_context *ctx)write_sysreg_el0 (0 ,SYS_CNTP_CTL );isb (); +set_cntpoff (0 );break ;case NR_KVM_TIMERS :BUG ();
@@ -550,6 +562,7 @@ static void timer_restore_state(struct arch_timer_context *ctx)write_sysreg_el0 (timer_get_ctl (ctx),SYS_CNTV_CTL );break ;case TIMER_PTIMER : +set_cntpoff (timer_get_offset (ctx));write_sysreg_el0 (timer_get_cval (ctx),SYS_CNTP_CVAL );isb ();write_sysreg_el0 (timer_get_ctl (ctx),SYS_CNTP_CTL );
@@ -767,6 +780,7 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) vtimer->vcpu = vcpu; vtimer->offset.vm_offset = &vcpu->kvm->arch.timer_data.voffset; ptimer->vcpu = vcpu; + ptimer->offset.vm_offset = &vcpu->kvm->arch.timer_data.poffset;/* Synchronize cntvoff across all vtimers of a VM. */ timer_set_offset (vtimer,kvm_phys_timer_read ());
@@ -1297,6 +1311,8 @@ void kvm_timer_init_vhe(void) val =read_sysreg (cnthctl_el2); val |= (CNTHCTL_EL1PCEN << cnthctl_shift); val |= (CNTHCTL_EL1PCTEN << cnthctl_shift); +if (cpus_have_final_cap (ARM64_HAS_ECV_CNTPOFF )) + val |=CNTHCTL_ECV ;write_sysreg (val, cnthctl_el2); }
arch/arm64/kvm/hypercalls.c +1 -1
@@ -47,7 +47,7 @@ static void kvm_ptp_get_time(struct kvm_vcpu *vcpu, u64 *val) cycles = systime_snapshot.cycles - vcpu->kvm->arch.timer_data.voffset;break ;case KVM_PTP_PHYS_COUNTER : - cycles = systime_snapshot.cycles; + cycles = systime_snapshot.cycles - vcpu->kvm->arch.timer_data.poffset;break ;default :return ;
include/clocksource/arm_arch_timer.h +1 -0
@@ -21,6 +21,7 @@#define CNTHCTL_EVNTEN (1 << 2)#define CNTHCTL_EVNTDIR (1 << 3)#define CNTHCTL_EVNTI (0xF << 4) +#define CNTHCTL_ECV (1 << 12)enum arch_timer_reg {ARCH_TIMER_REG_CTRL ,
include/kvm/arm_arch_timer.h +2 -0
@@ -34,6 +34,8 @@ struct arch_timer_offset {struct arch_timer_vm_data {/* Offset applied to the virtual timer/counter */ u64 voffset; +/* Offset applied to the physical timer/counter */ +u64 poffset; };struct arch_timer_context {
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 5/18 KVM: arm64: timers: Allow physical offset without CNTPOFF_EL2
In reply to Marc ZyngierCNTPOFF_EL2 is awesome, but it is mostly vapourware, and no publicly available implementation has it. So for the common mortals, let's implement the emulated version of this thing.
It means trapping accesses to the physical counter and timer, and emulate some of it as necessary.
As for CNTPOFF_EL2, nobody sets the offset yet.
--- arch/arm64/include/asm/sysreg.h | 2 + arch/arm64/kvm/arch_timer.c | 98 +++++++++++++++++++++++------- arch/arm64/kvm/hyp/nvhe/timer-sr.c | 18 ++++-- arch/arm64/kvm/sys_regs.c | 9 +++ 4 files changed, 98 insertions(+), 29 deletions(-)
arch/arm64/include/asm/sysreg.h +2 -0
@@ -388,6 +388,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_CNTPCTSS_EL0 sys_reg(3, 3, 14, 0, 5)#define SYS_CNTVCTSS_EL0 sys_reg(3, 3, 14, 0, 6)
@@ -400,6 +401,7 @@#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_CNTP_CVAL sys_reg(0, 2, 0, 14, 0)#define __PMEV_op2 (n) ((n) & 0x7)
arch/arm64/kvm/arch_timer.c +75 -23
145 lines not on this page · raw message
arch/arm64/kvm/hyp/nvhe/timer-sr.c +12 -6
@@ -9,6 +9,7 @@#include <linux/kvm_host.h> #include <asm/kvm_hyp.h> +#include <asm/kvm_mmu.h> void __kvm_timer_set_cntvoff (u64 cntvoff) {
@@ -35,14 +36,19 @@ void __timer_disable_traps(struct kvm_vcpu *vcpu)*/ void __timer_enable_traps (struct kvm_vcpu *vcpu) { -u64 val; +u64 clr =0 , set =0 ;/* * Disallow physical timer access for the guest -* Physical counter access is allowed +* Physical counter access is allowed if no offset is enforced +* or running protected (we don't offset anything in this case). */ - val =read_sysreg (cnthctl_el2); - val &= ~CNTHCTL_EL1PCEN ; - val |=CNTHCTL_EL1PCTEN ; -write_sysreg (val, cnthctl_el2); + clr =CNTHCTL_EL1PCEN ; +if (is_protected_kvm_enabled () || + !kern_hyp_va (vcpu->kvm)->arch.timer_data.poffset) + set |=CNTHCTL_EL1PCTEN ; +else + clr |=CNTHCTL_EL1PCTEN ; + +sysreg_clear_set (cnthctl_el2, clr, set); }
arch/arm64/kvm/sys_regs.c +9 -0
@@ -1139,6 +1139,12 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu, tmr =TIMER_PTIMER ; treg =TIMER_REG_CVAL ;break ; +case SYS_CNTPCT_EL0 : +case SYS_CNTPCTSS_EL0 : +case SYS_AARCH32_CNTPCT : + tmr =TIMER_PTIMER ; + treg =TIMER_REG_CNT ; +break ;default :print_sys_reg_msg (p,"%s" ,"Unhandled trapped timer register" );kvm_inject_undefined (vcpu);
@@ -2075,6 +2081,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {AMU_AMEVTYPER1_EL0 (14 ),AMU_AMEVTYPER1_EL0 (15 ), + { SYS_DESC(SYS_CNTPCT_EL0), access_arch_timer }, + { SYS_DESC(SYS_CNTPCTSS_EL0), access_arch_timer }, { SYS_DESC(SYS_CNTP_TVAL_EL0), access_arch_timer }, { SYS_DESC(SYS_CNTP_CTL_EL0), access_arch_timer }, { SYS_DESC(SYS_CNTP_CVAL_EL0), access_arch_timer },
@@ -2525,6 +2533,7 @@ static const struct sys_reg_desc cp15_64_regs[] = { { Op1( 0), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, TTBR0_EL1 }, { CP15_PMU_SYS_REG(DIRECT, 0, 0, 9, 0), .access = access_pmu_evcntr }, { Op1( 0), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_SGI1R */ + { SYS_DESC(SYS_AARCH32_CNTPCT), access_arch_timer }, { Op1( 1), CRn( 0), CRm( 2), Op2( 0), access_vm_reg, NULL, TTBR1_EL1 }, { Op1( 1), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_ASGI1R */ { Op1( 2), CRn( 0), CRm(12), Op2( 0), access_gic_sgi }, /* ICC_SGI0R */
Signature · 1 line
2.34.1
Re: [PATCH v3 05/18] KVM: arm64: timers: Allow physical offset without CNTPOFF_EL2
Hey Marc,
Quoted 8 lines · Marc Zyngier
CNTPOFF_EL2is awesome, but it is mostly vapourware, and no publicly available implementation has it. So for the common mortals, let's implement the emulated version of this thing.It means trapping accesses to the physical counter and timer, and emulate some of it as necessary.
As for
CNTPOFF_EL2, nobody sets the offset yet.
Did you consider implementing a 'fast' exit handler for counter reads? When I took a stab at this a long time ago I had something similar [*]. Since physical counter emulation is going to effectively be used by all systems out there right now it might be worth giving the best emulation implementation we can.
[*]: https://lore.kernel.org/kvmarm/[email protected]/
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerRe: [PATCH v3 05/18] KVM: arm64: timers: Allow physical offset without CNTPOFF_EL2
Quoted 11 lines · Oliver Upton
Hey Marc,
Quoted 7 lines · Marc Zyngier
CNTPOFF_EL2is awesome, but it is mostly vapourware, and no publicly available implementation has it. So for the common mortals, let's implement the emulated version of this thing.It means trapping accesses to the physical counter and timer, and emulate some of it as necessary.
Quoted 7 lines
Quoted 1 line
As for
CNTPOFF_EL2, nobody sets the offset yet.Did you consider implementing a 'fast' exit handler for counter reads? When I took a stab at this a long time ago I had something similar [*]. Since physical counter emulation is going to effectively be used by all systems out there right now it might be worth giving the best emulation implementation we can.
Indeed, I have some of that in the NV code, which needs to be augmented. I'm not sure any guest deeply cares about the physical counter outside of NV, but I'm happy to hack something together if someone screams loudly enough!
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v3 6/18 KVM: arm64: Expose {un,}lock_all_vcpus() to the rest of KVM
In reply to Marc ZyngierBeing able to lock/unlock all vcpus in one go is a feature that only the vgic has enjoyed so far. Let's be brave and expose it to the world.
--- arch/arm64/include/asm/kvm_host.h | 3 ++ arch/arm64/kvm/arm.c | 43 +++++++++++++++++++++++++++ arch/arm64/kvm/vgic/vgic-kvm-device.c | 38 ----------------------- arch/arm64/kvm/vgic/vgic.h | 3 -- 4 files changed, 46 insertions(+), 41 deletions(-)
arch/arm64/include/asm/kvm_host.h +3 -0
@@ -922,6 +922,9 @@ void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);int __init kvm_sys_reg_table_init (void ); +bool lock_all_vcpus (struct kvm *kvm); +void unlock_all_vcpus (struct kvm *kvm); +/* MMIO helpers */ void kvm_mmio_write_buf (void *buf,unsigned int len,unsigned long data);unsigned long kvm_mmio_read_buf (const void *buf,unsigned int len);
arch/arm64/kvm/arm.c +43 -0
49 lines not on this page · raw message
arch/arm64/kvm/vgic/vgic-kvm-device.c +0 -38
@@ -342,44 +342,6 @@ int vgic_v2_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,return 0 ; } -/* unlocks vcpus from @vcpu_lock_idx and smaller */ -static void unlock_vcpus (struct kvm *kvm,int vcpu_lock_idx) -{ -struct kvm_vcpu *tmp_vcpu; - -for (; vcpu_lock_idx >=0 ; vcpu_lock_idx--) { - tmp_vcpu =kvm_get_vcpu (kvm, vcpu_lock_idx); -mutex_unlock (&tmp_vcpu->mutex); - } -} - -void unlock_all_vcpus (struct kvm *kvm) -{ -unlock_vcpus (kvm,atomic_read (&kvm->online_vcpus) -1 ); -} - -/* Returns true if all vcpus were locked, false otherwise */ -bool lock_all_vcpus (struct kvm *kvm) -{ -struct kvm_vcpu *tmp_vcpu; -unsigned long c; - -/* -* Any time a vcpu is run, vcpu_load is called which tries to grab the -* vcpu->mutex. By grabbing the vcpu->mutex of all VCPUs we ensure -* that no other VCPUs are run and fiddle with the vgic state while we -* access it. -*/ -kvm_for_each_vcpu (c, tmp_vcpu, kvm) { -if (!mutex_trylock (&tmp_vcpu->mutex)) { -unlock_vcpus (kvm, c -1 ); -return false; - } - } - -return true; -} -/** * vgic_v2_attr_regs_access - allows user space to access VGIC v2 state *
arch/arm64/kvm/vgic/vgic.h +0 -3
@@ -273,9 +273,6 @@ int vgic_init(struct kvm *kvm);void vgic_debug_init (struct kvm *kvm);void vgic_debug_destroy (struct kvm *kvm); -bool lock_all_vcpus (struct kvm *kvm); -void unlock_all_vcpus (struct kvm *kvm); -static inline int vgic_v3_max_apr_idx (struct kvm_vcpu *vcpu) {struct vgic_cpu *cpu_if = &vcpu->arch.vgic_cpu;
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 7/18 KVM: arm64: timers: Allow userspace to set the global counter offset
In reply to Marc ZyngierAnd this is the moment you have all been waiting for: setting the counter offset from userspace.
We expose a brand new capability that reports the ability to set the offset for both the virtual and physical sides.
In keeping with the architecture, the offset is expressed as a delta that is substracted from the physical counter value.
Once this new API is used, there is no going back, and the counters cannot be written to to set the offsets implicitly (the writes are instead ignored).
--- arch/arm64/include/asm/kvm_host.h | 4 +++ arch/arm64/include/uapi/asm/kvm.h | 9 ++++++ arch/arm64/kvm/arch_timer.c | 46 +++++++++++++++++++++++++++---- arch/arm64/kvm/arm.c | 8 ++++++ include/uapi/linux/kvm.h | 3 ++ 5 files changed, 65 insertions(+), 5 deletions(-)
arch/arm64/include/asm/kvm_host.h +4 -0
@@ -221,6 +221,8 @@ struct kvm_arch {#define KVM_ARCH_FLAG_EL1_32BIT 4/* PSCI SYSTEM_SUSPEND enabled for the guest */ #define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5 +/* VM counter offset */ +#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6unsigned long flags;
@@ -1010,6 +1012,8 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,long kvm_vm_ioctl_mte_copy_tags (struct kvm *kvm,struct kvm_arm_copy_mte_tags *copy_tags); +int kvm_vm_ioctl_set_counter_offset (struct kvm *kvm, +struct kvm_arm_counter_offset *offset);/* Guest/host FPSIMD coordination helpers */ int kvm_arch_vcpu_run_map_fp (struct kvm_vcpu *vcpu);
arch/arm64/include/uapi/asm/kvm.h +9 -0
@@ -198,6 +198,15 @@ struct kvm_arm_copy_mte_tags {__u64 reserved[2 ]; }; +/* +* Counter/Timer offset structure. Describe the virtual/physical offset. +* To be used with KVM_ARM_SET_COUNTER_OFFSET. +*/ +struct kvm_arm_counter_offset { +__u64 counter_offset; +__u64 reserved; +}; +#define KVM_ARM_TAGS_TO_GUEST 0#define KVM_ARM_TAGS_FROM_GUEST 1
arch/arm64/kvm/arch_timer.c +41 -5
67 lines not on this page · raw message
arch/arm64/kvm/arm.c +8 -0
@@ -220,6 +220,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_VCPU_ATTRIBUTES :case KVM_CAP_PTP_KVM :case KVM_CAP_ARM_SYSTEM_SUSPEND : +case KVM_CAP_COUNTER_OFFSET : r =1 ;break ;case KVM_CAP_SET_GUEST_DEBUG2 :
@@ -1479,6 +1480,13 @@ long kvm_arch_vm_ioctl(struct file *filp,return -EFAULT ;return kvm_vm_ioctl_mte_copy_tags (kvm, ©_tags); } +case KVM_ARM_SET_COUNTER_OFFSET : { +struct kvm_arm_counter_offset offset; + +if (copy_from_user (&offset, argp,sizeof (offset))) +return -EFAULT ; +return kvm_vm_ioctl_set_counter_offset (kvm, &offset); + }default :return -EINVAL ; }
include/uapi/linux/kvm.h +3 -0
@@ -1184,6 +1184,7 @@ struct kvm_ppc_resize_hpt {#define KVM_CAP_S390_PROTECTED_ASYNC_DISABLE 224#define KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP 225#define KVM_CAP_PMU_EVENT_MASKED_EVENTS 226 +#define KVM_CAP_COUNTER_OFFSET 227#ifdef KVM_CAP_IRQ_ROUTING
@@ -1543,6 +1544,8 @@ struct kvm_s390_ucas_mapping {#define KVM_SET_PMU_EVENT_FILTER _IOW(KVMIO, 0xb2, struct kvm_pmu_event_filter)#define KVM_PPC_SVM_OFF _IO(KVMIO, 0xb3)#define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags) +/* Available with KVM_CAP_COUNTER_OFFSET */ +#define KVM_ARM_SET_COUNTER_OFFSET _IOW(KVMIO, 0xb5, struct kvm_arm_counter_offset)/* ioctl for vm fd */ #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
Signature · 1 line
2.34.1
Re: [PATCH v3 07/18] KVM: arm64: timers: Allow userspace to set the global counter offset
Hi Marc,
Quoted 115 lines · Marc Zyngier
And this is the moment you have all been waiting for: setting the counter offset from userspace.
We expose a brand new capability that reports the ability to set the offset for both the virtual and physical sides.
In keeping with the architecture, the offset is expressed as a delta that is substracted from the physical counter value.
Once this new API is used, there is no going back, and the counters cannot be written to to set the offsets implicitly (the writes are instead ignored).
Reviewed-by Colton Lewis <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/kvm_host.h | 4 +++ arch/arm64/include/uapi/asm/kvm.h | 9 ++++++ arch/arm64/kvm/arch_timer.c | 46 +++++++++++++++++++++++++++---- arch/arm64/kvm/arm.c | 8 ++++++ include/uapi/linux/kvm.h | 3 ++ 5 files changed, 65 insertions(+), 5 deletions(-)arch/arm64/include/asm/kvm_host.h +4
-0@@ -221,6 +221,8 @@ struct kvm_arch {#define KVM_ARCH_FLAG_EL1_32BIT 4/* PSCI SYSTEM_SUSPEND enabled for the guest */ #define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5 +/* VM counter offset */ +#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6unsigned long flags;@@ -1010,6 +1012,8 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,long kvm_vm_ioctl_mte_copy_tags (struct kvm *kvm,struct kvm_arm_copy_mte_tags *copy_tags); +int kvm_vm_ioctl_set_counter_offset (struct kvm *kvm, +struct kvm_arm_counter_offset *offset);/* Guest/host FPSIMD coordination helpers */ int kvm_arch_vcpu_run_map_fp (struct kvm_vcpu *vcpu);arch/arm64/include/uapi/asm/kvm.h +9
-0@@ -198,6 +198,15 @@ struct kvm_arm_copy_mte_tags {__u64 reserved[2 ]; }; +/* +* Counter/Timer offset structure. Describe the virtual/physical offset. +* To be used with KVM_ARM_SET_COUNTER_OFFSET. +*/ +struct kvm_arm_counter_offset { +__u64 counter_offset; +__u64 reserved; +}; +#define KVM_ARM_TAGS_TO_GUEST 0#define KVM_ARM_TAGS_FROM_GUEST 1arch/arm64/kvm/arch_timer.c +17
-5@@ -851,9 +851,11 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) ptimer->vcpu = vcpu; ptimer->offset.vm_offset = &vcpu->kvm->arch.timer_data.poffset; -/* Synchronize cntvoff across all vtimers of a VM. */ -timer_set_offset (vtimer,kvm_phys_timer_read ()); -timer_set_offset (ptimer,0 ); +/* Synchronize offsets across timers of a VM if not already provided */ +if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , &vcpu->kvm->arch.flags)) { +timer_set_offset (vtimer,kvm_phys_timer_read ()); +timer_set_offset (ptimer,0 ); + }hrtimer_init (&timer->bg_timer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); timer->bg_timer.function = kvm_bg_timer_expire;@@ -897,8 +899,11 @@ int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)kvm_arm_timer_write (vcpu, timer,TIMER_REG_CTL , value);break ;case KVM_REG_ARM_TIMER_CNT : - timer =vcpu_vtimer (vcpu); -timer_set_offset (timer,kvm_phys_timer_read () - value); +if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , + &vcpu->kvm->arch.flags)) { + timer =vcpu_vtimer (vcpu); +timer_set_offset (timer,kvm_phys_timer_read () - value); + }break ;case KVM_REG_ARM_TIMER_CVAL : timer =vcpu_vtimer (vcpu);@@ -908,6 +913,13 @@ int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value) timer =vcpu_ptimer (vcpu);kvm_arm_timer_write (vcpu, timer,TIMER_REG_CTL , value);break ; +case KVM_REG_ARM_PTIMER_CNT : +if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , + &vcpu->kvm->arch.flags)) { + timer =vcpu_ptimer (vcpu); +timer_set_offset (timer,kvm_phys_timer_read () - value); + } +break ;case KVM_REG_ARM_PTIMER_CVAL : timer =vcpu_ptimer (vcpu);kvm_arm_timer_write (vcpu, timer,TIMER_REG_CVAL , value);@@ -1443,3 +1455,27 @@ int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)return -ENXIO ;
arch/arm64/kvm/arch_timer.c int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) · line 1466
Quoted 10 lines
arch/arm64/kvm/arch_timer.c +9
-0} + +int kvm_vm_ioctl_set_counter_offset (struct kvm *kvm, +struct kvm_arm_counter_offset *offset) +{ +if (offset->reserved) +return -EINVAL ; + +if (!lock_all_vcpus (kvm)) +return -EBUSY ;
Similar to what you had mentioned over on the lock (un)inversion series, doesn't this risk racing with vCPU creation w/o holding the kvm->lock?
Alternatively you could require this ioctl to be issued before any vCPUs are created to avoid the need to lock them all here. But, if you see value in poking this at runtime then fine by me.
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerRe: [PATCH v3 07/18] KVM: arm64: timers: Allow userspace to set the global counter offset
Quoted 118 lines · Oliver Upton
Hi Marc,
Quoted 114 lines · Marc Zyngier
And this is the moment you have all been waiting for: setting the counter offset from userspace.
We expose a brand new capability that reports the ability to set the offset for both the virtual and physical sides.
In keeping with the architecture, the offset is expressed as a delta that is substracted from the physical counter value.
Once this new API is used, there is no going back, and the counters cannot be written to to set the offsets implicitly (the writes are instead ignored).
Reviewed-by Colton Lewis <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/kvm_host.h | 4 +++ arch/arm64/include/uapi/asm/kvm.h | 9 ++++++ arch/arm64/kvm/arch_timer.c | 46 +++++++++++++++++++++++++++---- arch/arm64/kvm/arm.c | 8 ++++++ include/uapi/linux/kvm.h | 3 ++ 5 files changed, 65 insertions(+), 5 deletions(-)arch/arm64/include/asm/kvm_host.h +4
-0@@ -221,6 +221,8 @@ struct kvm_arch {#define KVM_ARCH_FLAG_EL1_32BIT 4/* PSCI SYSTEM_SUSPEND enabled for the guest */ #define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5 +/* VM counter offset */ +#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6unsigned long flags;@@ -1010,6 +1012,8 @@ int kvm_arm_vcpu_arch_has_attr(struct kvm_vcpu *vcpu,long kvm_vm_ioctl_mte_copy_tags (struct kvm *kvm,struct kvm_arm_copy_mte_tags *copy_tags); +int kvm_vm_ioctl_set_counter_offset (struct kvm *kvm, +struct kvm_arm_counter_offset *offset);/* Guest/host FPSIMD coordination helpers */ int kvm_arch_vcpu_run_map_fp (struct kvm_vcpu *vcpu);arch/arm64/include/uapi/asm/kvm.h +9
-0@@ -198,6 +198,15 @@ struct kvm_arm_copy_mte_tags {__u64 reserved[2 ]; }; +/* +* Counter/Timer offset structure. Describe the virtual/physical offset. +* To be used with KVM_ARM_SET_COUNTER_OFFSET. +*/ +struct kvm_arm_counter_offset { +__u64 counter_offset; +__u64 reserved; +}; +#define KVM_ARM_TAGS_TO_GUEST 0#define KVM_ARM_TAGS_FROM_GUEST 1arch/arm64/kvm/arch_timer.c +17
-5@@ -851,9 +851,11 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) ptimer->vcpu = vcpu; ptimer->offset.vm_offset = &vcpu->kvm->arch.timer_data.poffset; -/* Synchronize cntvoff across all vtimers of a VM. */ -timer_set_offset (vtimer,kvm_phys_timer_read ()); -timer_set_offset (ptimer,0 ); +/* Synchronize offsets across timers of a VM if not already provided */ +if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , &vcpu->kvm->arch.flags)) { +timer_set_offset (vtimer,kvm_phys_timer_read ()); +timer_set_offset (ptimer,0 ); + }hrtimer_init (&timer->bg_timer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); timer->bg_timer.function = kvm_bg_timer_expire;@@ -897,8 +899,11 @@ int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)kvm_arm_timer_write (vcpu, timer,TIMER_REG_CTL , value);break ;case KVM_REG_ARM_TIMER_CNT : - timer =vcpu_vtimer (vcpu); -timer_set_offset (timer,kvm_phys_timer_read () - value); +if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , + &vcpu->kvm->arch.flags)) { + timer =vcpu_vtimer (vcpu); +timer_set_offset (timer,kvm_phys_timer_read () - value); + }break ;case KVM_REG_ARM_TIMER_CVAL : timer =vcpu_vtimer (vcpu);@@ -908,6 +913,13 @@ int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value) timer =vcpu_ptimer (vcpu);kvm_arm_timer_write (vcpu, timer,TIMER_REG_CTL , value);break ; +case KVM_REG_ARM_PTIMER_CNT : +if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , + &vcpu->kvm->arch.flags)) { + timer =vcpu_ptimer (vcpu); +timer_set_offset (timer,kvm_phys_timer_read () - value); + } +break ;case KVM_REG_ARM_PTIMER_CVAL : timer =vcpu_ptimer (vcpu);kvm_arm_timer_write (vcpu, timer,TIMER_REG_CVAL , value);@@ -1443,3 +1455,27 @@ int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)
Quoted 14 lines
arch/arm64/kvm/arch_timer.c int kvm_arm_timer_has_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr) · line 1466
Quoted 11 lines
arch/arm64/kvm/arch_timer.c +9
-0return -ENXIO ; } + +int kvm_vm_ioctl_set_counter_offset (struct kvm *kvm, +struct kvm_arm_counter_offset *offset) +{ +if (offset->reserved) +return -EINVAL ; + +if (!lock_all_vcpus (kvm)) +return -EBUSY ;Similar to what you had mentioned over on the lock (un)inversion series, doesn't this risk racing with vCPU creation w/o holding the
kvm->lock?
Huh, I was under the impression that kvm_vm_ioctl() would take the kvm->lock, but it really doesn't! I'll fix that right away.
Quoted 3 lines
Alternatively you could require this ioctl to be issued before any vCPUs are created to avoid the need to lock them all here. But, if you see value in poking this at runtime then fine by me.
I think there is some value in allowing the offset adjustment to be set at the last possible point, right before a vcpu is ran.
Thanks for the heads up!
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v3 8/18 KVM: arm64: timers: Allow save/restoring of the physical timer
In reply to Marc ZyngierNothing like being 10 year late to a party!
Now that userspace can set counter offsets, we can save/restore the physical timer as well!
Nobody really cared so far, but you're welcome anyway.
--- arch/arm64/kvm/guest.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-)
arch/arm64/kvm/guest.c +17 -12
@@ -590,11 +590,16 @@ static unsigned long num_core_regs(const struct kvm_vcpu *vcpu)return copy_core_reg_indices (vcpu,NULL ); } -/** -* ARM64 versions of the TIMER registers, always available on arm64 -*/ +static const u64 timer_reg_list[] = { +KVM_REG_ARM_TIMER_CTL , +KVM_REG_ARM_TIMER_CNT , +KVM_REG_ARM_TIMER_CVAL , +KVM_REG_ARM_PTIMER_CTL , +KVM_REG_ARM_PTIMER_CNT , +KVM_REG_ARM_PTIMER_CVAL , +}; -#define NUM_TIMER_REGS 3 +#define NUM_TIMER_REGS ARRAY_SIZE(timer_reg_list)static bool is_timer_reg (u64 index) {
@@ -602,6 +607,9 @@ static bool is_timer_reg(u64 index)case KVM_REG_ARM_TIMER_CTL :case KVM_REG_ARM_TIMER_CNT :case KVM_REG_ARM_TIMER_CVAL : +case KVM_REG_ARM_PTIMER_CTL : +case KVM_REG_ARM_PTIMER_CNT : +case KVM_REG_ARM_PTIMER_CVAL :return true; }return false;
@@ -609,14 +617,11 @@ static bool is_timer_reg(u64 index)static int copy_timer_indices (struct kvm_vcpu *vcpu,u64 __user *uindices) { -if (put_user (KVM_REG_ARM_TIMER_CTL , uindices)) -return -EFAULT ; - uindices++; -if (put_user (KVM_REG_ARM_TIMER_CNT , uindices)) -return -EFAULT ; - uindices++; -if (put_user (KVM_REG_ARM_TIMER_CVAL , uindices)) -return -EFAULT ; +for (int i =0 ; i <NUM_TIMER_REGS ; i++) { +if (put_user (timer_reg_list[i], uindices)) +return -EFAULT ; + uindices++; + }return 0 ; }
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 9/18 KVM: arm64: timers: Rationalise per-vcpu timer init
In reply to Marc ZyngierThe way we initialise our timer contexts may be satisfactory for two timers, but will be getting pretty annoying with four.
Cleanup the whole thing by removing the code duplication and getting rid of unused IRQ configuration elements.
--- arch/arm64/kvm/arch_timer.c | 73 +++++++++++++++++++----------------- include/kvm/arm_arch_timer.h | 1 - 2 files changed, 39 insertions(+), 35 deletions(-)
arch/arm64/kvm/arch_timer.c +39 -34
@@ -30,14 +30,9 @@ static u32 host_ptimer_irq_flags;static DEFINE_STATIC_KEY_FALSE (has_gic_active_state); -static const struct kvm_irq_level default_ptimer_irq = { - .irq =30 , - .level =1 , -}; - -static const struct kvm_irq_level default_vtimer_irq = { - .irq =27 , - .level =1 , +static const u8 default_ppi[] = { + [TIMER_PTIMER ] =30 , + [TIMER_VTIMER ] =27 , };static bool kvm_timer_irq_can_fire (struct arch_timer_context *timer_ctx);
@@ -820,12 +815,14 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)* resets the timer to be disabled and unmasked and is compliant with * the ARMv7 architecture. */ -timer_set_ctl (vcpu_vtimer (vcpu),0 ); -timer_set_ctl (vcpu_ptimer (vcpu),0 ); +for (int i =0 ; i <NR_KVM_TIMERS ; i++) +timer_set_ctl (vcpu_get_timer (vcpu, i),0 ); +if (timer->enabled) { -kvm_timer_update_irq (vcpu, false,vcpu_vtimer (vcpu)); -kvm_timer_update_irq (vcpu, false,vcpu_ptimer (vcpu)); +for (int i =0 ; i <NR_KVM_TIMERS ; i++) +kvm_timer_update_irq (vcpu, false, +vcpu_get_timer (vcpu, i));if (irqchip_in_kernel (vcpu->kvm)) {kvm_vgic_reset_mapped_irq (vcpu, map.direct_vtimer->irq.irq);
@@ -840,39 +837,47 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)return 0 ; } +static void timer_context_init (struct kvm_vcpu *vcpu,int timerid) +{ +struct arch_timer_context *ctxt =vcpu_get_timer (vcpu, timerid); +struct kvm *kvm = vcpu->kvm; + + ctxt->vcpu = vcpu; + +if (timerid ==TIMER_VTIMER ) + ctxt->offset.vm_offset = &kvm->arch.timer_data.voffset; +else + ctxt->offset.vm_offset = &kvm->arch.timer_data.poffset; + +hrtimer_init (&ctxt->hrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); + ctxt->hrtimer.function = kvm_hrtimer_expire; + ctxt->irq.irq = default_ppi[timerid]; + +switch (timerid) { +case TIMER_PTIMER : + ctxt->host_timer_irq = host_ptimer_irq; +break ; +case TIMER_VTIMER : + ctxt->host_timer_irq = host_vtimer_irq; +break ; + } +} +void kvm_timer_vcpu_init (struct kvm_vcpu *vcpu) {struct arch_timer_cpu *timer =vcpu_timer (vcpu); -struct arch_timer_context *vtimer =vcpu_vtimer (vcpu); -struct arch_timer_context *ptimer =vcpu_ptimer (vcpu); - vtimer->vcpu = vcpu; - vtimer->offset.vm_offset = &vcpu->kvm->arch.timer_data.voffset; - ptimer->vcpu = vcpu; - ptimer->offset.vm_offset = &vcpu->kvm->arch.timer_data.poffset; +for (int i =0 ; i <NR_KVM_TIMERS ; i++) +timer_context_init (vcpu, i);/* Synchronize offsets across timers of a VM if not already provided */ if (!test_bit (KVM_ARCH_FLAG_VM_COUNTER_OFFSET , &vcpu->kvm->arch.flags)) { -timer_set_offset (vtimer,kvm_phys_timer_read ()); -timer_set_offset (ptimer,0 ); +timer_set_offset (vcpu_vtimer (vcpu),kvm_phys_timer_read ()); +timer_set_offset (vcpu_ptimer (vcpu),0 ); }hrtimer_init (&timer->bg_timer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); timer->bg_timer.function = kvm_bg_timer_expire; - -hrtimer_init (&vtimer->hrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); -hrtimer_init (&ptimer->hrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); - vtimer->hrtimer.function = kvm_hrtimer_expire; - ptimer->hrtimer.function = kvm_hrtimer_expire; - - vtimer->irq.irq = default_vtimer_irq.irq; - ptimer->irq.irq = default_ptimer_irq.irq; - - vtimer->host_timer_irq = host_vtimer_irq; - ptimer->host_timer_irq = host_ptimer_irq; - - vtimer->host_timer_irq_flags = host_vtimer_irq_flags; - ptimer->host_timer_irq_flags = host_ptimer_irq_flags; }void kvm_timer_cpu_up (void )
include/kvm/arm_arch_timer.h +0 -1
@@ -59,7 +59,6 @@ struct arch_timer_context {/* Duplicated state from arch_timer.c for convenience */ u32 host_timer_irq; -u32 host_timer_irq_flags; };struct timer_map {
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 10/18 KVM: arm64: timers: Abstract per-timer IRQ access
In reply to Marc ZyngierAs we are about to move the location of the per-timer IRQ into the VM structure, abstract the location of the IRQ behind an accessor. This will make the repainting sligntly less painful.
--- arch/arm64/kvm/arch_timer.c | 38 ++++++++++++++++++------------------ include/kvm/arm_arch_timer.h | 2 ++ 2 files changed, 21 insertions(+), 19 deletions(-)
arch/arm64/kvm/arch_timer.c +19 -19
@@ -392,12 +392,12 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level,int ret; timer_ctx->irq.level = new_level; -trace_kvm_timer_update_irq (vcpu->vcpu_id, timer_ctx->irq.irq, +trace_kvm_timer_update_irq (vcpu->vcpu_id,timer_irq (timer_ctx), timer_ctx->irq.level);if (!userspace_irqchip (vcpu->kvm)) { ret =kvm_vgic_inject_irq (vcpu->kvm, vcpu->vcpu_id, - timer_ctx->irq.irq, +timer_irq (timer_ctx), timer_ctx->irq.level, timer_ctx);WARN_ON (ret);
@@ -607,7 +607,7 @@ static void kvm_timer_vcpu_load_gic(struct arch_timer_context *ctx)kvm_timer_update_irq (ctx->vcpu,kvm_timer_should_fire (ctx), ctx);if (irqchip_in_kernel (vcpu->kvm)) - phys_active =kvm_vgic_map_is_active (vcpu, ctx->irq.irq); + phys_active =kvm_vgic_map_is_active (vcpu,timer_irq (ctx)); phys_active |= ctx->irq.level;
@@ -825,9 +825,9 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu) vcpu_get_timer(vcpu, i));if (irqchip_in_kernel (vcpu->kvm)) { -kvm_vgic_reset_mapped_irq (vcpu, map.direct_vtimer->irq.irq); +kvm_vgic_reset_mapped_irq (vcpu,timer_irq (map.direct_vtimer));if (map.direct_ptimer) -kvm_vgic_reset_mapped_irq (vcpu, map.direct_ptimer->irq.irq); +kvm_vgic_reset_mapped_irq (vcpu,timer_irq (map.direct_ptimer)); } }
@@ -851,7 +851,7 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)hrtimer_init (&ctxt->hrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); ctxt->hrtimer.function = kvm_hrtimer_expire; - ctxt->irq.irq = default_ppi[timerid]; +timer_irq (ctxt) = default_ppi[timerid];switch (timerid) {case TIMER_PTIMER :
@@ -1295,19 +1295,19 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)int vtimer_irq, ptimer_irq, ret;unsigned long i; - vtimer_irq =vcpu_vtimer (vcpu)->irq.irq; + vtimer_irq =timer_irq (vcpu_vtimer (vcpu)); ret =kvm_vgic_set_owner (vcpu, vtimer_irq,vcpu_vtimer (vcpu));if (ret)return false; - ptimer_irq =vcpu_ptimer (vcpu)->irq.irq; + ptimer_irq =timer_irq (vcpu_ptimer (vcpu)); ret =kvm_vgic_set_owner (vcpu, ptimer_irq,vcpu_ptimer (vcpu));if (ret)return false;kvm_for_each_vcpu (i, vcpu, vcpu->kvm) { -if (vcpu_vtimer (vcpu)->irq.irq != vtimer_irq || -vcpu_ptimer (vcpu)->irq.irq != ptimer_irq) +if (timer_irq (vcpu_vtimer (vcpu)) != vtimer_irq || +timer_irq (vcpu_ptimer (vcpu)) != ptimer_irq)return false; }
@@ -1322,9 +1322,9 @@ bool kvm_arch_timer_get_input_level(int vintid)if (WARN (!vcpu,"No vcpu context!\n" ))return false; -if (vintid ==vcpu_vtimer (vcpu)->irq.irq) +if (vintid ==timer_irq (vcpu_vtimer (vcpu))) timer =vcpu_vtimer (vcpu); -else if (vintid ==vcpu_ptimer (vcpu)->irq.irq) +else if (vintid ==timer_irq (vcpu_ptimer (vcpu))) timer =vcpu_ptimer (vcpu);else BUG ();
@@ -1358,7 +1358,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu) ret =kvm_vgic_map_phys_irq (vcpu, map.direct_vtimer->host_timer_irq, - map.direct_vtimer->irq.irq, +timer_irq (map.direct_vtimer), &arch_timer_irq_ops);if (ret)return ret;
@@ -1366,7 +1366,7 @@ int kvm_timer_enable(struct kvm_vcpu *vcpu)if (map.direct_ptimer) { ret =kvm_vgic_map_phys_irq (vcpu, map.direct_ptimer->host_timer_irq, - map.direct_ptimer->irq.irq, +timer_irq (map.direct_ptimer), &arch_timer_irq_ops); }
@@ -1391,8 +1391,8 @@ static void set_timer_irqs(struct kvm *kvm, int vtimer_irq, int ptimer_irq)unsigned long i;kvm_for_each_vcpu (i, vcpu, kvm) { -vcpu_vtimer (vcpu)->irq.irq = vtimer_irq; -vcpu_ptimer (vcpu)->irq.irq = ptimer_irq; +timer_irq (vcpu_vtimer (vcpu)) = vtimer_irq; +timer_irq (vcpu_ptimer (vcpu)) = ptimer_irq; } }
@@ -1417,10 +1417,10 @@ int kvm_arm_timer_set_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)switch (attr->attr) {case KVM_ARM_VCPU_TIMER_IRQ_VTIMER : -set_timer_irqs (vcpu->kvm, irq, ptimer->irq.irq); +set_timer_irqs (vcpu->kvm, irq,timer_irq (ptimer));break ;case KVM_ARM_VCPU_TIMER_IRQ_PTIMER : -set_timer_irqs (vcpu->kvm, vtimer->irq.irq, irq); +set_timer_irqs (vcpu->kvm,timer_irq (vtimer), irq);break ;default :return -ENXIO ;
@@ -1446,7 +1446,7 @@ int kvm_arm_timer_get_attr(struct kvm_vcpu *vcpu, struct kvm_device_attr *attr)return -ENXIO ; } - irq = timer->irq.irq; + irq =timer_irq (timer);return put_user (irq, uaddr); }
include/kvm/arm_arch_timer.h +2 -0
@@ -109,6 +109,8 @@ bool kvm_arch_timer_get_input_level(int vintid);#define arch_timer_ctx_index (ctx) ((ctx) - vcpu_timer((ctx)->vcpu)->timers) +#define timer_irq (ctx) ((ctx)->irq.irq) +u64 kvm_arm_timer_read_sysreg (struct kvm_vcpu *vcpu,enum kvm_arch_timers tmr,enum kvm_arch_timer_regs treg);
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 11/18 KVM: arm64: timers: Move the timer IRQs into arch_timer_vm_data
In reply to Marc ZyngierHaving the timer IRQs duplicated into each vcpu isn't great, and becomes absolutely awful with NV. So let's move these into the per-VM arch_timer_vm_data structure.
This simplifies a lot of code, but requires us to introduce a mutex so that we can reason about userspace trying to change an interrupt number while another vcpu is running, something that wasn't really well handled so far.
--- arch/arm64/include/asm/kvm_host.h | 2 + arch/arm64/kvm/arch_timer.c | 104 +++++++++++++++++------------- arch/arm64/kvm/arm.c | 2 + include/kvm/arm_arch_timer.h | 18 ++++-- 4 files changed, 78 insertions(+), 48 deletions(-)
arch/arm64/include/asm/kvm_host.h +2 -0
@@ -223,6 +223,8 @@ struct kvm_arch {#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5/* VM counter offset */ #define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6 +/* Timer PPIs made immutable */ +#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 7unsigned long flags;
arch/arm64/kvm/arch_timer.c +60 -44
161 lines not on this page · raw message
arch/arm64/kvm/arm.c +2 -0
@@ -148,6 +148,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)kvm_vgic_early_init (kvm); +kvm_timer_init_vm (kvm); +/* The maximum number of VCPUs is limited by the host's GIC model */ kvm->max_vcpus =kvm_arm_default_max_vcpus ();
include/kvm/arm_arch_timer.h +14 -4
@@ -36,14 +36,16 @@ struct arch_timer_vm_data {u64 voffset;/* Offset applied to the physical timer/counter */ u64 poffset; + +struct mutex lock; + +/* The PPI for each timer, global to the VM */ +u8 ppi[NR_KVM_TIMERS ]; };struct arch_timer_context {struct kvm_vcpu *vcpu; -/* Timer IRQ */ -struct kvm_irq_level irq; -/* Emulated Timer (may be unused) */ struct hrtimer hrtimer;u64 ns_frac;
@@ -57,6 +59,11 @@ struct arch_timer_context {*/ bool loaded; +/* Output level of the timer IRQ */ +struct { +bool level; + } irq; +/* Duplicated state from arch_timer.c for convenience */ u32 host_timer_irq; };
@@ -86,6 +93,8 @@ bool kvm_timer_should_notify_user(struct kvm_vcpu *vcpu);void kvm_timer_update_run (struct kvm_vcpu *vcpu);void kvm_timer_vcpu_terminate (struct kvm_vcpu *vcpu); +void kvm_timer_init_vm (struct kvm *kvm); +u64 kvm_arm_timer_get_reg (struct kvm_vcpu *,u64 regid);int kvm_arm_timer_set_reg (struct kvm_vcpu *,u64 regid,u64 value);
@@ -109,7 +118,8 @@ bool kvm_arch_timer_get_input_level(int vintid);#define arch_timer_ctx_index (ctx) ((ctx) - vcpu_timer((ctx)->vcpu)->timers) -#define timer_irq (ctx) ((ctx)->irq.irq) +#define timer_vm_data (ctx) (&(ctx)->vcpu->kvm->arch.timer_data) +#define timer_irq (ctx) (timer_vm_data(ctx)->ppi[arch_timer_ctx_index(ctx)]) u64 kvm_arm_timer_read_sysreg(struct kvm_vcpu *vcpu, enum kvm_arch_timers tmr,
Signature · 1 line
2.34.1
Re: [PATCH v3 11/18] KVM: arm64: timers: Move the timer IRQs into arch_timer_vm_data
Quoted 91 lines · Marc Zyngier
Having the timer IRQs duplicated into each vcpu isn't great, and becomes absolutely awful with NV. So let's move these into the per-VM
arch_timer_vm_datastructure.This simplifies a lot of code, but requires us to introduce a mutex so that we can reason about userspace trying to change an interrupt number while another vcpu is running, something that wasn't really well handled so far.
Reviewed-by Colton Lewis <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/kvm_host.h | 2 + arch/arm64/kvm/arch_timer.c | 104 +++++++++++++++++------------- arch/arm64/kvm/arm.c | 2 + include/kvm/arm_arch_timer.h | 18 ++++-- 4 files changed, 78 insertions(+), 48 deletions(-)arch/arm64/include/asm/kvm_host.h +2
-0@@ -223,6 +223,8 @@ struct kvm_arch {#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5/* VM counter offset */ #define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6 +/* Timer PPIs made immutable */ +#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 7unsigned long flags;arch/arm64/kvm/arch_timer.c +20
-15@@ -851,7 +851,6 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)hrtimer_init (&ctxt->hrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); ctxt->hrtimer.function = kvm_hrtimer_expire; -timer_irq (ctxt) = default_ppi[timerid];switch (timerid) {case TIMER_PTIMER :@@ -880,6 +879,13 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) timer->bg_timer.function = kvm_bg_timer_expire; } +void kvm_timer_init_vm (struct kvm *kvm) +{ +mutex_init (&kvm->arch.timer_data.lock); +for (int i =0 ; i <NR_KVM_TIMERS ; i++) + kvm->arch.timer_data.ppi[i] = default_ppi[i]; +} +void kvm_timer_cpu_up (void ) {enable_percpu_irq (host_vtimer_irq, host_vtimer_irq_flags);@@ -1292,44 +1298,52 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)static bool timer_irqs_are_valid (struct kvm_vcpu *vcpu) { -int vtimer_irq, ptimer_irq, ret; -unsigned long i; +u32 ppis =0 ; - vtimer_irq =timer_irq (vcpu_vtimer (vcpu)); - ret =kvm_vgic_set_owner (vcpu, vtimer_irq,vcpu_vtimer (vcpu)); -if (ret) -return false; +mutex_lock (&vcpu->kvm->arch.timer_data.lock); - ptimer_irq =timer_irq (vcpu_ptimer (vcpu)); - ret =kvm_vgic_set_owner (vcpu, ptimer_irq,vcpu_ptimer (vcpu)); -if (ret) -return false; +for (int i =0 ; i <NR_KVM_TIMERS ; i++) { +struct arch_timer_context *ctx; +int irq; -kvm_for_each_vcpu (i, vcpu, vcpu->kvm) { -if (timer_irq (vcpu_vtimer (vcpu)) != vtimer_irq || -timer_irq (vcpu_ptimer (vcpu)) != ptimer_irq) -return false; + ctx =vcpu_get_timer (vcpu, i); + irq =timer_irq (ctx); +if (kvm_vgic_set_owner (vcpu, irq, ctx)) +break ; + +/* +* We know by construction that we only have PPIs, so +* all values are less than 32.
arch/arm64/kvm/arch_timer.c void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu) · line 1325
Quoted 10 lines
arch/arm64/kvm/arch_timer.c +7
-1+*/ + ppis |=BIT (irq); } -return true; +set_bit (KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE , &vcpu->kvm->arch.flags); + +mutex_unlock (&vcpu->kvm->arch.timer_data.lock); + +return hweight32 (ppis) ==NR_KVM_TIMERS ;
Does it make sense to only set the IMMUTABLE flag if the timer IRQs are indeed valid? I doubt userspace would do anything when it gets the EINVAL, but it is possible userspace could make another attempt at configuring the IRQs correctly.
I believe that was the existing behavior of the UAPI.
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerRe: [PATCH v3 11/18] KVM: arm64: timers: Move the timer IRQs into arch_timer_vm_data
Quoted 97 lines · Oliver Upton
Quoted 95 lines · Marc Zyngier
Having the timer IRQs duplicated into each vcpu isn't great, and becomes absolutely awful with NV. So let's move these into the per-VM
arch_timer_vm_datastructure.This simplifies a lot of code, but requires us to introduce a mutex so that we can reason about userspace trying to change an interrupt number while another vcpu is running, something that wasn't really well handled so far.
Reviewed-by Colton Lewis <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/include/asm/kvm_host.h | 2 + arch/arm64/kvm/arch_timer.c | 104 +++++++++++++++++------------- arch/arm64/kvm/arm.c | 2 + include/kvm/arm_arch_timer.h | 18 ++++-- 4 files changed, 78 insertions(+), 48 deletions(-)arch/arm64/include/asm/kvm_host.h +2
-0@@ -223,6 +223,8 @@ struct kvm_arch {#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 5/* VM counter offset */ #define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 6 +/* Timer PPIs made immutable */ +#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 7unsigned long flags;arch/arm64/kvm/arch_timer.c +22
-15@@ -851,7 +851,6 @@ static void timer_context_init(struct kvm_vcpu *vcpu, int timerid)hrtimer_init (&ctxt->hrtimer,CLOCK_MONOTONIC ,HRTIMER_MODE_ABS_HARD ); ctxt->hrtimer.function = kvm_hrtimer_expire; -timer_irq (ctxt) = default_ppi[timerid];switch (timerid) {case TIMER_PTIMER :@@ -880,6 +879,13 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu) timer->bg_timer.function = kvm_bg_timer_expire; } +void kvm_timer_init_vm (struct kvm *kvm) +{ +mutex_init (&kvm->arch.timer_data.lock); +for (int i =0 ; i <NR_KVM_TIMERS ; i++) + kvm->arch.timer_data.ppi[i] = default_ppi[i]; +} +void kvm_timer_cpu_up (void ) {enable_percpu_irq (host_vtimer_irq, host_vtimer_irq_flags);@@ -1292,44 +1298,52 @@ void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu)static bool timer_irqs_are_valid (struct kvm_vcpu *vcpu) { -int vtimer_irq, ptimer_irq, ret; -unsigned long i; +u32 ppis =0 ; - vtimer_irq =timer_irq (vcpu_vtimer (vcpu)); - ret =kvm_vgic_set_owner (vcpu, vtimer_irq,vcpu_vtimer (vcpu)); -if (ret) -return false; +mutex_lock (&vcpu->kvm->arch.timer_data.lock); - ptimer_irq =timer_irq (vcpu_ptimer (vcpu)); - ret =kvm_vgic_set_owner (vcpu, ptimer_irq,vcpu_ptimer (vcpu)); -if (ret) -return false; +for (int i =0 ; i <NR_KVM_TIMERS ; i++) { +struct arch_timer_context *ctx; +int irq; -kvm_for_each_vcpu (i, vcpu, vcpu->kvm) { -if (timer_irq (vcpu_vtimer (vcpu)) != vtimer_irq || -timer_irq (vcpu_ptimer (vcpu)) != ptimer_irq) -return false; + ctx =vcpu_get_timer (vcpu, i); + irq =timer_irq (ctx); +if (kvm_vgic_set_owner (vcpu, irq, ctx)) +break ; + +/* +* We know by construction that we only have PPIs, so +* all values are less than 32. +*/ + ppis |=BIT (irq); }
Quoted 11 lines
arch/arm64/kvm/arch_timer.c void kvm_timer_vcpu_terminate(struct kvm_vcpu *vcpu) · line 1325
Quoted 6 lines
arch/arm64/kvm/arch_timer.c +5
-1-return true; +set_bit (KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE , &vcpu->kvm->arch.flags); + +mutex_unlock (&vcpu->kvm->arch.timer_data.lock); + +return hweight32 (ppis) ==NR_KVM_TIMERS ;Does it make sense to only set the IMMUTABLE flag if the timer IRQs are indeed valid? I doubt userspace would do anything when it gets the EINVAL, but it is possible userspace could make another attempt at configuring the IRQs correctly.
Yup, that's fair enough. I'll flip things around.
Thanks!
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v3 12/18 KVM: arm64: Abstract the number of valid timers per vcpu
In reply to Marc ZyngierWe so far have a pretty fixed number of timers to take care of. This is about to change as NV brings another two into the picture, and we must be careful not to try and emulate non-valid timers in a given VM.
For this, abstract the number of timers for a given vcpu behind an accessor, which helpfully returns a constant for now.
--- arch/arm64/kvm/arch_timer.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
arch/arm64/kvm/arch_timer.c +11 -6
@@ -52,6 +52,11 @@ static bool has_cntpoff(void)return (has_vhe () &&cpus_have_final_cap (ARM64_HAS_ECV_CNTPOFF )); } +static int nr_timers (struct kvm_vcpu *vcpu) +{ +return NR_KVM_TIMERS ; +} +u32 timer_get_ctl (struct arch_timer_context *ctxt) {struct kvm_vcpu *vcpu = ctxt->vcpu;
@@ -255,7 +260,7 @@ static u64 kvm_timer_earliest_exp(struct kvm_vcpu *vcpu)u64 min_delta =ULLONG_MAX ;int i; -for (i =0 ; i <NR_KVM_TIMERS ; i++) { +for (i =0 ; i <nr_timers (vcpu); i++) {struct arch_timer_context *ctx = &vcpu->arch.timer_cpu.timers[i];WARN (ctx->loaded,"timer %d loaded\n" , i);
@@ -815,12 +820,12 @@ int kvm_timer_vcpu_reset(struct kvm_vcpu *vcpu)* resets the timer to be disabled and unmasked and is compliant with * the ARMv7 architecture. */ -for (int i =0 ; i <NR_KVM_TIMERS ; i++) +for (int i =0 ; i <nr_timers (vcpu); i++)timer_set_ctl (vcpu_get_timer (vcpu, i),0 );if (timer->enabled) { -for (int i =0 ; i <NR_KVM_TIMERS ; i++) +for (int i =0 ; i <nr_timers (vcpu); i++)kvm_timer_update_irq (vcpu, false,vcpu_get_timer (vcpu, i));
@@ -1302,7 +1307,7 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)mutex_lock (&vcpu->kvm->arch.timer_data.lock); -for (int i =0 ; i <NR_KVM_TIMERS ; i++) { +for (int i =0 ; i <nr_timers (vcpu); i++) {struct arch_timer_context *ctx;int irq;
@@ -1322,7 +1327,7 @@ static bool timer_irqs_are_valid(struct kvm_vcpu *vcpu)mutex_unlock (&vcpu->kvm->arch.timer_data.lock); -return hweight32 (ppis) ==NR_KVM_TIMERS ; +return hweight32 (ppis) ==nr_timers (vcpu); }bool kvm_arch_timer_get_input_level (int vintid)
@@ -1332,7 +1337,7 @@ bool kvm_arch_timer_get_input_level(int vintid)if (WARN (!vcpu,"No vcpu context!\n" ))return false; -for (int i =0 ; i <NR_KVM_TIMERS ; i++) { +for (int i =0 ; i <nr_timers (vcpu); i++) {struct arch_timer_context *ctx; ctx =vcpu_get_timer (vcpu, i);
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 13/18 KVM: arm64: Document KVM_ARM_SET_CNT_OFFSETS and co
In reply to Marc ZyngierAdd some basic documentation on the effects of KVM_ARM_SET_CNT_OFFSETS.
--- Documentation/virt/kvm/api.rst | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
Documentation/virt/kvm/api.rst +38 -0
@@ -6029,6 +6029,44 @@ delivery must be provided via the "reg_aen" struct. The "pad" and "reserved" fields may be used for future extensions and should be set to 0s by userspace. +4.138 KVM_ARM_SET_COUNTER_OFFSET +-------------------------------- + +:Capability: KVM_CAP_COUNTER_OFFSET +:Architectures: arm64 +:Type: vm ioctl +:Parameters: struct kvm_arm_counter_offset (in) +:Returns: 0 on success, < 0 on error + +This capability indicates that userspace is able to apply a single VM-wide +offset to both the virtual and physical counters as viewed by the guest +using the KVM_ARM_SET_CNT_OFFSET ioctl and the following data structure: + +:: + + struct kvm_arm_counter_offset { + __u64 counter_offset; + __u64 reserved; + }; + +The offset describes a number of counter cycles that are subtracted from +both virtual and physical counter views (similar to the effects of the +CNTVOFF_EL2 and CNTPOFF_EL2 system registers, but only global). The offset +always applies to all vcpus (already created or created after this ioctl) +for this VM. + +It is userspace's responsibility to compute the offset based, for example, +on previous values of the guest counters. + +Any value other than 0 for the "reserved" field may result in an error +(-EINVAL) being returned. This ioctl can also return -EBUSY if any vcpu +ioctl is issued concurrently. + +Note that using this ioctl results in KVM ignoring subsequent userspace +writes to the CNTVCT_EL0 and CNTPCT_EL0 registers using the SET_ONE_REG +interface. No error will be returned, but the resulting offset will not be +applied. + 5. The kvm_run structure ========================
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 14/18 KVM: arm64: nv: timers: Add a per-timer, per-vcpu offset
In reply to Marc ZyngierBeing able to set a global offset isn't enough.
With NV, we also need to a per-vcpu, per-timer offset (for example, CNTVCT_EL0 being offset by CNTVOFF_EL2).
Use a similar method as the VM-wide offset to have a timer point to the shadow register that contains the offset value.
--- arch/arm64/kvm/arch_timer.c | 13 ++++++++++--- include/kvm/arm_arch_timer.h | 5 +++++ 2 files changed, 15 insertions(+), 3 deletions(-)
arch/arm64/kvm/arch_timer.c +10 -3
@@ -89,10 +89,17 @@ u64 timer_get_cval(struct arch_timer_context *ctxt)static u64 timer_get_offset (struct arch_timer_context *ctxt) { -if (ctxt && ctxt->offset.vm_offset) -return *ctxt->offset.vm_offset; +u64 offset =0 ; -return 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)
include/kvm/arm_arch_timer.h +5 -0
@@ -29,6 +29,11 @@ struct arch_timer_offset {* structure. If NULL, assume a zero offset. */ u64 *vm_offset; +/* +* If set, pointer to one of the offsets in the vcpu's sysreg +* array. If NULL, assume a zero offset. +*/ +u64 *vcpu_offset; };struct arch_timer_vm_data {
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 15/18 KVM: arm64: nv: timers: Support hyp timer emulation
In reply to Marc ZyngierEmulating EL2 also means emulating the EL2 timers. To do so, we expand our timer framework to deal with at most 4 timers. At any given time, two timers are using the HW timers, and the two others are purely emulated.
The role of deciding which is which at any given time is left to a mapping function which is called every time we need to make such a decision.
--- arch/arm64/include/asm/kvm_host.h | 4 + arch/arm64/include/uapi/asm/kvm.h | 2 + arch/arm64/kvm/arch_timer.c | 180 ++++++++++++++++++++++++++++-- arch/arm64/kvm/trace_arm.h | 6 +- arch/arm64/kvm/vgic/vgic.c | 15 +++ include/kvm/arm_arch_timer.h | 9 +- include/kvm/arm_vgic.h | 1 + 7 files changed, 205 insertions(+), 12 deletions(-)
arch/arm64/include/asm/kvm_host.h +4 -0
@@ -369,6 +369,10 @@ enum vcpu_sysreg {TPIDR_EL2 ,/* EL2 Software Thread ID Register */ CNTHCTL_EL2 ,/* Counter-timer Hypervisor Control register */ SP_EL2 ,/* EL2 Stack Pointer */ +CNTHP_CTL_EL2 , +CNTHP_CVAL_EL2 , +CNTHV_CTL_EL2 , +CNTHV_CVAL_EL2 , NR_SYS_REGS /* Nothing after this line! */ };
arch/arm64/include/uapi/asm/kvm.h +2 -0
@@ -420,6 +420,8 @@ enum {#define KVM_ARM_VCPU_TIMER_CTRL 1#define KVM_ARM_VCPU_TIMER_IRQ_VTIMER 0#define KVM_ARM_VCPU_TIMER_IRQ_PTIMER 1 +#define KVM_ARM_VCPU_TIMER_IRQ_HVTIMER 2 +#define KVM_ARM_VCPU_TIMER_IRQ_HPTIMER 3#define KVM_ARM_VCPU_PVTIME_CTRL 2#define KVM_ARM_VCPU_PVTIME_IPA 0
arch/arm64/kvm/arch_timer.c +171 -9
391 lines not on this page · raw message
arch/arm64/kvm/trace_arm.h +5 -1
@@ -206,6 +206,7 @@ TRACE_EVENT(kvm_get_timer_map, __field( unsigned long, vcpu_id ) __field( int, direct_vtimer ) __field( int, direct_ptimer ) + __field( int, emul_vtimer ) __field( int, emul_ptimer ) ),
@@ -214,14 +215,17 @@ TRACE_EVENT(kvm_get_timer_map, __entry->direct_vtimer =arch_timer_ctx_index (map->direct_vtimer); __entry->direct_ptimer = (map->direct_ptimer) ?arch_timer_ctx_index (map->direct_ptimer) :-1 ; + __entry->emul_vtimer = + (map->emul_vtimer) ?arch_timer_ctx_index (map->emul_vtimer) :-1 ; __entry->emul_ptimer = (map->emul_ptimer) ?arch_timer_ctx_index (map->emul_ptimer) :-1 ; ), -TP_printk ("VCPU: %ld, dv: %d, dp: %d, ep: %d" , +TP_printk ("VCPU: %ld, dv: %d, dp: %d, ev: %d, ep: %d" , __entry->vcpu_id, __entry->direct_vtimer, __entry->direct_ptimer, + __entry->emul_vtimer, __entry->emul_ptimer) );
arch/arm64/kvm/vgic/vgic.c +15 -0
@@ -573,6 +573,21 @@ int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int vintid)return 0 ; } +int kvm_vgic_get_map (struct kvm_vcpu *vcpu,unsigned int vintid) +{ +struct vgic_irq *irq =vgic_get_irq (vcpu->kvm, vcpu, vintid); +unsigned long flags; +int ret =-1 ; + +raw_spin_lock_irqsave (&irq->irq_lock, flags); +if (irq->hw) + ret = irq->hwintid; +raw_spin_unlock_irqrestore (&irq->irq_lock, flags); + +vgic_put_irq (vcpu->kvm, irq); +return ret; +} +/** * kvm_vgic_set_owner - Set the owner of an interrupt for a VM *
include/kvm/arm_arch_timer.h +7 -2
@@ -13,6 +13,9 @@enum kvm_arch_timers {TIMER_PTIMER ,TIMER_VTIMER , +NR_KVM_EL0_TIMERS , +TIMER_HVTIMER =NR_KVM_EL0_TIMERS , +TIMER_HPTIMER ,NR_KVM_TIMERS };
@@ -21,6 +24,7 @@ enum kvm_arch_timer_regs {TIMER_REG_CVAL ,TIMER_REG_TVAL ,TIMER_REG_CTL , + TIMER_REG_VOFF, }; struct arch_timer_offset {
@@ -76,6 +80,7 @@ struct arch_timer_context {struct timer_map {struct arch_timer_context *direct_vtimer;struct arch_timer_context *direct_ptimer; +struct arch_timer_context *emul_vtimer;struct arch_timer_context *emul_ptimer; };
@@ -114,12 +119,12 @@ void kvm_timer_vcpu_put(struct kvm_vcpu *vcpu);void kvm_timer_init_vhe (void ); -bool kvm_arch_timer_get_input_level (int vintid); -#define vcpu_timer (v) (&(v)->arch.timer_cpu)#define vcpu_get_timer (v,t) (&vcpu_timer(v)->timers[(t)])#define vcpu_vtimer (v) (&(v)->arch.timer_cpu.timers[TIMER_VTIMER])#define vcpu_ptimer (v) (&(v)->arch.timer_cpu.timers[TIMER_PTIMER]) +#define vcpu_hvtimer (v) (&(v)->arch.timer_cpu.timers[TIMER_HVTIMER]) +#define vcpu_hptimer (v) (&(v)->arch.timer_cpu.timers[TIMER_HPTIMER])#define arch_timer_ctx_index (ctx) ((ctx) - vcpu_timer((ctx)->vcpu)->timers)
include/kvm/arm_vgic.h +1 -0
@@ -380,6 +380,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid,int kvm_vgic_map_phys_irq (struct kvm_vcpu *vcpu,unsigned int host_irq,u32 vintid,struct irq_ops *ops);int kvm_vgic_unmap_phys_irq (struct kvm_vcpu *vcpu,unsigned int vintid); +int kvm_vgic_get_map (struct kvm_vcpu *vcpu,unsigned int vintid);bool kvm_vgic_map_is_active (struct kvm_vcpu *vcpu,unsigned int vintid);int kvm_vgic_vcpu_pending_irq (struct kvm_vcpu *vcpu);
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 16/18 KVM: arm64: selftests: Add physical timer registers to the sysreg list
In reply to Marc ZyngierNow that KVM exposes CNTPCT_EL0, CNTP_CTL_EL0 and CNT_CVAL_EL0 to userspace, add them to the get-reg-list selftest.
--- tools/testing/selftests/kvm/aarch64/get-reg-list.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/get-reg-list.c +4 -1
@@ -651,7 +651,7 @@ int main(int ac, char **av)* The current blessed list was primed with the output of kernel version * v4.15 with --core-reg-fixup and then later updated with new registers. * -* The blessed list is up to date with kernel version v5.13-rc3 +* The blessed list is up to date with kernel version v6.4 (or so we hope) */ static __u64 base_regs[] = {KVM_REG_ARM64 |KVM_REG_SIZE_U64 |KVM_REG_ARM_CORE |KVM_REG_ARM_CORE_REG (regs.regs[0 ]),
@@ -858,6 +858,9 @@ static __u64 base_regs[] = {ARM64_SYS_REG (3 ,2 ,0 ,0 ,0 ),/* CSSELR_EL1 */ ARM64_SYS_REG (3 ,3 ,13 ,0 ,2 ),/* TPIDR_EL0 */ ARM64_SYS_REG (3 ,3 ,13 ,0 ,3 ),/* TPIDRRO_EL0 */ +ARM64_SYS_REG (3 ,3 ,14 ,0 ,1 ),/* CNTPCT_EL0 */ +ARM64_SYS_REG (3 ,3 ,14 ,2 ,1 ),/* CNTP_CTL_EL0 */ +ARM64_SYS_REG (3 ,3 ,14 ,2 ,2 ),/* CNTP_CVAL_EL0 */ ARM64_SYS_REG (3 ,4 ,3 ,0 ,0 ),/* DACR32_EL2 */ ARM64_SYS_REG (3 ,4 ,5 ,0 ,1 ),/* IFSR32_EL2 */ ARM64_SYS_REG(3, 4, 5, 3, 0), /* FPEXC32_EL2 */
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 17/18 KVM: arm64: selftests: Deal with spurious timer interrupts
In reply to Marc ZyngierMake sure the timer test can properly handle a spurious timer interrupt, something that is far from being unlikely.
This involves checking for the GIC IAR return value (don't bother handling the interrupt if it was spurious) as well as the timer control register (don't do anything if the interrupt is masked or the timer disabled). Take this opportunity to rewrite the timer handler in a more readable way.
This solves a bunch of failures that creep up on systems that are slow to retire the interrupt, something that the GIC architecture makes no guarantee about.
--- .../selftests/kvm/aarch64/arch_timer.c | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +25 -15
@@ -121,25 +121,35 @@ static void guest_validate_irq(unsigned int intid,uint64_t xcnt =0 , xcnt_diff_us, cval =0 ;unsigned long xctl =0 ;unsigned int timer_irq =0 ; +unsigned int accessor; -if (stage ==GUEST_STAGE_VTIMER_CVAL || - stage ==GUEST_STAGE_VTIMER_TVAL ) { - xctl =timer_get_ctl (VIRTUAL ); -timer_set_ctl (VIRTUAL ,CTL_IMASK ); - xcnt =timer_get_cntct (VIRTUAL ); - cval =timer_get_cval (VIRTUAL ); +if (intid ==IAR_SPURIOUS ) +return ; + +switch (stage) { +case GUEST_STAGE_VTIMER_CVAL : +case GUEST_STAGE_VTIMER_TVAL : + accessor =VIRTUAL ; timer_irq = vtimer_irq; - }else if (stage ==GUEST_STAGE_PTIMER_CVAL || - stage ==GUEST_STAGE_PTIMER_TVAL ) { - xctl =timer_get_ctl (PHYSICAL ); -timer_set_ctl (PHYSICAL ,CTL_IMASK ); - xcnt =timer_get_cntct (PHYSICAL ); - cval =timer_get_cval (PHYSICAL ); +break ; +case GUEST_STAGE_PTIMER_CVAL : +case GUEST_STAGE_PTIMER_TVAL : + accessor =PHYSICAL ; timer_irq = ptimer_irq; - }else { +break ; +default :GUEST_ASSERT (0 ); +return ; } + xctl =timer_get_ctl (accessor); +if ((xctl &CTL_IMASK ) || !(xctl &CTL_ENABLE )) +return ; + +timer_set_ctl (accessor,CTL_IMASK ); + xcnt =timer_get_cntct (accessor); + cval =timer_get_cval (accessor); + xcnt_diff_us =cycles_to_usec (xcnt - shared_data->xcnt);/* Make sure we are dealing with the correct timer IRQ */
@@ -148,6 +158,8 @@ static void guest_validate_irq(unsigned int intid,/* Basic 'timer condition met' check */ GUEST_ASSERT_3 (xcnt >= cval, xcnt, cval, xcnt_diff_us);GUEST_ASSERT_1 (xctl &CTL_ISTATUS , xctl); + +WRITE_ONCE (shared_data->nr_iter, shared_data->nr_iter +1 ); }static void guest_irq_handler (struct ex_regs *regs)
@@ -158,8 +170,6 @@ static void guest_irq_handler(struct ex_regs *regs)guest_validate_irq (intid, shared_data); -WRITE_ONCE (shared_data->nr_iter, shared_data->nr_iter +1 ); -gic_set_eoi (intid); }
Signature · 1 line
2.34.1
Marc Zyngier
maintainerPATCH v3 18/18 KVM: arm64: selftests: Augment existing timer test to handle variable offset
In reply to Marc ZyngierAllow a user to specify the global offset on the command-line.
--- tools/testing/selftests/kvm/aarch64/arch_timer.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)
tools/testing/selftests/kvm/aarch64/arch_timer.c +15 -1
@@ -47,6 +47,7 @@ struct test_args {int nr_iter;int timer_period_ms;int migration_freq_ms; +struct kvm_arm_counter_offset offset; };static struct test_args test_args = {
@@ -54,6 +55,7 @@ static struct test_args test_args = { .nr_iter = NR_TEST_ITERS_DEF, .timer_period_ms = TIMER_TEST_PERIOD_MS_DEF, .migration_freq_ms = TIMER_TEST_MIGRATION_FREQ_MS, + .offset = { .reserved = 1 }, };#define msecs_to_usecs (msec) ((msec) * 1000LL)
@@ -382,6 +384,13 @@ static struct kvm_vm *test_vm_create(void)vm_init_descriptor_tables (vm);vm_install_exception_handler (vm,VECTOR_IRQ_CURRENT , guest_irq_handler); +if (!test_args.offset.reserved) { +if (kvm_has_cap (KVM_CAP_COUNTER_OFFSET )) +vm_ioctl (vm,KVM_ARM_SET_COUNTER_OFFSET , &test_args.offset); +else +TEST_FAIL ("no support for global offset\n" ); + } +for (i =0 ; i < nr_vcpus; i++)vcpu_init_descriptor_tables (vcpus[i]);
@@ -413,6 +422,7 @@ static void test_print_help(char *name) TIMER_TEST_PERIOD_MS_DEF);pr_info ("\t-m: Frequency (in ms) of vCPUs to migrate to different pCPU. 0 to turn off (default: %u)\n" ,TIMER_TEST_MIGRATION_FREQ_MS ); +pr_info ("\t-o: Counter offset (in counter cycles, default: 0)\n" );pr_info ("\t-h: print this help screen\n" ); }
@@ -420,7 +430,7 @@ static bool parse_args(int argc, char *argv[]) {int opt; -while ((opt =getopt (argc, argv,"hn:i:p:m:" )) !=-1 ) { +while ((opt =getopt (argc, argv,"hn:i:p:m:o:" )) !=-1 ) {switch (opt) {case 'n' : test_args.nr_vcpus =atoi_positive ("Number of vCPUs" , optarg);
@@ -439,6 +449,10 @@ static bool parse_args(int argc, char *argv[])case 'm' : test_args.migration_freq_ms =atoi_non_negative ("Frequency" , optarg);break ; +case 'o' : + test_args.offset.counter_offset =strtol (optarg,NULL ,0 ); + test_args.offset.reserved =0 ; +break ;case 'h' :default : goto err;
Signature · 1 line
2.34.1
Hello Marc,
thanks again for your proposal.
Quoted 35 lines · Marc Zyngier
This series aims at satisfying multiple goals:
- allow a VMM to atomically restore a timer offset for a whole VM instead of updating the offset each time a vcpu get its counter written
- allow a VMM to save/restore the physical timer context, something that we cannot do at the moment due to the lack of offsetting
- provide a framework that is suitable for NV support, where we get both global and per timer, per vcpu offsetting, and manage interrupts in a less braindead way.
We fix a couple of issues along the way, both from a stylistic and correctness perspective. This results in a new per VM KVM API that allows a global offset to be set at any point in time, overriding both
of the timer counter writebacks.We also take this opportunity to rework the way IRQs are associated with timers, something that was always a bit dodgy. This relies on a new lock, which should disappear once Oliver's lock ordering series is
merged (we can reuse theconfig_lockfor this).This has been tested with nVHE, VHE and NV. I do not have access to CNTPOFF-aware HW, but Colton managed to give it a go. Note that the NV patches in this series are here to give a perspective on how this gets used.
I've updated the
arch_timerselftest to allow an offset to be provided
from the command line, and fixed a couple of glaring issues along the way.
Quoted 7 lines
Note that this is at best 6.4 material. I have a branch stashed at
[0]
and based on 6.3-rc3, as well as a minimal example of the use of the API at [3] based on kvmtool.Simon: I'd appreciate some feedback as whether this change fits your requirements, given that you brought this up the first place.
The interface looks good to me. I have yet to deliver on my promise to test your patch series with our userspace; I am on leave this week, but I'll give your latest iteration a go next week.
Thanks
Simon
Quoted 91 lines
Thanks,
M.
* From v2 [2]:
- Fixed 32bit handling of the physical counter when the offset is non-zero
- Dropped unused -O option from the selftest
- Added
lockdep_assert_held()to (un)lock_all_vcpus()- Reordered the last two patches
- Added Colton's RBs, with thanks
- Dropped the initial patch which has been merged
- Rebased on 6.3-rc3
* From v1 [1]:
- Switched from a dual offset to a single one which gets applied to both virtual and physical counters. Which means that NV doesn't behave oddly anymore by ignoring the virtual offset.
- Some cosmetic repainting of the UAPI symbols
- Added patches to rework the IRQ mapping to timers
- Patch #1 on its way to Paolo
- Rebased on 6.3-rc1
[0]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=kvm-arm64/timer-vm-offsets
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3]
https://git.kernel.org/pub/scm/linux/kernel/git/maz/kvmtool.git/commit/?h=zero-offset&id=3b1253073ee57c0d92baf7b214362829b487b8d5Marc Zyngier (18):
KVM: arm64: timers: Use a per-vcpu, per-timer accumulator for fractional ns
arm64: AddCNTPOFF_EL2register definition
arm64: AddHAS_ECV_CNTPOFFcapability
KVM: arm64: timers: UseCNTPOFF_EL2to offset the physical timer
KVM: arm64: timers: Allow physical offset withoutCNTPOFF_EL2
KVM: arm64: Expose {un,}lock_all_vcpus() to the rest of KVM
KVM: arm64: timers: Allow userspace to set the global counter offset
KVM: arm64: timers: Allow save/restoring of the physical timer
KVM: arm64: timers: Rationalise per-vcpu timer init
KVM: arm64: timers: Abstract per-timer IRQ access
KVM: arm64: timers: Move the timer IRQs intoarch_timer_vm_data
KVM: arm64: Abstract the number of valid timers per vcpu
KVM: arm64: DocumentKVM_ARM_SET_CNT_OFFSETSand co
KVM: arm64: nv: timers: Add a per-timer, per-vcpu offset
KVM: arm64: nv: timers: Support hyp timer emulation
KVM: arm64: selftests: Add physical timer registers to the sysreg list
KVM: arm64: selftests: Deal with spurious timer interrupts
KVM: arm64: selftests: Augment existing timer test to handle variable
offsetDocumentation/virt/kvm/api.rst | 38 ++ arch/arm64/include/asm/kvm_host.h | 13 + arch/arm64/include/asm/sysreg.h | 2 + arch/arm64/include/uapi/asm/kvm.h | 11 + arch/arm64/kernel/cpufeature.c | 11 + arch/arm64/kvm/arch_timer.c | 539 ++++++++++++++-- --
arch/arm64/kvm/arm.c | 53 ++ arch/arm64/kvm/guest.c | 29 +- arch/arm64/kvm/hyp/nvhe/timer-sr.c | 18 +- arch/arm64/kvm/hypercalls.c | 2 +- arch/arm64/kvm/sys_regs.c | 9 + arch/arm64/kvm/trace_arm.h | 6 +- arch/arm64/kvm/vgic/vgic-kvm-device.c | 38 -- arch/arm64/kvm/vgic/vgic.c | 15 + arch/arm64/kvm/vgic/vgic.h | 3 - arch/arm64/tools/cpucaps | 1 + arch/arm64/tools/sysreg | 4 + include/clocksource/arm_arch_timer.h | 1 + include/kvm/arm_arch_timer.h | 36 +- include/kvm/arm_vgic.h | 1 + include/uapi/linux/kvm.h | 3 + .../selftests/kvm/aarch64/arch_timer.c | 56 +- .../selftests/kvm/aarch64/get-reg-list.c | 5 +-
Quoted 5 lines
23 files changed, 690 insertions(+), 204 deletions(-)--
2.34.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Marc Zyngier
maintainerQuoted 46 lines · Simon
Hello Marc,
thanks again for your proposal.
Quoted 40 lines · Marc Zyngier
This series aims at satisfying multiple goals:
- allow a VMM to atomically restore a timer offset for a whole VM instead of updating the offset each time a vcpu get its counter written
- allow a VMM to save/restore the physical timer context, something that we cannot do at the moment due to the lack of offsetting
- provide a framework that is suitable for NV support, where we get both global and per timer, per vcpu offsetting, and manage interrupts in a less braindead way.
We fix a couple of issues along the way, both from a stylistic and correctness perspective. This results in a new per VM KVM API that allows a global offset to be set at any point in time, overriding both
of the timer counter writebacks.We also take this opportunity to rework the way IRQs are associated with timers, something that was always a bit dodgy. This relies on a new lock, which should disappear once Oliver's lock ordering series is
merged (we can reuse theconfig_lockfor this).This has been tested with nVHE, VHE and NV. I do not have access to CNTPOFF-aware HW, but Colton managed to give it a go. Note that the NV patches in this series are here to give a perspective on how this gets used.
I've updated the
arch_timerselftest to allow an offset to be provided
from the command line, and fixed a couple of glaring issues along the way.Note that this is at best 6.4 material. I have a branch stashed at
[0]
and based on 6.3-rc3, as well as a minimal example of the use of the API at [3] based on kvmtool.
Quoted 6 lines
Quoted 2 lines
Simon: I'd appreciate some feedback as whether this change fits your requirements, given that you brought this up the first place.
The interface looks good to me. I have yet to deliver on my promise to test your patch series with our userspace; I am on leave this week, but I'll give your latest iteration a go next week.
No worries. I'm about to post v4, which I think is pretty ripe at this stage. Do let me know when you get a chance to try it.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.