Re: [Xen-devel] [PATCH] interrupts: allow guest to set and clear MSI-X mask bit

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2013-07-18 at 18:44 +0100, Joby Poriyath wrote:
> Guest needs the ability to enable and disable MSI-X interrupts
> by setting the MSI-X control bit. Currently, a write to MSI-X
> mask bit by the guest is silently ignored.
> 
> A likely scenario is where we have a 82599 SR-IOV nic passed
> through to a guest. From the guest if you do
> 
>   ifconfig <ETH_DEV> down
>   ifconfig <ETH_DEV> up
> 
> the interrupts remain masked.  The the mask bit for the VF is
> being set by the PF performing a reset (at the request of the VF).
> However, interrupts are enabled by VF driver by clearing the mask
> bit by writing directly to BAR3 region containing the MSI-X table.
> 
> From dom0, we can verify that
> interrupts are being masked using 'xl debug-keys M'.
> 
> Intially, guest was allowed to modify MSI-X bit.
> Later this behaviour was changed.
> See changeset 74c213c506afcd74a8556dd092995fd4dc38b225.

That commit message says:
    - the interrupt mask bit was permitted to be written by the guest
      (while Xen's interrupt flow control routines need to control it)
I guess it's not entirely clear that simply reversing this is
sufficient. The above doesn't give much to go on but I would have
naÃvely thought that any change to allow the guest to control this bit
would be accompanied by some sort of call to Xen's interrupt flow
control routines.

> -       * As the mask bit is the only defined bit in the word, and as the
> -       * host MSI-X code doesn't preserve the other bits anyway, doing
> -       * this is pointless. So for now just discard the write (also
> -       * saving us from having to determine the matching irq_desc).
> -       */
> -    spin_lock_irqsave(&desc->lock, flags);
> -    orig = readl(virt);
> -    val &= ~PCI_MSIX_VECTOR_BITMASK;
> -    val |= orig & PCI_MSIX_VECTOR_BITMASK;
> +    desc = pirq_spin_lock_irq_desc(entry->pirq, &flags);
> +    if ( !desc )
> +        goto out;
> +
> +    val &= PCI_MSIX_VECTOR_BITMASK;

I think you need to at least retain the bits of the comment which
explain why we don't preserve the other bits, or actually preserve them.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel