Re: [PATCH v5 6/7] KVM: SVM: Add support for AMD IOMMU Guest APIC Physical Processor Interrupt (GAPPI)

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: svm: vCPU leaked on GAPPI pi_wakeup_list when AVIC deactivated
--

commit cd7fa3c00904984a2127bee934d4e48326ead7a8
Author: Sairaj Kodilkar <sarunkod@amd.com>

    KVM: SVM: Add support for AMD IOMMU Guest APIC Physical Processor Interrupt (GAPPI)
    
    With AVIC guest-mode interrupt remapping, device interrupts are posted into
    the guest vAPIC backing page by the IOMMU. When the vCPU is not running
    (IRTE[IsRun] = 0), KVM must still be notified to schedule it. The legacy
    path uses the GA log.

> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index 8b1fa5bded3db..054b89c3cb0f9 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
[ ... ]
> @@ -1018,7 +1038,7 @@ enum avic_vcpu_action {
>  };
>  
>  static void avic_update_iommu_vcpu_affinity(struct kvm_vcpu *vcpu, int apicid,
> -					    enum avic_vcpu_action action)
> +					    int cpu, enum avic_vcpu_action action)
>  {
>  	bool wakeup_intr = (action & AVIC_START_BLOCKING);
>  	bool is_running = apicid >= 0;
> @@ -1027,6 +1047,14 @@ static void avic_update_iommu_vcpu_affinity(struct kvm_vcpu *vcpu, int apicid,
>  
>  	lockdep_assert_held(&svm->ir_list_lock);
>  
> +	if (amd_iommu_gappi && is_running) {
> +		kvm_pi_disable_wakeup_handler(vcpu, svm->gappi_cpu);
> +		svm->gappi_cpu = cpu; /* Store cpu number as target for GAPPI */
> +	} else if (amd_iommu_gappi) {
> +		apicid = cpu_physical_id(svm->gappi_cpu);
> +		kvm_pi_enable_wakeup_handler(vcpu, svm->gappi_cpu);
> +	}
> +

[Severity: Medium]
Does this unconditionally add the vCPU to the pi_wakeup_list when is_running is
false, even during AVIC_DEACTIVATE?

When AVIC is deactivated via avic_refresh_apicv_exec_ctrl(), it calls
__avic_vcpu_put(AVIC_DEACTIVATE), which in turn calls
avic_update_iommu_vcpu_affinity() with a target apicid of -1 (so is_running
is false). This causes KVM to add the vCPU to the pi_wakeup_list of
svm->gappi_cpu.

Since AVIC is deactivated, the IOMMU falls back to legacy mode later in this
function:

	if (!(action & AVIC_TOGGLE_ON_OFF))
		WARN_ON_ONCE(amd_iommu_update_ga(...));
	else if (is_running)
		WARN_ON_ONCE(amd_iommu_activate_guest_mode(...));
	else
		WARN_ON_ONCE(amd_iommu_deactivate_guest_mode(data));

Because the IOMMU is reverted to legacy mode, it will not send GAPPI interrupts
for this vCPU.

However, the vCPU remains indefinitely on the per-CPU pi_wakeup_list because
subsequent context switches (via svm_vcpu_load()) ignore deactivated vCPUs and
will never call kvm_pi_disable_wakeup_handler().

Could this cause KVM to unnecessarily traverse the leaked vCPU on every GAPPI
interrupt targeted at that pCPU from other active VMs, wasting CPU cycles and
risking spurious wakeups?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015064515.10700-1-sarunkod@amd.com?part=6