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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.