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

	

On 7/13/2026 4:37 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] GAPPI uses the lower 8 bits of the IRTE `ga_tag` as the host physical APIC interrupt vector, but this tag holds the guest's unconstrained `vcpu_idx`, leading to host CPU exception injection.
> - [High] When GAPPI is enabled, putting a vCPU into a blocked state configures the IOMMU IRTE with a broadcast physical destination APIC ID (0xffffffff), causing an interrupt storm.
> --
> 
> --- Patch [4]: [RFC PATCH v3 4/6] iommu/amd: Program guest-mode IRTEs for GAPPI wakeup when IRTE[IsRun] = 0 ---
> commit aad2d4e0f82cbce4cf0a8e8b65994633202311a3
> Author: Sairaj Kodilkar <[email protected]>
> 
> iommu/amd: Program guest-mode IRTEs for GAPPI wakeup when IRTE[IsRun] = 0
> 
> This commit enables Guest APIC Physical Processor Interrupt (GAPPI) support
> for the AMD IOMMU. When a vCPU is scheduled out, it configures the IOMMU to
> deliver a physical APIC interrupt using the IRTE destination and GATag
> fields to wake up the blocked vCPU.
> 
>> Guest APIC Physical Processor Interrupt (GAPPI) is an alternative to the
>> GA log for notifying the host when a device interrupt targets a
>> non-running vCPU (IRTE[IsRun] = 0). Per the AMD IOMMU specification [1]
>> (section 2.2.5.4), with GAPPI enabled the IOMMU delivers a physical APIC
>> interrupt to the CPU described by IRTE[Destination], using
>> IRTE[GATag][7:0] as the vector, while still updating the guest vAPIC
>> backing page IRR as usual.
> 
> [Severity: Critical]
> KVM constructs the ga_tag using AVIC_GATAG(vm_id, vcpu_idx), which embeds
> the guest vcpu_idx into the lowest 8 bits. This unmodified tag is
> programmed into the IRTE's ga_tag field when a guest mode is activated:
> 
> drivers/iommu/amd/iommu.c:amd_iommu_activate_guest_mode() {
>     ...
>     entry->lo.fields_vapic.ga_tag = ir_data->ga_tag;
>     ...
> }
> 
> If GAPPI uses IRTE[GATag][7:0] as the physical APIC interrupt vector, and a
> device interrupts vCPU 0, will the IOMMU deliver a physical interrupt with
> vector 0?
> 
> Since vectors 0-31 on x86 are reserved for CPU exceptions, could this
> regression allow an untrusted guest VM to trigger a host kernel panic
> (e.g., #DE Divide-by-Zero) by firing an interrupt while a low-numbered vCPU
> is blocked?

Currently GAPPI depends on the variable amd_iommu_gappi which is 0, and
is turned on by kernel command line parameter in the last patch of this
series. So IOMMU will not use vectors 0-31. In fact next patch sets
ga_tag to POSTED_INTR_WAKEUP_VECTOR when amd_iommu_gappi is 1.

Thanks
Sairaj

> 
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index 91405e71b3c3a..7eebf4745a678 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -3970,8 +3970,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;
>>  	} 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);
> 
> [Severity: High]
> When KVM blocks a vCPU, it calls amd_iommu_update_ga() with apicid = -1 and
> wakeup_intr = true.
> 
> If apicid is -1, won't APICID_TO_IRTE_DEST_LO() and APICID_TO_IRTE_DEST_HI()
> convert this to 0xffffffff?
> 
> Does this regression result in writing the x2APIC broadcast address to the
> IRTE destination field, potentially causing a host-wide interrupt storm when
> a passed-through device fires an interrupt?

No, When amd_iommu_gappi is 1 and vCPU is unloaded, the SVM layer will
pass a valid apicid. Please see next patch for complete SVM side
implementation.

Thanks
Sairaj

> 
>> +		} else {
>> +			entry->lo.fields_vapic.is_run = false;
>> +			entry->lo.fields_vapic.ga_log_intr = wakeup_intr;
>> +		}
>>  	}
>>  }
>