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

On 8/11/2026 6:13 AM, Sean Christopherson wrote:
> On Mon, Aug 10, 2026, Sairaj Kodilkar wrote:
>> On 7/13/2026 4:41 PM, [email protected] wrote:
>>> [Severity: High]
>>> Can an unprivileged user-space process trigger this WARN_ON?
>>>
>>> When a vCPU is created, svm->gappi_cpu is initialized to -1 in
>>> avic_init_vcpu(). It is only set to a valid CPU ID later during
>>> __avic_vcpu_load().
>>>
>>> If host user-space configures device interrupt routing via the KVM_IRQFD ioctl
>>> before the vCPU runs, avic_pi_update_irte() is invoked. This will pass the
>>> uninitialized svm->gappi_cpu (-1) down to this function, hitting the WARN_ON.
>>> If the host has panic_on_warn enabled, this allows host userspace to trigger
>>> a kernel panic.
>>
>> This is a valid concern.
>>
>> If host userspace attaches a bypass IRQ targeting a vCPU that has never
>> been loaded. Functionally, there is nothing to do in that window. A vCPU
>> that has never been loaded cannot be blocking, so no GAPPI wakeup is
>> required. The IOMMU still posts the interrupt into the vAPIC backing
>> page, and the pending IRR is evaluated at the first VMRUN after
>> avic_vcpu_load(), which is also where the IRTE gets a valid Destination
>> and IsRun = 1.
>>
>> This can be resolved by assigning a arbitrary gappi destination, without
>> actually updating the gappi wakeup list of that CPU.
> 
> With the disclaimer that I haven't look super closely at this series, and haven't
> thought too deeply about the feature itself either...
> 
> Why are we doing anything different than what VMX does?  vCPUs on the wakeup
> list when they block, and come off the list when they wakeup.  It's literally
> one flow that's guarantee to pair put()+load(), and the logic for manipulating
> the list is quite simple as a result.
> 

I was trying to manipulate the vCPU list after is_empty(ir_list) check
in put() and load() path. Which complicated the things, since
pi_update_irte() will have to add the vCPU to the list if it was a first
interrupt assigned to given vCPU.

I think its better to keep the list operations before is_empty(ir_list)
check in order to simplify things a little bit.Note that it may increase
the list size and potentially increasing time for gappi_wakeup_handler.

> Going a step further, why is GAPPI not sharing code with VMX Posted Interupts?
> At a glance, the only meaningful difference in the wakeup flow is the "should
> this particular vCPU be awakened".

On Intel there is a level of indirection: KVM hands the IOMMU the
physical address of the PI descriptor once, at IRQ affinity setup time
(vmx_pi_update_irte()). After that the descriptor is the only thing that
needs updating, so vcpu_load()/vcpu_put() just write NDST/NV/SN in
memory and never call into the IOMMU driver again.

AMD has no such indirection. The destination APIC ID, IsRun and GATag
live directly in the IRTE, and there is no per-vCPU structure that the
IOMMU dereferences. So every vcpu_load()/vcpu_put() has to call into the
AMD IOMMU driver to update each IRTE targeting the vCPU, via
avic_update_iommu_vcpu_affinity() -> amd_iommu_update_ga().

That difference leaks into the wakeup list handling. On Intel, putting a
vCPU on the per-pCPU list is self-contained. On AMD it has to happen in
the same ir_list_lock critical section as the IRTE update, and it is
conditional on the vCPU actually having posted IRQs (ir_list being non
empty), because the pCPU that the vCPU is enqueued on is also what gets
programmed into IRTE[Destination].

Because of above complications I did not try to factor out the common code.

Thanks
Sairaj