Re: [PATCH v3 08/11] x86/dpci: switch to use a GSI EOI callback

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Apr 08, 2021 at 04:49:48PM +0200, Jan Beulich wrote:
> On 31.03.2021 12:33, Roger Pau Monne wrote:
> > @@ -515,17 +528,44 @@ int pt_irq_create_bind(
> >          }
> >          else
> >          {
> > +            /*
> > +             * NB: the callback structure allocated below will never be 
> > freed
> > +             * once setup because it's used by the hardware domain and will
> > +             * never be unregistered.
> > +             */
> > +            cb = xmalloc(struct hvm_gsi_eoi_callback);
> 
> Is this comment as well as ...
> 
> >              ASSERT(is_hardware_domain(d));
> >  
> > +            if ( !cb )
> > +            {
> > +                spin_unlock(&d->event_lock);
> > +                return -ENOMEM;
> > +            }
> > +
> >              /* MSI_TRANSLATE is not supported for the hardware domain. */
> >              if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_PCI ||
> >                   pirq >= hvm_domain_irq(d)->nr_gsis )
> >              {
> >                  spin_unlock(&d->event_lock);
> > -
> > +                xfree(cb);
> >                  return -EINVAL;
> >              }
> >              guest_gsi = pirq;
> > +
> > +            cb->callback = dpci_eoi;
> > +            cb->data = d;
> > +            /*
> > +             * IRQ binds created for the hardware domain are never 
> > destroyed,
> > +             * so it's fine to not keep a reference to cb here.
> > +             */
> > +            rc = hvm_gsi_register_callback(d, guest_gsi, cb);
> 
> ... the one here really true? vpci_msi_arch_update() and
> vpci_msi_disable() seem to tell me otherwise (or for the former
> comment, they suggest there should be un-registration somewhere).

MSI doesn't use hvm_gsi_register_callback at all, since those are only
used for GSI interrupts. I should replace IRQ with GSI in the comment
above to make it clearer.

> It also doesn't seem logical to me, considering (yet to be made
> work) pass-through of devices or hot-unplugged ones, at which
> point Dom0 shouldn't retain IRQ bindings, I would think.

Hm, maybe. I think we are still very far from that. Right now GSIs are
bound to a PVH dom0 based on the unmasked vIO-APIC pins, and they are
never unbound. We could see about unbinding them, but TBH I would
expect a PVH dom0 to just mask the vIO-APIC pin when it has no
devices using it if those are unplugged.

Thanks, Roger.