|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/6] x86/pass-through: no locking around pt_irq_{create,destroy}_bind()
On Thu, Sep 24, 2026 at 11:57:19AM +0200, Jan Beulich wrote:
> On 23.09.2026 12:37, Roger Pau Monné wrote:
> > On Tue, Sep 08, 2026 at 03:01:51PM +0200, Jan Beulich wrote:
> >> The questionable use of pcidevs_lock() there was discussed more than once.
> >> It really is pointless: The functions synchronize primarily via the per-
> >> domain event lock. They also may already be called with the global PCI
> >> devices lock not held: See hvm/vmsi.c:vpci_msi_update(),
> >> hvm/vmsi.c:vpci_msi_arch_update(), and hvm/vmsi.c:vpci_msi_disable().
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>
> >> --- a/xen/arch/x86/domctl.c
> >> +++ b/xen/arch/x86/domctl.c
> >> @@ -636,10 +636,7 @@ long arch_do_domctl(
> >> ret = -EPERM;
> >> else if ( is_iommu_enabled(d) )
> >> {
> >> - pcidevs_lock();
> >> ret = pt_irq_create_bind(d, bind);
> >> - pcidevs_unlock();
> >
> > pt_irq_create_bind() might call into msixtbl_pt_register() which
> > requires either the pcidevs_lock() or the per-domain d->pci_lock lock
> > to be taken, which I think is not the case in the context here?
>
> Hmm, indeed. Not having seen the assertion there trigger kind of worries
> me a little. Do you agree that the change to vioapic_hwdom_map_gsi() can,
> otoh, be left as is?
Hm, I'm borderline on that one - I can't find a path where d->pci_lock
will be needed for legacy PCI interrupt binding, yet at the same time
I feel it would be better if the locking context is uniform across
call sites. I guess I'm fine with the asymmetric locking context if
that's your preference. Maybe worth a mention in a comment somewhere.
> In turn I will then extend patch 3 to also tighten the assertions in
> msixtbl_pt_{,un}register(), as each of them has only this one call site.
> Would you mind indicating whether in doing so I may retain you A-b there?
Please keep the A-b there.
Thanks, Roger.
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |