From: Alexey Kardashevskiy <aik@ozlabs.ru>
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
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: [RFC PATCH] PCI: Introduce INTx check & mask API
Date: Thu, 24 May 2012 17:44:22 +1000 [thread overview]
Message-ID: <4FBDE6D6.80700@ozlabs.ru> (raw)
[Found while debugging VFIO on POWER but it is platform independent]
There is a feature in PCI (>=2.3?) to mask/unmask INTx via PCI_COMMAND and
PCI_STATUS registers.
And there is some API to support that (commit a2e27787f893621c5a6b865acf6b7766f8671328).
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+ Stepping- SERR+ FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
pci_intx_mask_supported() reports that the feature is supported for this 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 as VFIO-PCI device.
If I remove the check of this bit, it works fine as it is called from an interrupt handler and
Status bit check is redundant.
Opened a spec:
PCI LOCAL BUS SPECIFICATION, REV. 3.0, Table 6-2: Status Register Bits
===
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’s/function’s INTx# signal be asserted. Setting the Interrupt
Disable bit to a 1 has no effect on the state of this bit.
===
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 masking?
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);
static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask)
{
struct pci_bus *bus = dev->bus;
bool mask_updated = true;
u32 cmd_status_dword;
u16 origcmd, newcmd;
unsigned long flags;
bool irq_pending;
/*
* 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 != PCI_STATUS);
raw_spin_lock_irqsave(&pci_lock, flags);
bus->ops->read(bus, dev->devfn, PCI_COMMAND, 4, &cmd_status_dword);
irq_pending = (cmd_status_dword >> 16) & PCI_STATUS_INTERRUPT;
/*
* 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 != irq_pending) {
+/* if (mask != irq_pending) {
mask_updated = false;
goto done;
- }
+ }*/
origcmd = cmd_status_dword;
newcmd = origcmd & ~PCI_COMMAND_INTX_DISABLE;
if (mask)
newcmd |= PCI_COMMAND_INTX_DISABLE;
if (newcmd != origcmd)
bus->ops->write(bus, dev->devfn, PCI_COMMAND, 2, newcmd);
done:
raw_spin_unlock_irqrestore(&pci_lock, flags);
return mask_updated;
}
/**
* 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);
--
1.7.7.3
WARNING: multiple messages have this Message-ID (diff)
From: Alexey Kardashevskiy <aik@ozlabs.ru>
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
Subject: [Qemu-devel] [RFC PATCH] PCI: Introduce INTx check & mask API
Date: Thu, 24 May 2012 17:44:22 +1000 [thread overview]
Message-ID: <4FBDE6D6.80700@ozlabs.ru> (raw)
[Found while debugging VFIO on POWER but it is platform independent]
There is a feature in PCI (>=2.3?) to mask/unmask INTx via PCI_COMMAND and
PCI_STATUS registers.
And there is some API to support that (commit a2e27787f893621c5a6b865acf6b7766f8671328).
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+ Stepping- SERR+ FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
pci_intx_mask_supported() reports that the feature is supported for this 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 as VFIO-PCI device.
If I remove the check of this bit, it works fine as it is called from an interrupt handler and
Status bit check is redundant.
Opened a spec:
PCI LOCAL BUS SPECIFICATION, REV. 3.0, Table 6-2: Status Register Bits
===
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’s/function’s INTx# signal be asserted. Setting the Interrupt
Disable bit to a 1 has no effect on the state of this bit.
===
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 masking?
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);
static bool pci_check_and_set_intx_mask(struct pci_dev *dev, bool mask)
{
struct pci_bus *bus = dev->bus;
bool mask_updated = true;
u32 cmd_status_dword;
u16 origcmd, newcmd;
unsigned long flags;
bool irq_pending;
/*
* 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 != PCI_STATUS);
raw_spin_lock_irqsave(&pci_lock, flags);
bus->ops->read(bus, dev->devfn, PCI_COMMAND, 4, &cmd_status_dword);
irq_pending = (cmd_status_dword >> 16) & PCI_STATUS_INTERRUPT;
/*
* 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 != irq_pending) {
+/* if (mask != irq_pending) {
mask_updated = false;
goto done;
- }
+ }*/
origcmd = cmd_status_dword;
newcmd = origcmd & ~PCI_COMMAND_INTX_DISABLE;
if (mask)
newcmd |= PCI_COMMAND_INTX_DISABLE;
if (newcmd != origcmd)
bus->ops->write(bus, dev->devfn, PCI_COMMAND, 2, newcmd);
done:
raw_spin_unlock_irqrestore(&pci_lock, flags);
return mask_updated;
}
/**
* 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);
--
1.7.7.3
next reply other threads:[~2012-05-24 7:44 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-24 7:44 Alexey Kardashevskiy [this message]
2012-05-24 7:44 ` [Qemu-devel] [RFC PATCH] PCI: Introduce INTx check & mask API Alexey Kardashevskiy
2012-05-24 12:02 ` Jan Kiszka
2012-05-24 12:02 ` [Qemu-devel] " Jan Kiszka
2012-05-24 14:41 ` Alex Williamson
2012-05-24 14:41 ` [Qemu-devel] " Alex Williamson
2012-05-25 1:06 ` Alexey Kardashevskiy
2012-05-25 1:06 ` [Qemu-devel] " Alexey Kardashevskiy
2012-06-05 1:39 ` Benjamin Herrenschmidt
2012-06-05 1:39 ` [Qemu-devel] " Benjamin Herrenschmidt
2012-06-05 1:44 ` Alexander Graf
2012-06-05 1:44 ` [Qemu-devel] " Alexander Graf
2012-06-05 2:56 ` Benjamin Herrenschmidt
2012-06-05 2:56 ` [Qemu-devel] " Benjamin Herrenschmidt
2012-05-25 1:18 ` Alexey Kardashevskiy
2012-05-25 1:18 ` [Qemu-devel] " Alexey Kardashevskiy
2012-05-25 2:29 ` Jan Kiszka
2012-05-25 2:29 ` [Qemu-devel] " Jan Kiszka
2012-05-25 2:47 ` Alexey Kardashevskiy
2012-05-25 2:47 ` [Qemu-devel] " Alexey Kardashevskiy
2012-05-25 10:43 ` Jan Kiszka
2012-05-25 10:43 ` [Qemu-devel] " Jan Kiszka
2012-05-25 11:26 ` Alexey Kardashevskiy
2012-05-25 11:26 ` [Qemu-devel] " Alexey Kardashevskiy
2012-05-25 11:31 ` Jan Kiszka
2012-05-25 11:31 ` [Qemu-devel] " Jan Kiszka
2012-06-05 1:39 ` Benjamin Herrenschmidt
2012-06-05 1:39 ` [Qemu-devel] " Benjamin Herrenschmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4FBDE6D6.80700@ozlabs.ru \
--to=aik@ozlabs.ru \
--cc=agraf@suse.de \
--cc=alex.williamson@redhat.com \
--cc=benh@kernel.crashing.org \
--cc=david@gibson.dropbear.id.au \
--cc=kvm@vger.kernel.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.