Re: [Xen-devel] [PATCH v3 02/10] x86/MSI-X: access MSI-X table only after having enabled MSI-X

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/06/15 12:20, Jan Beulich wrote:
> As done in Linux by f598282f51 ("PCI: Fix the NIU MSI-X problem in a
> better way") and its broken predecessor, make sure we don't access the
> MSI-X table without having enabled MSI-X first, using the mask-all flag
> instead to prevent interrupts from occurring.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, with one suggestion.

> @@ -401,35 +414,38 @@ static bool_t msi_set_mask_bit(struct ir
>          }
>          break;
>      case PCI_CAP_ID_MSIX:
> +        control = pci_conf_read16(seg, bus, slot, func,
> +                                  msix_control_reg(entry->msi_attrib.pos));
> +        if ( unlikely(!(control & PCI_MSIX_FLAGS_ENABLE)) )
> +            pci_conf_write16(seg, bus, slot, func,
> +                             msix_control_reg(entry->msi_attrib.pos),
> +                             control | (PCI_MSIX_FLAGS_ENABLE |
> +                                        PCI_MSIX_FLAGS_MASKALL));
>          if ( likely(memory_decoded(pdev)) )
>          {
>              writel(flag, entry->mask_base + 
> PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
>              readl(entry->mask_base + PCI_MSIX_ENTRY_VECTOR_CTRL_OFFSET);
> -            break;
> +            if ( likely(control & PCI_MSIX_FLAGS_ENABLE) )
> +                break;
> +            flag = 1;
>          }
> -        if ( flag )
> +        else if ( flag && !(control & PCI_MSIX_FLAGS_MASKALL) )
>          {
> -            u16 control;
>              domid_t domid = pdev->domain->domain_id;
>  
> -            control = pci_conf_read16(seg, bus, slot, func,
> -                                      
> msix_control_reg(entry->msi_attrib.pos));
> -            if ( control & PCI_MSIX_FLAGS_MASKALL )
> -                break;
> -            pci_conf_write16(seg, bus, slot, func,
> -                             msix_control_reg(entry->msi_attrib.pos),
> -                             control | PCI_MSIX_FLAGS_MASKALL);
> +            control |= PCI_MSIX_FLAGS_MASKALL;
>              if ( pdev->msix->warned != domid )
>              {
>                  pdev->msix->warned = domid;
>                  printk(XENLOG_G_WARNING
> -                       "cannot mask IRQ %d: masked MSI-X on Dom%d's 
> %04x:%02x:%02x.%u\n",
> +                       "cannot mask IRQ %d: masking MSI-X on Dom%d's 
> %04x:%02x:%02x.%u\n",

"masking all", which is a more clear statement.

~Andrew

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