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

[Sairaj Kodilkar <sarunkod@amd.com>]

On 6/26/2026 4:59 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:
> - [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?

The gappi_dis bit is not cached. See the IOMMU spec [1] section
2.2.5.2 Interrupt Virtualization Tables with Guest Virtual APIC Enabled
"
When virtual interrupts are enabled by setting MMIO Offset 0018h[GAEn] 
and IRTE[GuestMode=1],
the fields IRTE[IsRun], IRTE[Destination], IRTE[GALogIntr], 
IRTE[GAPPIDis], and, if present,
IRTE[GATag] are not cached by the IOMMU. Modifications to these fields 
do not require an
invalidation of the Interrupt Remapping Table.
"

Hence we do not need to flush the IRTE after modifying gappi_dis bit.

[1] https://docs.amd.com/v/u/en-US/48882_3.11_IOMMU_PUB

Thanks
Sairaj
>> +                     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