From: Zhenzhong Duan <zhenzhong.duan@oracle.com>
To: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
xen-devel <xen-devel@lists.xen.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Feng Jin <joe.jin@oracle.com>,
Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Subject: [PATCH 2/3 v3] Refactor MSI restore call-chain to drop unnecessary argument
Date: Thu, 29 Aug 2013 10:52:27 +0800 [thread overview]
Message-ID: <521EB76B.8090903@oracle.com> (raw)
Driver init call graph under baremetal:
driver_init->
msix_capability_init->
msix_program_entries->
msix_mask_irq->
entry->masked = 1
request_irq->
__setup_irq->
irq_startup->
unmask_msi_irq->
msix_mask_irq->
entry->masked = 0
So entry->masked is always updated with newest value and its value could be used
to restore to mask register in device.
But in initial domain (aka priviliged guest), it's different.
Driver init call graph under initial domain:
driver_init->
msix_capability_init->
msix_program_entries->
msix_mask_irq->
entry->masked = 1
request_irq->
__setup_irq->
irq_startup->
__startup_pirq->
EVTCHNOP_bind_pirq hypercall (trap into Xen)
[Xen:]
pirq_guest_bind->
startup_msi_irq->
unmask_msi_irq->
msi_set_mask_bit->
entry->msi_attrib.masked = 0
So entry->msi_attrib.masked in xen side always has newest value. entry->masked
in initial domain is untouched and is 1 after msix_capability_init.
Based on above, it's Xen's duty to restore entry->msi_attrib.masked to device,
but with current code, entry->masked is used and MSI-x interrupt is masked.
Before patch, restore call graph under initial domain:
pci_reset_function->
pci_restore_state->
__pci_restore_msix_state->
arch_restore_msi_irqs->
xen_initdom_restore_msi_irqs->
PHYSDEVOP_restore_msi hypercall (first mask restore)
msix_mask_irq(entry, entry->masked) (second mask restore)
So msix_mask_irq call in initial domain call graph needs to be removed.
Based on this we can move the restore of the mask in default_restore_msi_irqs
which would avoid restoring the invalid mask under Xen. Furthermore this
simplifies the API by making everything related to restoring an MSI be in the
platform specific APIs instead of just parts of it.
After patch, restore call graph under initial domain:
pci_reset_function->
pci_restore_state->
__pci_restore_msix_state->
arch_restore_msi_irqs->
xen_initdom_restore_msi_irqs->
PHYSDEVOP_restore_msi hypercall (first mask restore)
Logic for baremetal is not changed.
Before patch, restore call graph under baremetal:
pci_reset_function->
pci_restore_state->
__pci_restore_msix_state->
arch_restore_msi_irqs->
default_restore_msi_irqs->
msix_mask_irq(entry, entry->masked) (first mask restore)
After patch, restore call graph under baremetal:
pci_reset_function->
pci_restore_state->
__pci_restore_msix_state->
arch_restore_msi_irqs->
default_restore_msi_irqs->
msix_mask_irq(entry, entry->masked) (first mask restore)
The process for MSI code is similiar.
-v3: Update patch description per Konrad suggestion, thanks.
Tested-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
---
drivers/pci/msi.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 87223ae..922fb49 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -216,6 +216,8 @@ void unmask_msi_irq(struct irq_data *data)
#ifdef HAVE_DEFAULT_MSI_RESTORE_IRQS
void default_restore_msi_irqs(struct pci_dev *dev, int irq)
{
+ int pos;
+ u16 control;
struct msi_desc *entry;
entry = NULL;
@@ -228,8 +230,19 @@ void default_restore_msi_irqs(struct pci_dev *dev, int irq)
entry = irq_get_msi_desc(irq);
}
- if (entry)
+ if (entry) {
write_msi_msg(irq, &entry->msg);
+ if (dev->msix_enabled) {
+ msix_mask_irq(entry, entry->masked);
+ readl(entry->mask_base);
+ } else {
+ pos = entry->msi_attrib.pos;
+ pci_read_config_word(dev, pos + PCI_MSI_FLAGS,
+ &control);
+ msi_mask_irq(entry, msi_capable_mask(control),
+ entry->masked);
+ }
+ }
}
#endif
@@ -406,7 +419,6 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
arch_restore_msi_irqs(dev, dev->irq);
pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
- msi_mask_irq(entry, msi_capable_mask(control), entry->masked);
control &= ~PCI_MSI_FLAGS_QSIZE;
control |= (entry->msi_attrib.multiple << 4) | PCI_MSI_FLAGS_ENABLE;
pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, control);
@@ -430,7 +442,6 @@ static void __pci_restore_msix_state(struct pci_dev *dev)
list_for_each_entry(entry, &dev->msi_list, list) {
arch_restore_msi_irqs(dev, entry->irq);
- msix_mask_irq(entry, entry->masked);
}
control &= ~PCI_MSIX_FLAGS_MASKALL;
--
1.7.3
next reply other threads:[~2013-08-29 2:52 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-29 2:52 Zhenzhong Duan [this message]
2013-08-29 7:23 ` [Xen-devel] [PATCH 2/3 v3] Refactor MSI restore call-chain to drop unnecessary argument Jan Beulich
2013-08-29 9:50 ` Zhenzhong Duan
2013-08-29 10:47 ` Jan Beulich
2013-08-29 13:31 ` Konrad Rzeszutek Wilk
2013-09-25 22:22 ` Bjorn Helgaas
2013-09-26 1:19 ` Zhenzhong Duan
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=521EB76B.8090903@oracle.com \
--to=zhenzhong.duan@oracle.com \
--cc=bhelgaas@google.com \
--cc=joe.jin@oracle.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=sucheta.chakraborty@qlogic.com \
--cc=xen-devel@lists.xen.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).