From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v3 02/10] x86/MSI-X: access MSI-X table only
 after having enabled MSI-X
Date: Fri, 5 Jun 2015 14:01:02 +0100
Message-ID: <55719D8E.1090801@citrix.com>
References: <55719F9D0200007800081425@mail.emea.novell.com>
	<5571A2300200007800081455@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta5.messagelabs.com ([195.245.231.135])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=5913fcd06=Andrew.Cooper3@citrix.com>)
	id 1Z0rFG-0007Kf-Sy
	for xen-devel@lists.xenproject.org; Fri, 05 Jun 2015 13:01:10 +0000
In-Reply-To: <5571A2300200007800081455@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, xen-devel <xen-devel@lists.xenproject.org>
Cc: Keir Fraser <keir@xen.org>
List-Id: xen-devel@lists.xenproject.org

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@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, 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