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

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

[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

Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept

[sashiko-bot]

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

Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept

[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.