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.
