Re: [PATCH v7 7/7] iommu/amd: Provide kernel command line option to enable GAPPI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] iommu/amd: enable_iommus_vapic() leaves amd_iommu_gappi true
  on early return
--

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

iommu/amd: Provide kernel command line option to enable GAPPI

This commit adds the amd_iommu=gappi kernel command-line option to enable
GAPPI when the IOMMU GAPPISup extended feature bit is supported, and
documents the option and mechanism.

> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 054ef7262f3cc..6a248327047ac 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c

[ ... ]

> @@ -3131,18 +3141,27 @@ static void enable_iommus_vapic(void)
>  	if (!AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))
>  		return;
>  
> +	if (amd_iommu_gappi &&
> +	    !(check_feature(FEATURE_GAPPI) &&
> +	      AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir))) {
> +		pr_warn("GAPPI is not supported.\n");
> +		amd_iommu_gappi = false;
> +	}

[Severity: Medium]
Does the early return when !AMD_IOMMU_GUEST_IR_VAPIC(amd_iommu_guest_ir)
make the AMD_IOMMU_GUEST_IR_VAPIC() check right below it unreachable when
false and leave amd_iommu_gappi set to true?

parse_amd_iommu_options() sets amd_iommu_gappi = true before Virtual APIC
support is validated.  If the kernel is booted with amd_iommu=gappi and
IOMMU Virtual APIC is disabled or downgraded (for example with
amd_iommu_intr=legacy or when check_vapic_support() falls back to legacy
GA mode), enable_iommus_vapic() returns before clearing amd_iommu_gappi
(and likewise if the IOMMU is disabled or fails to initialize).

When CPU AVIC is active in KVM SVM, avic_update_iommu_vcpu_affinity() and
avic_vcpu_free() check amd_iommu_gappi before checking whether the vCPU
has any IOMMU IRTEs:

arch/x86/kvm/svm/avic.c:avic_update_iommu_vcpu_affinity() {
	...
	if (!amd_iommu_gappi)
		goto skip_wakeup_handler;
	...
	if (is_running) {
		kvm_pi_disable_wakeup_handler(vcpu, svm->gappi_cpu);
		svm->gappi_cpu = cpu;
	} else {
		apicid = cpu_physical_id(svm->gappi_cpu);
		kvm_pi_enable_wakeup_handler(vcpu, svm->gappi_cpu);
	}

skip_wakeup_handler:
	if (list_empty(&svm->ir_list))
		return;
	...
}

Leaving amd_iommu_gappi set to true when CONTROL_GAPPI_EN is not enabled
causes KVM to register and unregister per-CPU wakeup handlers on every
vCPU load, put, block, and free even though IOMMU Virtual APIC is not in
use.

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