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: 17+ 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-16 5:33 ` [PATCH 4/4] KVM: selftests: Add arm64 coverage for steal time pid changes 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.