Re: [PATCH v4 6/7] KVM: SVM: Add support for AMD IOMMU Guest APIC Physical Processor Interrupt (GAPPI)

[Sairaj Kodilkar]

On 8/21/2026 9:45 PM, Vasant Hegde wrote:
> 
> 
> On 8/21/2026 11:26 AM, Sairaj Kodilkar wrote:
>> With AVIC guest-mode interrupt remapping, device interrupts are posted into
>> the guest vAPIC backing page by the IOMMU. When the vCPU is not running
>> (IRTE[IsRun] = 0), KVM must still be notified to schedule it. The legacy
>> path uses the GA log.
>>
>> GAPPI (Guest APIC Physical Processor Interrupt) is an alternative to the
>> GA log mechanism provided by the AMD IOMMU. With GAPPI enabled, the IOMMU
>> still updates the vAPIC backing page IRR, but the host wakeup notification
>> is delivered as a physical APIC interrupt to IRTE[Destination], using
>> IRTE[GATag][7:0] as the vector (POSTED_INTR_WAKEUP_VECTOR).
>>
>> SVM follows the Intel posted-interrupt wakeup model. Each pCPU maintains
>> a list of blocked vCPUs that may be woken by a GAPPI delivery to that CPU.
>> When a vCPU blocks while waiting for an interrupt, SVM enqueues it on the
>> wakeup list of the pCPU on which it was previously running and passes that
>> same pCPU's physical APIC ID to the IOMMU to program IRTE[Destination].
>> The rationale is that the vCPU is likely to run again on the same pCPU,
>> which is common when vCPUs are pinned; targeting GAPPI notifications there
>> reduces unnecessary VMEXITs from GAPPI deliveries on other CPUs. SVM
>> registers the GAPPI handler via kvm_set_posted_intr_wakeup_handler(). On
>> delivery, it walks the local vCPU list and wakes vCPUs with a pending IRR.
>>
>> All GAPPI logic is gated on amd_iommu_gappi. Without it, KVM and the IOMMU
>> falls back to the legacy GA log mechanism for vCPU wakeup.
>>
>> Signed-off-by: Sairaj Kodilkar <sarunkod@xxxxxxx>
>> ---
>>  arch/x86/kvm/svm/avic.c | 72 ++++++++++++++++++++++++++++++++++++-----
>>  arch/x86/kvm/svm/svm.c  |  3 ++
>>  arch/x86/kvm/svm/svm.h  |  5 +++
>>  3 files changed, 72 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
>> index dd497530d365..18ac24ef40e1 100644
>> --- a/arch/x86/kvm/svm/avic.c
>> +++ b/arch/x86/kvm/svm/avic.c
>> @@ -874,6 +874,8 @@ int avic_init_vcpu(struct vcpu_svm *svm)
>>  	INIT_LIST_HEAD(&svm->ir_list);
>>  	raw_spin_lock_init(&svm->ir_list_lock);
>>  
>> +	svm->gappi_cpu = -1;
> 
> Can we initialize it to CPU0?

Yeah, I think we can do this, I was worried about passing gappi_cpu=0 to
kvm_pi_disable_wakeup_handler() when vCPU is not in list but I think it
should be safe because of list_empty() check.

>> +
>>  	if (!enable_apicv || !irqchip_in_kernel(vcpu->kvm))
>>  		return 0;
>>  
>> @@ -886,6 +888,20 @@ int avic_init_vcpu(struct vcpu_svm *svm)
>>  	return ret;
>>  }
>>  
>> +void avic_destroy_vcpu(struct vcpu_svm *svm)
>> +{
>> +	if (amd_iommu_gappi && svm->gappi_cpu != -1) {
> 
> Redundant amd_iommu_gappi check?

I am not sure if we should call kvm_pi_disable_wakeup_handler() outside
amd_iommu_gappi check. Although it is safe to do so, I want to have this
call inside amd_iommu_gappi check.

Thanks
Sairaj.

> 
>> +		unsigned long flags;
>> +
>> +		local_irq_save(flags);
>> +
>> +		kvm_pi_disable_wakeup_handler(&svm->vcpu, svm->gappi_cpu);
>> +		svm->gappi_cpu = -1;
> 
> Redundant assignement?
> 
> -Vasant
>