Re: [PATCH 1/6] x86/pass-through: defer event unlock in pt_irq_create_bind()

[Jan Beulich <jbeulich@suse.com>]

On 22.09.2026 15:37, Roger Pau Monné wrote:
> On Tue, Sep 08, 2026 at 03:01:27PM +0200, Jan Beulich wrote:
>> The radix tree holding struct pirq * as obtained by pirq_get_info() is
>> protected by the domain's event lock. The result ("info") and the derived
>> "pirq_dpci" therefore may not be de-referenced past the dropping of that
>> lock. Moving the unlock down is safe, but perhaps not obviously so:
>> - vector_hashing_dest() does a memory allocation, but core event channel
>>   code does so too while holding the lock; the call to
>>   vlapic_match_dest() doesn't involve any further locking,
>> - hvm_migrate_pirq() operates on the corresponding IRQ descriptor, where
>>   obtaining of its lock is of course fine (those locks always nest inside
>>   the event lock),
>> - {hvm,vmx}_pi_update_irte() are very similar to hvm_migrate_pirq()
>>   locking-wise,
>> - the locking around guest_mask_msi_irq() is the same as in the earlier
>>   two bullet points.
>>
>> As long as the PCI-devs lock is held around both
>> pt_irq_{create,destroy}_bind(), this is only a latent issue.
>>
>> Fixes: 35a1caf8b6b5 ("pass-through: update IRTE according to guest interrupt config changes")
>> Fixes: 1066331913c9 ("passthrough: don't migrate pirq when it is delivered through VT-d PI")
>> Fixes: 782cf8ba4678 ("pass-through: adjust pIRQ migration")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks.

>> Seeing that the IRQ descriptor lock is taken up to three times in a row,
>> I wonder whether we shouldn't consolidate this (by obtaining desc once and
>> then passing it into hvm_migrate_pirq() (or a suitable new sibling
>> thereof) and hvm_pi_update_irte()).
> 
> Possibly?  I haven't looked what would be the adjustment needed on
> other callers.  Just passing the locked irq_desc?

Yes. Yet in carrying that out I realize that calling iommu_update_ire_from_msi()
(out of vmx_pi_update_irte()) with the lock held is perhaps more risky than I'd
like it to be. With limited effort I couldn't convince myself that there might
not be a lock order inversion issue somewhere.

Jan