Re: [PATCH v5 4/7] iommu/amd: Program guest-mode IRTEs for GAPPI wakeup when IRTE[IsRun] = 0

[Sairaj Kodilkar <sarunkod@amd.com>]

On 10/15/2026 12:33 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] iommu/amd: stale gappi_dis hardware flag in IRTE when is_running=true
> --
> 
> commit fed3ee87aa05f53cbc2ccd8dd9c45c2a8ad8bb44
> Author: Sairaj Kodilkar <sarunkod@amd.com>
> 
> iommu/amd: Program guest-mode IRTEs for GAPPI wakeup when IRTE[IsRun] = 0
> 
> This commit enables programming the IRTE for Guest APIC Physical Processor
> Interrupt (GAPPI) wakeup when a vCPU is not running. It allows suppressing
> these interrupts using the IRTE[GAPPIDis] bit when KVM does not request a
> wakeup interrupt.
> 
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index c0eec32ee3cb6..7ceb6bf508f03 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -4004,8 +4004,18 @@ static void __amd_iommu_update_ga(struct irte_ga *entry, int apicid,
>>  		entry->lo.fields_vapic.is_run = true;
>>  		entry->lo.fields_vapic.ga_log_intr = false;
> 
> [Severity: Low]
> When a vCPU transitions to the running state, ga_log_intr is explicitly
> cleared here. Does gappi_dis also need to be cleared?
> 
> If a vCPU transitions to is_running=false with wakeup_intr=false, 
> gappi_dis can be set to 1 in the newly added else branch below. When the 
> vCPU transitions back to is_running=true, this code path is taken, but it 
> leaves the gappi_dis bit stale.
> 
> Could this cause unintended interrupt suppression or hardware state mismatches 
> if the IOMMU hardware evaluates the bit unexpectedly?
> 

The gappi_dis bit is evaluated only when is_run is false and is
otherwise ignored, hence it is safe to not assign gappi_dis=1 when vCPU
transitions to running state.

Thanks
Sairaj

>>  	} else {
>> -		entry->lo.fields_vapic.is_run = false;
>> -		entry->lo.fields_vapic.ga_log_intr = wakeup_intr;
>> +		if (amd_iommu_gappi) {
>> +			entry->lo.fields_vapic.gappi_dis = !wakeup_intr &&
>> +						check_feature2(FEATURE_GAPPIDISSUP);
>> +			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);
>> +		} else {
>> +			entry->lo.fields_vapic.is_run = false;
>> +			entry->lo.fields_vapic.ga_log_intr = wakeup_intr;
>> +		}
>>  	}
>>  }
>