From: Dongli Zhang <dongli.zhang@oracle.com>
To: sashiko-reviews@lists.linux.dev
Cc: Oliver Upton <oupton@kernel.org>, Marc Zyngier <maz@kernel.org>,
kvm@vger.kernel.org, kvmarm@lists.linux.dev
Subject: Re: [PATCH 1/4] KVM: x86: Reset last_steal on vCPU pid change
Date: Mon, 17 Aug 2026 13:21:36 -0700 [thread overview]
Message-ID: <427a5d9c-7641-48c5-8a51-2f78688de4f0@oracle.com> (raw)
In-Reply-To: <20260816055525.A41D61F000E9@smtp.kernel.org>
On Sat, Aug 15, 2026 10:55:25PM -0700, sashiko-bot@kernel.org wrote:
> 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
>
>
> 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?
Patch 2 addresses the arm64 case.
risc-v and loongarch may need similar handling, but I didn't include them in
this series because I only have x86 and arm64 KVM test environments available to
validate the fix. I don't currently have a risc-v or loongarch setup for KVM
testing.
>
>> 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?
>
As explained in the cover letter, "Although David also suggested doing the same
for Xen-on-KVM vCPUs, this series does not reset last_steal for Xen vCPUs. That
change itself would not be difficult, but Xen uses a different mechanism to
account downtime, including runnable time and offline time when a vCPU is not
running. It may therefore need no additional ioctl, or a smaller ioctl
extension, to account run_delay left over from the previous PID. For now, this
series changes only regular x86 steal time and arm64 PV time."
Dongli Zhang
next prev parent reply other threads:[~2026-08-17 20:21 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-16 5:33 [PATCH 0/4] KVM: Reset steal time accounting on vCPU pid change (x86 and arm64) Dongli Zhang
2026-08-16 5:33 ` [PATCH 1/4] KVM: x86: Reset last_steal on vCPU pid change Dongli Zhang
2026-08-16 5:55 ` sashiko-bot
2026-08-17 20:21 ` Dongli Zhang [this message]
2026-08-16 5:33 ` [PATCH 2/4] KVM: arm64: " Dongli Zhang
2026-08-17 8:42 ` Marc Zyngier
2026-08-17 21:29 ` Dongli Zhang
2026-08-18 13:24 ` Marc Zyngier
2026-08-16 5:33 ` [PATCH 3/4] KVM: selftests: Test steal time across vCPU pid changes on x86 Dongli Zhang
2026-08-16 5:55 ` sashiko-bot
2026-08-17 20:28 ` Dongli Zhang
2026-08-17 8:15 ` [PATCH 0/4] KVM: Reset steal time accounting on vCPU pid change (x86 and arm64) Marc Zyngier
2026-08-17 21:05 ` Dongli Zhang
2026-08-17 11:51 ` David Woodhouse
2026-08-17 22:04 ` Dongli Zhang
2026-08-17 22:23 ` David Woodhouse
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=427a5d9c-7641-48c5-8a51-2f78688de4f0@oracle.com \
--to=dongli.zhang@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.linux.dev \
--cc=maz@kernel.org \
--cc=oupton@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox