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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.08.13 at 18:12, Joby Poriyath <joby.poriyath@xxxxxxxxxx> wrote:
> +static struct msi_desc *virt_to_msi_desc(struct pci_dev *dev, void *virt)
> +{
> +    struct msi_desc *desc;
> +
> +    list_for_each_entry( desc, &dev->msi_list, list )
> +        if ( desc->msi_attrib.type == PCI_CAP_ID_MSIX  &&
> +             desc->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET 
> +                == virt )

To match the function's name I'd prefer this to hit on the full 16-byte
range rather than just the control word.

>  static int msixtbl_write(struct vcpu *v, unsigned long address,
> -                        unsigned long len, unsigned long val)
> +                         unsigned long len, unsigned long val)
>  {
>      unsigned long offset;
>      struct msixtbl_entry *entry;
> +    struct msi_desc *m_desc;

Please name this msi_desc, consistent with other variables of this
type in this file. And afaict this could (once again) be const.

> +    m_desc = virt_to_msi_desc(entry->pdev, virt);
> +    if ( !m_desc || m_desc->irq < 0 )
> +        goto out;
> +    
> +    desc = irq_to_desc(m_desc->irq);
> +    if ( !desc )
> +        goto out;
> +
>      spin_lock_irqsave(&desc->lock, flags);
> +
> +    if ( !desc->msi_desc )
> +        goto unlock;

I'd again strongly recommend adding an ASSERT() here, checking
desc->msi_desc against (as it's currently named) m_desc.

But overall this looks much better than the earlier, no reverted
variant.

Jan


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