From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexey Kardashevskiy <aik@ozlabs.ru>
Subject: [RFC PATCH] PCI: Introduce INTx check & mask API
Date: Thu, 24 May 2012 17:44:22 +1000
Message-ID: <4FBDE6D6.80700@ozlabs.ru>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Alex Williamson <alex.williamson@redhat.com>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Alex Williamson <alex.williamson@redhat.com>,
	David Gibson <david@gibson.dropbear.id.au>,
	Alex Graf <agraf@suse.de>, kvm@vger.kernel.org,
	qemu-devel@nongnu.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-gg0-f174.google.com ([209.85.161.174]:34679 "EHLO
	mail-gg0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752759Ab2EXHoh (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 24 May 2012 03:44:37 -0400
Received: by gglu4 with SMTP id u4so7414623ggl.19
        for <kvm@vger.kernel.org>; Thu, 24 May 2012 00:44:31 -0700 (PDT)
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

[Found while debugging VFIO on POWER but it is platform independent]

There is a feature in PCI (>=3D2.3?) to mask/unmask INTx via PCI_COMMAN=
D and
PCI_STATUS registers.

And there is some API to support that (commit a2e27787f893621c5a6b865ac=
f6b7766f8671328).

I have a network adapter:
0001:00:01.0 Ethernet controller: Chelsio Communications Inc T310 10GbE=
 Single Port Adapter
	Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ St=
epping- SERR+ FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=3Dfast >TAbort- <TAbo=
rt- <MAbort- >SERR- <PERR- INTx-

pci_intx_mask_supported() reports that the feature is supported for thi=
s adapter
BUT the adapter does not set PCI_STATUS_INTERRUPT so pci_check_and_set_=
intx_mask()
never changes PCI_COMMAND and INTx does not work on it when we use it a=
s VFIO-PCI device.

If I remove the check of this bit, it works fine as it is called from a=
n interrupt handler and
Status bit check is redundant.

Opened a spec:
PCI LOCAL BUS SPECIFICATION, REV. 3.0, Table 6-2: Status Register Bits
=3D=3D=3D
3	This read-only bit reflects the state of the interrupt in the
device/function. Only when the Interrupt Disable bit in the command
register is a 0 and this Interrupt Status bit is a 1, will the
device=E2=80=99s/function=E2=80=99s INTx# signal be asserted. Setting t=
he Interrupt
   Disable bit to a 1 has no effect on the state of this bit.
=3D=3D=3D
With this adapter, INTx# is asserted but Status bit is still 0.

Is it mandatory for a device to set Status bit if it supports INTx mask=
ing?

2 Alex: if it is mandatory, then we need to be able to disable pci_2_3 =
in VFIO-PCI
somehow.


Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 drivers/pci/pci.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index ab6c2a6..ee4c804 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2978,60 +2978,60 @@ EXPORT_SYMBOL_GPL(pci_intx_mask_supported);
=20
 static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask=
)
 {
 	struct pci_bus *bus =3D dev->bus;
 	bool mask_updated =3D true;
 	u32 cmd_status_dword;
 	u16 origcmd, newcmd;
 	unsigned long flags;
 	bool irq_pending;
=20
 	/*
 	 * We do a single dword read to retrieve both command and status.
 	 * Document assumptions that make this possible.
 	 */
 	BUILD_BUG_ON(PCI_COMMAND % 4);
 	BUILD_BUG_ON(PCI_COMMAND + 2 !=3D PCI_STATUS);
=20
 	raw_spin_lock_irqsave(&pci_lock, flags);
=20
 	bus->ops->read(bus, dev->devfn, PCI_COMMAND, 4, &cmd_status_dword);
=20
 	irq_pending =3D (cmd_status_dword >> 16) & PCI_STATUS_INTERRUPT;
=20
 	/*
 	 * Check interrupt status register to see whether our device
 	 * triggered the interrupt (when masking) or the next IRQ is
 	 * already pending (when unmasking).
 	 */
-	if (mask !=3D irq_pending) {
+/*	if (mask !=3D irq_pending) {
 		mask_updated =3D false;
 		goto done;
-	}
+	}*/
=20
 	origcmd =3D cmd_status_dword;
 	newcmd =3D origcmd & ~PCI_COMMAND_INTX_DISABLE;
 	if (mask)
 		newcmd |=3D PCI_COMMAND_INTX_DISABLE;
 	if (newcmd !=3D origcmd)
 		bus->ops->write(bus, dev->devfn, PCI_COMMAND, 2, newcmd);
=20
 done:
 	raw_spin_unlock_irqrestore(&pci_lock, flags);
=20
 	return mask_updated;
 }
=20
 /**
  * pci_check_and_mask_intx - mask INTx on pending interrupt
  * @dev: the PCI device to operate on
  *
  * Check if the device dev has its INTx line asserted, mask it and
  * return true in that case. False is returned if not interrupt was
  * pending.
  */
 bool pci_check_and_mask_intx(struct pci_dev *dev)
 {
 	return pci_check_and_set_intx_mask(dev, true);
 }
 EXPORT_SYMBOL_GPL(pci_check_and_mask_intx);
=20
--=20
1.7.7.3