Re: [Xen-devel] [PATCH v7 for-next 10/12] vpci/msi: add MSI handlers

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

On Mon, Jan 22, 2018 at 05:58:24AM -0700, Jan Beulich wrote:
> >>> On 22.01.18 at 13:48, <roger.pau@xxxxxxxxxx> wrote:
> > I think the proper way to solve this is to reset the mask bits to
> > masked when the vector is unbound, so that at bind time the state of
> > the mask is consistent regardless of whether the vector has been
> > previously bound or not. The following patch should fix this:
> > 
> > diff --git a/xen/drivers/passthrough/io.c b/xen/drivers/passthrough/io.c
> > index 8f16e6c0a5..bab3aa349a 100644
> > --- a/xen/drivers/passthrough/io.c
> > +++ b/xen/drivers/passthrough/io.c
> > @@ -645,7 +645,22 @@ int pt_irq_destroy_bind(
> >          }
> >          break;
> >      case PT_IRQ_TYPE_MSI:
> > +    {
> > +        unsigned long flags;
> > +        struct irq_desc *desc = domain_spin_lock_irq_desc(d, machine_gsi,
> > +                                                          &flags);
> > +
> > +        if ( !desc )
> > +            return -EINVAL;
> > +        /*
> > +         * Leave the MSI masked, so that the state when calling
> > +         * pt_irq_create_bind is consistent across bind/unbinds.
> > +         */
> > +        guest_mask_msi_irq(desc, true);
> > +        spin_unlock_irqrestore(&desc->lock, flags);
> >          break;
> > +    }
> > +
> >      default:
> >          return -EOPNOTSUPP;
> >      }
> > 
> > I think this should be send as a separate patch of this series, since
> > it's a fix for pt_irq_destroy_bind.
> 
> Looks plausible, but I'll defer my ack until I've also seen the
> description for it, because if the above is really necessary I'd
> sort of expect there to be an actual issue without any of your
> series applied.

We haven't seen any issues because the passthrough code in QEMU
doesn't support multiple vectors with MSI, and in any case QEMU
always uses the newly introduced XEN_PT_GFLAGSSHIFT_UNMASKED when
binding interrupts, so it's always unmasked.

I can formally submit the above, but I'm afraid the arguments I have
for getting it committed are 'it's more correct', because the masking
state of the vector would always be masked when unbound and after
binding if no unmask is requested. With the current code the masking
state is much more hard to track.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel