Re: [PATCH 1/4] KVM: x86: Reset last_steal on vCPU pid change

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch addresses the steal time corruption on vCPU PID change for x86 KVM PV but completely misses the equivalent KVM Xen steal time (`vcpu->arch.xen.last_steal`), leaving it exposed to the identical integer underflow.
- [High] The identical vCPU thread migration bug exists in ARM64, RISC-V, and LoongArch steal time accounting, but the patch only implements the fix for x86.
--

--- Patch [1]: [PATCH 1/4] KVM: x86: Reset last_steal on vCPU pid change ---
commit d7367ba78784ba11d9b540abce98359a8e902828
Author: Dongli Zhang <dongli.zhang@oracle.com>

KVM: x86: Reset last_steal on vCPU pid change

KVM does not reset last_steal on vCPU re-creation/hotplug, causing a bogus
large steal time due to run_delay starting from zero. This patch fixes it by
resetting vcpu->arch.st.last_steal in kvm_arch_vcpu_run_pid_change on x86.

> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 801bf9e520db3..b7cb2ceda6d9a 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -40,6 +40,7 @@ config KVM_X86
>  	select HAVE_KVM_MSI
>  	select HAVE_KVM_CPU_RELAX_INTERCEPT
>  	select HAVE_KVM_NO_POLL
> +	select HAVE_KVM_VCPU_RUN_PID_CHANGE

[Severity: High]
This is a pre-existing issue, but does this same integer underflow bug exist
on other architectures?

Looking at ARM64, RISC-V, and LoongArch, they all track steal time using
current->sched_info.run_delay and subtract the old last_steal value, which
appears identically vulnerable to underflow on vCPU thread recreation.

For example, on ARM64:

arch/arm64/kvm/pvtime.c:kvm_update_stolen_time() {
    ...
    steal += vcpu->arch.steal.last_steal - last_steal;
    ...
}

(Note that arm64's kvm_arch_vcpu_run_pid_change() returns early if
vcpu_has_run_once(vcpu) is true, skipping the opportunity to update last_steal.)

Are there plans to address this on the other architectures?

>  	select VIRT_XFER_TO_GUEST_WORK
>  	select KVM_GENERIC_DIRTYLOG_READ_PROTECT
>  	select KVM_VFIO
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 47cb9eba113b1..33be45eec32bc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -3717,6 +3717,13 @@ void kvm_service_local_tlb_flush_requests(struct kvm_vcpu *vcpu)
>  }
>  EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_service_local_tlb_flush_requests);
>  
> +int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> +{
> +	vcpu->arch.st.last_steal = current->sched_info.run_delay;
> +

[Severity: High]
This isn't a bug introduced by this patch, but should this function also
reset vcpu->arch.xen.last_steal?

Looking at kvm_xen_update_runstate() in arch/x86/kvm/xen.c, the stolen time
for Xen runstate is calculated similarly:

arch/x86/kvm/xen.c:kvm_xen_update_runstate() {
    ...
    u64 steal_ns = run_delay - vx->last_steal;
    delta_ns -= steal_ns;
    vx->runstate_times[RUNSTATE_runnable] += steal_ns;
    ...
    vx->last_steal = run_delay;
}

If the thread changes and run_delay resets to 0, wouldn't vx->last_steal
also cause an identical integer underflow?

> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260816053630.527528-1-dongli.zhang@oracle.com?part=1