Re: [PATCH 1/3] KVM: SVM: Re-queue events that were never injected

["Pratik R. Sampat" <prsampat@amd.com>]

On 9/14/26 1:16 PM, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] svm_complete_interrupts() incorrectly checks if (control->event_inj) instead of the SVM_EVTINJ_VALID bit, which can silently drop valid hardware-reported events (like guest page faults) by clobbering EXITINTINFO.
> --
> 
> commit 50716e3ba11eb9d000a0186ffad984ac08df1dcb
> Author: Pratik R. Sampat <prsampat@amd.com>
> 
> KVM: SVM: Re-queue events that were never injected
> 
> When injecting an event into the guest via the event_inj field, a non-zero
> event_inj value on #VMEXIT means the hardware was unable to inject it.
> This commit adds a check to recover potentially lost interrupts by detecting
> an undelivered injected event and re-queuing it.
> 
>> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
>> index d0971685034b9..e2be2cd86c320 100644
>> --- a/arch/x86/kvm/svm/svm.c
>> +++ b/arch/x86/kvm/svm/svm.c
> [ ... ]
>> @@ -4323,6 +4324,27 @@ static void svm_complete_interrupts(struct kvm_vcpu *vcpu)
>>       svm->nmi_l1_to_l2 = false;
>>       svm->soft_int_injected = false;
>>
>> +     /*
>> +      * Hardware clears EVENTINJ field when it injects an event.
>> +      * A non-empty EVENTINJ on #VMEXIT means the vCPU never entered guest
>> +      * mode, and thus that the event was never delivered.  Migrate the event
>> +      * to EXITINTINFO so that it's requeued instead of being dropped.
>> +      *
>> +      * An undelivered event doesn't imply a fatal VMEXIT_INVALID. With
>> +      * Enhanced SMT Protection, VMRUN may exit with an ordinary #VMEXIT
>> +      * without having injected that event into the guest.
>> +      *
>> +      * Clobbering EXITINTINFO is safe precisely because the vCPU never
>> +      * entered guest mode.
>> +      */
>> +     if (control->event_inj) {
> [Severity: High]
> Does this code silently drop valid hardware-reported events?
> 
> If an L1 hypervisor sets event_inj to a non-zero value but leaves the
> SVM_EVTINJ_VALID bit clear, hardware will ignore the injection and won't
> clear EVENTINJ.


The first half is right, nested_svm_check_event_inj() intentionally returns
true when V is clear, so vmcb02->control.event_inj can be non-zero with V=0.

The second half isn't. Clearing EVENTINJ is not part of injection, it's part of
#VMEXIT, and it's unconditional. APM vol 3, VMRUN:

  Upon #VMEXIT, the processor performs the following actions in order to
  return to the host execution context:
  ...
  EXITINTINFO
  clear EVENTINJ field in VMCB

KVM already relies on this, nested_svm_vmexit_update_vmcb12() zeroes vmcb12's
EVENTINJ rather than propagating vmcb02's, citing the same thing.


> 
> If a genuine exception occurs in the L2 guest that populates EXITINTINFO,
> this check incorrectly evaluates to true since event_inj is non-zero.
> 
> This causes EXITINTINFO to be clobbered with the L1's invalid event_inj
> data. Because the clobbered value lacks the VALID bit, KVM will then ignore
> it and drop the event.
> 
> Should this code check the SVM_EVTINJ_VALID bit instead of checking for a
> non-zero event_inj value?

I believe we don't need to since the valid check is made later below.

-- Pratik

> 
>> +             control->exit_int_info = control->event_inj;
>> +             control->exit_int_info_err = control->event_inj_err;
>> +             control->event_inj = 0;
>> +
>> +             exitintinfo = control->exit_int_info;
>> +     }
>> +
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789399214.git.prsampat@amd.com?part=1