Re: [RFC PATCH 2/5] iommu/amd: Configure IRTE to use the GAPPI for posted interrupts

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `amd_iommu_update_ga` dynamically modifies the `gappi_dis` field in the IRTE without issuing an IOMMU invalidation, which may result in permanently lost wakeups.
--

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

iommu/amd: Configure IRTE to use the GAPPI for posted interrupts

This commit updates the AMD IOMMU driver to support Guest APIC Posted
Interrupts (GAPPI). It configures the Interrupt Remapping Table Entry (IRTE)
for wakeups using the reserved POSTED_INTR_WAKEUP_VECTOR when the guest
is not running.

> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
[ ... ]
> @@ -3969,9 +3969,18 @@ static void __amd_iommu_update_ga(struct irte_ga *entry, int apicid, int flags)
>  		entry->lo.fields_vapic.is_run = true;
>  		entry->lo.fields_vapic.ga_log_intr = false;
>  	} else {
> -		entry->lo.fields_vapic.is_run = false;
> -		entry->lo.fields_vapic.ga_log_intr = !!(flags &
> -							AMD_IOMMU_FLAG_POSTED_INTR);
> +		bool posted_intr = !!(flags & AMD_IOMMU_FLAG_POSTED_INTR);
> +		if (amd_iommu_gappi) {
> +			entry->lo.fields_vapic.gappi_dis = !posted_intr &&
> +							   check_feature2(FEATURE_GAPPIDISSUP);

[Severity: Medium]
Does this code need to issue an IOMMU cache invalidation after modifying
gappi_dis?

Since __amd_iommu_update_ga() is a fast-path update function, it appears
there is no subsequent IOMMU cache flush when gappi_dis changes. If KVM
schedules out a vCPU (setting gappi_dis = 1) and then the vCPU blocks
(setting gappi_dis = 0), hardware that caches the gappi_dis bit might not
observe the transition back to 0.

Could this caching behavior lead to permanently dropped wakeup interrupts
and indefinite vCPU hangs?

> +			entry->lo.fields_vapic.is_run = false;
> +			entry->lo.fields_vapic.destination =
> +						APICID_TO_IRTE_DEST_LO(apicid);
> +			entry->hi.fields.destination = APICID_TO_IRTE_DEST_HI(apicid);

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