Re: [PATCH 2/4] KVM: arm64: Reset last_steal on vCPU pid change
Dongli Zhang <[email protected]>
| Newsgroups | dev.linux.lists.kvmarm,org.kernel.vger.kvm,org.kernel.vger.linux-kselftest |
|---|---|
| Message-ID | <[email protected]> |
On Mon, Aug 17, 2026 1:42:20AM -0700, Marc Zyngier wrote: > On Sun, 16 Aug 2026 06:33:03 +0100, > Dongli Zhang <[email protected]> wrote: >> >> The previous commit resets x86 steal time accounting when the vCPU PID is >> changed. Do the same for arm64. > > Drop this statement, it really doesn't provide any information. Sure. > >> >> KVM keeps a vCPU fd alive when userspace hot-unplugs a vCPU. If the fd is >> later reused from a new vCPU thread, vcpu->arch.steal.last_steal still >> reflects the old task's run_delay, while current->sched_info.run_delay >> belongs to the new task. >> >> Reset vcpu->arch.steal.last_steal from kvm_arch_vcpu_run_pid_change() >> unconditionally so the next stolen time update computes its delta against >> the new task's run_delay. >> >> Assisted-by: Codex:GPT-5.5 >> Signed-off-by: Dongli Zhang <[email protected]> >> --- >> arch/arm64/include/asm/kvm_host.h | 1 + >> arch/arm64/kvm/arm.c | 2 ++ >> arch/arm64/kvm/pvtime.c | 5 +++++ >> 3 files changed, 8 insertions(+) >> >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h >> index bae2c4f92ef5..4607f956e787 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -1339,6 +1339,7 @@ static inline bool kvm_arch_pmi_in_guest(struct kvm_vcpu *vcpu) >> long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu); >> gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu); >> void kvm_update_stolen_time(struct kvm_vcpu *vcpu); >> +void kvm_reset_stolen_time(struct kvm_vcpu *vcpu); >> >> bool kvm_arm_pvtime_supported(void); >> int kvm_arm_pvtime_set_attr(struct kvm_vcpu *vcpu, >> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c >> index 9a6c72a18672..0f6e63eace21 100644 >> --- a/arch/arm64/kvm/arm.c >> +++ b/arch/arm64/kvm/arm.c >> @@ -929,6 +929,8 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu) >> if (!kvm_arm_vcpu_is_finalized(vcpu)) >> return -EPERM; >> >> + kvm_reset_stolen_time(vcpu); >> + >> if (likely(vcpu_has_run_once(vcpu))) >> return 0; >> >> diff --git a/arch/arm64/kvm/pvtime.c b/arch/arm64/kvm/pvtime.c >> index 4ceabaa4c30b..000bf49cc0fd 100644 >> --- a/arch/arm64/kvm/pvtime.c >> +++ b/arch/arm64/kvm/pvtime.c >> @@ -32,6 +32,11 @@ void kvm_update_stolen_time(struct kvm_vcpu *vcpu) >> srcu_read_unlock(&kvm->srcu, idx); >> } >> >> +void kvm_reset_stolen_time(struct kvm_vcpu *vcpu) >> +{ >> + vcpu->arch.steal.last_steal = current->sched_info.run_delay; >> +} >> + >> long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu) >> { >> u32 feature = smccc_get_arg1(vcpu); > > Why isn't this common code? I really don't see the point in making > this arch-specific code. I'd expect something like this (untested): > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index ace5801a592f..3fb77360af01 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -950,6 +950,8 @@ struct kvm_vcpu_arch { > pid_t pid; > }; > > +#define kvm_arch_vcpu_last_steal(v) (v)->arch.steal.last_steal > + > /* > * Each 'flag' is composed of a comma-separated triplet: > * > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 45e784462ec6..117aeb49231a 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -4459,6 +4459,9 @@ static long kvm_vcpu_ioctl(struct file *filp, > if (r) > break; > > + if (IS_ENABLED(CONFIG_HAVE_PV_STEAL_CLOCK_GEN)) > + kvm_arch_vcpu_last_steal(vcpu) = current->sched_info.run_delay; > + > newpid = get_task_pid(current, PIDTYPE_PID); > write_lock(&vcpu->pid_lock); > vcpu->pid = newpid; > > where each architecture that implements steal time provides an > accessor, and the core code is in charge of the adjustment. > > It also makes sure that we don't leave any architecture behind. > Or how about making it something like below? if (IS_ENABLED(CONFIG_HAVE_PV_STEAL_CLOCK_GEN)) kvm_arch_vcpu_reset_last_steal(vcpu); That would still move the policy to common KVM code, while leaving the exact arch state to the architecture implementation. For x86, the hook can reset vcpu->arch.st.last_steal for regular KVM steal time. If we also decide to cover Xen runstate in this series, the same x86 hook can additionally reset vcpu->arch.xen.last_steal. Thank you very much! Dongli Zhang