Kernel KVM virtualization development
 help / color / mirror / Atom feed
* [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
@ 2026-08-24 13:18 Shivansh Dhiman
  2026-08-24 13:51 ` sashiko-bot
  2026-10-05 18:16 ` Shivansh Dhiman
  0 siblings, 2 replies; 6+ messages in thread
From: Shivansh Dhiman @ 2026-08-24 13:18 UTC (permalink / raw)
  To: seanjc, pbonzini, tglx, mingo
  Cc: kvm, x86, thomas.lendacky, nikunj.dadhania, santosh.shukla,
	shivansh.dhiman

The AMD APM previously stated that after an intercepted shutdown, the
entire VMCB state is undefined. In practice, only the save area is
undefined and most of the control area remains valid. Revision 3.45 of
the APM now makes this explicit:

  "After an intercepted shutdown, the VMCB control area is valid (with
   the exception of offsets 60h, 61h, and 68h) and the VMCB state save
   area is undefined."

KVM zeroes the entire VMCB before INITing the vCPU based on the old
wording, discarding control area state that hardware preserves.

Clear only the save area and the three undefined control area fields
(int_ctl[15:0] and int_state) in line with the updated APM.

Signed-off-by: Shivansh Dhiman <shivansh.dhiman@amd.com>
---
 arch/x86/kvm/svm/svm.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 9d607b98bd06..cb5e04fd518f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
 
 
 	/*
-	 * VMCB is undefined after a SHUTDOWN intercept.  INIT the vCPU to put
-	 * the VMCB in a known good state.  Unfortuately, KVM doesn't have
-	 * KVM_MP_STATE_SHUTDOWN and can't add it without potentially breaking
-	 * userspace.  At a platform view, INIT is acceptable behavior as
-	 * there exist bare metal platforms that automatically INIT the CPU
-	 * in response to shutdown.
+	 * The VMCB save area is undefined after a SHUTDOWN intercept, as are
+	 * control area offsets 60h, 61h, and 68h (int_ctl[15:0] and int_state).
+	 * INIT the vCPU to put the VMCB in a known good state.  Unfortuately,
+	 * KVM doesn't have KVM_MP_STATE_SHUTDOWN and can't add it without
+	 * potentially breaking userspace.  At a platform view, INIT is
+	 * acceptable behavior as there exist bare metal platforms that
+	 * automatically INIT the CPU in response to shutdown.
 	 *
 	 * The VM save area for SEV-ES guests has already been encrypted so it
 	 * cannot be reinitialized, i.e. synthesizing INIT is futile.
 	 */
 	if (!is_sev_es_guest(vcpu)) {
-		clear_page(svm->vmcb);
+		struct vmcb_control_area *control = &svm->vmcb->control;
+
+		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
+		control->int_ctl &= ~GENMASK(15, 0);
+		control->int_state = 0;
 #ifdef CONFIG_KVM_SMM
 		if (is_smm(vcpu))
 			kvm_smm_changed(vcpu, false);

base-commit: 1b731e5ded480bd1e5546aed35584238661ce72e
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
  2026-08-24 13:18 [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept Shivansh Dhiman
@ 2026-08-24 13:51 ` sashiko-bot
  2026-08-25  6:43   ` Shivansh Dhiman
  2026-10-05 18:16 ` Shivansh Dhiman
  1 sibling, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-08-24 13:51 UTC (permalink / raw)
  To: Shivansh Dhiman; +Cc: kvm

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing `clear_page(svm->vmcb)` leaves the `control->event_inj` and `control->event_inj_err` fields intact across a SHUTDOWN reset, causing KVM to spuriously inject stale events into the newly reset guest and leading to an immediate crash or triple fault.
--

--- Patch [1]: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept ---
commit 887c839299b43bfbaf990edaf22e0335bb4c3119
Author: Shivansh Dhiman <shivansh.dhiman@amd.com>

KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept

This commit updates KVM's handling of intercepted shutdowns for AMD SVM.
Instead of clearing the entire VMCB, it now only clears the save area and a
few undefined fields in the control area, aligning with updated APM
specifications.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
[ ... ]
> @@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
[ ... ]
>  	if (!is_sev_es_guest(vcpu)) {
> -		clear_page(svm->vmcb);
> +		struct vmcb_control_area *control = &svm->vmcb->control;
> +
> +		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
> +		control->int_ctl &= ~GENMASK(15, 0);
> +		control->int_state = 0;

[Severity: High]
Since clear_page() was replaced with targeted clearing, do we also need to
clear control->event_inj and control->event_inj_err here?

According to the AMD APM, hardware does not clear the EVENTINJ field in the
VMCB on a VM exit, which leaves the VALID bit set if an exit occurs during
event delivery (such as a triple fault leading to SVM_EXIT_SHUTDOWN).

If the EVENTINJ field is left intact across this shutdown reset, will KVM
execute the subsequent VMRUN with the stale EVENTINJ_VALID flag still set?

Could this cause KVM to spuriously inject the old fault into the newly
reset real-mode vCPU on its first instruction, leading to an immediate
crash or an infinite triple-fault loop?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824131824.6040-1-shivansh.dhiman@amd.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
  2026-08-24 13:51 ` sashiko-bot
@ 2026-08-25  6:43   ` Shivansh Dhiman
  2026-10-06  5:02     ` Sean Christopherson
  0 siblings, 1 reply; 6+ messages in thread
From: Shivansh Dhiman @ 2026-08-25  6:43 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: kvm, Shivansh Dhiman



On 24-08-26 19:21, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing `clear_page(svm->vmcb)` leaves the `control->event_inj` and `control->event_inj_err` fields intact across a SHUTDOWN reset, causing KVM to spuriously inject stale events into the newly reset guest and leading to an immediate crash or triple fault.
> --
> 
> --- Patch [1]: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept ---
> commit 887c839299b43bfbaf990edaf22e0335bb4c3119
> Author: Shivansh Dhiman <shivansh.dhiman@amd.com>
> 
> KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
> 
> This commit updates KVM's handling of intercepted shutdowns for AMD SVM.
> Instead of clearing the entire VMCB, it now only clears the save area and a
> few undefined fields in the control area, aligning with updated APM
> specifications.
> 
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
> [ ... ]
>> @@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
> [ ... ]
>>  	if (!is_sev_es_guest(vcpu)) {
>> -		clear_page(svm->vmcb);
>> +		struct vmcb_control_area *control = &svm->vmcb->control;
>> +
>> +		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
>> +		control->int_ctl &= ~GENMASK(15, 0);
>> +		control->int_state = 0;
> 
> [Severity: High]
> Since clear_page() was replaced with targeted clearing, do we also need to
> clear control->event_inj and control->event_inj_err here?
> 
> According to the AMD APM, hardware does not clear the EVENTINJ field in the
> VMCB on a VM exit, which leaves the VALID bit set if an exit occurs during
> event delivery (such as a triple fault leading to SVM_EXIT_SHUTDOWN).
> 
> If the EVENTINJ field is left intact across this shutdown reset, will KVM
> execute the subsequent VMRUN with the stale EVENTINJ_VALID flag still set?
> 
> Could this cause KVM to spuriously inject the old fault into the newly
> reset real-mode vCPU on its first instruction, leading to an immediate
> crash or an infinite triple-fault loop?
The latest APM says otherwise. The #VMEXIT sequence in APM ends with "clear
EVENTINJ field in VMCB". Hardware clears the field on every exit, so there
is nothing stale to carry across the shutdown reset and the scenario can't
arise.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
  2026-08-24 13:18 [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept Shivansh Dhiman
  2026-08-24 13:51 ` sashiko-bot
@ 2026-10-05 18:16 ` Shivansh Dhiman
  1 sibling, 0 replies; 6+ messages in thread
From: Shivansh Dhiman @ 2026-10-05 18:16 UTC (permalink / raw)
  To: seanjc, pbonzini, tglx, mingo
  Cc: kvm, x86, thomas.lendacky, nikunj.dadhania, santosh.shukla,
	Shivansh Dhiman



On 24-08-26 18:48, Shivansh Dhiman wrote:
> The AMD APM previously stated that after an intercepted shutdown, the
> entire VMCB state is undefined. In practice, only the save area is
> undefined and most of the control area remains valid. Revision 3.45 of
> the APM now makes this explicit:
> 
>   "After an intercepted shutdown, the VMCB control area is valid (with
>    the exception of offsets 60h, 61h, and 68h) and the VMCB state save
>    area is undefined."
> 
> KVM zeroes the entire VMCB before INITing the vCPU based on the old
> wording, discarding control area state that hardware preserves.
> 
> Clear only the save area and the three undefined control area fields
> (int_ctl[15:0] and int_state) in line with the updated APM.

Hi,

Gentle ping for reviewing/merging this patch for VMCB save area
clearing on a shutdown intercept. Would appreciate any feedback or
guidance on next steps.

Thanks,
Shivansh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
  2026-08-25  6:43   ` Shivansh Dhiman
@ 2026-10-06  5:02     ` Sean Christopherson
  2026-10-06 15:57       ` Shivansh Dhiman
  0 siblings, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2026-10-06  5:02 UTC (permalink / raw)
  To: Shivansh Dhiman; +Cc: sashiko-reviews, kvm

On Tue, Aug 25, 2026, Shivansh Dhiman wrote:

> On 24-08-26 19:21, sashiko-bot@kernel.org wrote:
> >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> >> --- a/arch/x86/kvm/svm/svm.c
> >> +++ b/arch/x86/kvm/svm/svm.c
> > [ ... ]
> >> @@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
> > [ ... ]
> >>  	if (!is_sev_es_guest(vcpu)) {
> >> -		clear_page(svm->vmcb);
> >> +		struct vmcb_control_area *control = &svm->vmcb->control;
> >> +
> >> +		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
> >> +		control->int_ctl &= ~GENMASK(15, 0);
> >> +		control->int_state = 0;
> > 
> > [Severity: High]
> > Since clear_page() was replaced with targeted clearing, do we also need to
> > clear control->event_inj and control->event_inj_err here?
> > 
> > According to the AMD APM, hardware does not clear the EVENTINJ field in the
> > VMCB on a VM exit, which leaves the VALID bit set if an exit occurs during
> > event delivery (such as a triple fault leading to SVM_EXIT_SHUTDOWN).
> > 
> > If the EVENTINJ field is left intact across this shutdown reset, will KVM
> > execute the subsequent VMRUN with the stale EVENTINJ_VALID flag still set?
> > 
> > Could this cause KVM to spuriously inject the old fault into the newly
> > reset real-mode vCPU on its first instruction, leading to an immediate
> > crash or an infinite triple-fault loop?
> The latest APM says otherwise. The #VMEXIT sequence in APM ends with "clear
> EVENTINJ field in VMCB". Hardware clears the field on every exit, so there
> is nothing stale to carry across the shutdown reset and the scenario can't
> arise.

But that seemingly unconditional statement has hidden clauses[*]:

 : >> The second half isn't. Clearing EVENTINJ is not part of injection, it's part of
 : >> #VMEXIT, and it's unconditional.
 : > So that doesn't mesh with the above comment, which says:
 : >
 : >   Hardware clears EVENTINJ field when it injects an event.
 : >
 : > And it begs the question of how this patch is at all useful.  Because all this
 : > fancy new paranoia is clearly generating #VMEXITs, and if #VMEXIT unconditionally
 : > clears control->event_inj, I don't see how control->event_inj can be non-zero if
 : > KVM attempted VMRUN.
 : >
 : > I.e. either this is all broken, or the APM is buggy.
 : 
 : My wording in the comment is misleading. The clear is part of the #VMEXIT path
 : out of guest mode, where it is indeed unconditional. However, VMRUN can
 : terminate even before the guest mode is ever entered. With ESMTP, the hardware
 : can give out a garden variety of exits at the sync point. In this case we
 : get a VMEXIT that implies the VMRUN never actually ran any guest code then
 : EVENTINJ won't be cleared. Since the exit code isn't a reliable discriminator,
 : a non-zero EVENTINJ is the only way to tell.

Ignoring that IMO the APM needs to be updated to clarify exactly when EVENTINJ
is cleared and when it isn't, I don't see any point in not manually clearing the
field on SHUTDOWN.  Common sense would say that it's unnecessary as SHUTDOWN can
only occur if VMRUN gets into the guest, but the cost is completely neglible and
there is zero chance EVENTINJ *needs* to be retained, unlike say the PML index.

We can certainly add a comment saying it's paranoid, but I do think we should
manually clear EVENTINJ and friend, e.g.

	/*
	 * EVENTINJ is cleared on #VMEXIT, but only if VMRUN fully entered the
	 * guest.  Manually clear the fields even though it should be redundant,
	 * as there is no downside to doing so.
	 */
	control->event_inj = 0;
	control->event_inj_err = 0;

[*] https://lore.kernel.org/all/d946de080e3dd34844a665c167179e4910fdc9d4.1789399214.git.prsampat@amd.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept
  2026-10-06  5:02     ` Sean Christopherson
@ 2026-10-06 15:57       ` Shivansh Dhiman
  0 siblings, 0 replies; 6+ messages in thread
From: Shivansh Dhiman @ 2026-10-06 15:57 UTC (permalink / raw)
  To: Sean Christopherson; +Cc: sashiko-reviews, kvm, Shivansh Dhiman



On 06-10-26 10:32, Sean Christopherson wrote:
> On Tue, Aug 25, 2026, Shivansh Dhiman wrote:
> 
>> On 24-08-26 19:21, sashiko-bot@kernel.org wrote:
>>>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>>>> --- a/arch/x86/kvm/svm/svm.c
>>>> +++ b/arch/x86/kvm/svm/svm.c
>>> [ ... ]
>>>> @@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
>>> [ ... ]
>>>>  	if (!is_sev_es_guest(vcpu)) {
>>>> -		clear_page(svm->vmcb);
>>>> +		struct vmcb_control_area *control = &svm->vmcb->control;
>>>> +
>>>> +		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
>>>> +		control->int_ctl &= ~GENMASK(15, 0);
>>>> +		control->int_state = 0;
>>>
>>> [Severity: High]
>>> Since clear_page() was replaced with targeted clearing, do we also need to
>>> clear control->event_inj and control->event_inj_err here?
>>>
>>> According to the AMD APM, hardware does not clear the EVENTINJ field in the
>>> VMCB on a VM exit, which leaves the VALID bit set if an exit occurs during
>>> event delivery (such as a triple fault leading to SVM_EXIT_SHUTDOWN).
>>>
>>> If the EVENTINJ field is left intact across this shutdown reset, will KVM
>>> execute the subsequent VMRUN with the stale EVENTINJ_VALID flag still set?
>>>
>>> Could this cause KVM to spuriously inject the old fault into the newly
>>> reset real-mode vCPU on its first instruction, leading to an immediate
>>> crash or an infinite triple-fault loop?
>> The latest APM says otherwise. The #VMEXIT sequence in APM ends with "clear
>> EVENTINJ field in VMCB". Hardware clears the field on every exit, so there
>> is nothing stale to carry across the shutdown reset and the scenario can't
>> arise.
> 
> But that seemingly unconditional statement has hidden clauses[*]:
> 
>  : >> The second half isn't. Clearing EVENTINJ is not part of injection, it's part of
>  : >> #VMEXIT, and it's unconditional.
>  : > So that doesn't mesh with the above comment, which says:
>  : >
>  : >   Hardware clears EVENTINJ field when it injects an event.
>  : >
>  : > And it begs the question of how this patch is at all useful.  Because all this
>  : > fancy new paranoia is clearly generating #VMEXITs, and if #VMEXIT unconditionally
>  : > clears control->event_inj, I don't see how control->event_inj can be non-zero if
>  : > KVM attempted VMRUN.
>  : >
>  : > I.e. either this is all broken, or the APM is buggy.
>  : 
>  : My wording in the comment is misleading. The clear is part of the #VMEXIT path
>  : out of guest mode, where it is indeed unconditional. However, VMRUN can
>  : terminate even before the guest mode is ever entered. With ESMTP, the hardware
>  : can give out a garden variety of exits at the sync point. In this case we
>  : get a VMEXIT that implies the VMRUN never actually ran any guest code then
>  : EVENTINJ won't be cleared. Since the exit code isn't a reliable discriminator,
>  : a non-zero EVENTINJ is the only way to tell.
> 
> Ignoring that IMO the APM needs to be updated to clarify exactly when EVENTINJ
> is cleared and when it isn't, I don't see any point in not manually clearing the
> field on SHUTDOWN.  Common sense would say that it's unnecessary as SHUTDOWN can
> only occur if VMRUN gets into the guest, but the cost is completely neglible and
> there is zero chance EVENTINJ *needs* to be retained, unlike say the PML index.
> 
> We can certainly add a comment saying it's paranoid, but I do think we should
> manually clear EVENTINJ and friend, e.g.

Agreed. I'll clear event_inj and event_inj_err with your comment and post v2
soon.

Thanks,
Shivansh

> 
> 	/*
> 	 * EVENTINJ is cleared on #VMEXIT, but only if VMRUN fully entered the
> 	 * guest.  Manually clear the fields even though it should be redundant,
> 	 * as there is no downside to doing so.
> 	 */
> 	control->event_inj = 0;
> 	control->event_inj_err = 0;
> 
> [*] https://lore.kernel.org/all/d946de080e3dd34844a665c167179e4910fdc9d4.1789399214.git.prsampat@amd.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-06 15:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 13:18 [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept Shivansh Dhiman
2026-08-24 13:51 ` sashiko-bot
2026-08-25  6:43   ` Shivansh Dhiman
2026-10-06  5:02     ` Sean Christopherson
2026-10-06 15:57       ` Shivansh Dhiman
2026-10-05 18:16 ` Shivansh Dhiman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox