From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: Alex Williamson <alex.williamson@redhat.com>,
"Raj, Ashok" <ashok.raj@intel.com>,
"David S. Miller" <davem@davemloft.net>,
Bjorn Helgaas <bhelgaas@google.com>,
linux-pci@vger.kernel.org, Kevin Tian <kevin.tian@intel.com>,
Marc Zyngier <maz@kernel.org>, Ingo Molnar <mingo@kernel.org>,
x86@kernel.org, linux-s390@vger.kernel.org,
Niklas Schnelle <schnelle@linux.ibm.com>,
Gerald Schaefer <gerald.schaefer@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Christian Borntraeger <borntraeger@de.ibm.com>
Subject: [patch V2 02/19] PCI/MSI: Mask all unused MSI-X entries
Date: Thu, 29 Jul 2021 23:51:41 +0200 [thread overview]
Message-ID: <20210729222542.403833459@linutronix.de> (raw)
In-Reply-To: 20210729215139.889204656@linutronix.de
From: Thomas Gleixner <tglx@linutronix.de>
When MSI-X is enabled the ordering of calls is:
msix_map_region();
msix_setup_entries();
pci_msi_setup_msi_irqs();
msix_program_entries();
This has a few interesting issues:
1) msix_setup_entries() allocates the MSI descriptors and initializes them
except for the msi_desc:masked member which is left zero initialized.
2) pci_msi_setup_msi_irqs() allocates the interrupt descriptors and sets
up the MSI interrupts which ends up in pci_write_msi_msg() unless the
interrupt chip provides its own irq_write_msi_msg() function.
3) msix_program_entries() does not do what the name suggests. It solely
updates the entries array (if not NULL) and initializes the masked
member for each MSI descriptor by reading the hardware state and then
masks the entry.
Obviously this has some issues:
1) The uninitialized masked member of msi_desc prevents the enforcement
of masking the entry in pci_write_msi_msg() depending on the cached
masked bit. Aside of that half initialized data is a NONO in general
2) msix_program_entries() only ensures that the actually allocated entries
are masked. This is wrong as experimentation with crash testing and
crash kernel kexec has shown.
This limited testing unearthed that when the production kernel had more
entries in use and unmasked when it crashed and the crash kernel
allocated a smaller amount of entries, then a full scan of all entries
found unmasked entries which were in use in the production kernel.
This is obviously a device or emulation issue as the device reset
should mask all MSI-X table entries, but obviously that's just part
of the paper specification.
Cure this by:
1) Masking all table entries in hardware
2) Initializing msi_desc::masked in msix_setup_entries()
3) Removing the mask dance in msix_program_entries()
4) Renaming msix_program_entries() to msix_update_entries() to
reflect the purpose of that function.
As the masking of unused entries has never been done the Fixes tag refers
to a commit in:
git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
Fixes: f036d4ea5fa7 ("[PATCH] ia32 Message Signalled Interrupt support")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Marc Zyngier <maz@kernel.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/msi.c | 45 +++++++++++++++++++++++++++------------------
1 file changed, 27 insertions(+), 18 deletions(-)
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -691,6 +691,7 @@ static int msix_setup_entries(struct pci
{
struct irq_affinity_desc *curmsk, *masks = NULL;
struct msi_desc *entry;
+ void __iomem *addr;
int ret, i;
int vec_count = pci_msix_vec_count(dev);
@@ -711,6 +712,7 @@ static int msix_setup_entries(struct pci
entry->msi_attrib.is_msix = 1;
entry->msi_attrib.is_64 = 1;
+
if (entries)
entry->msi_attrib.entry_nr = entries[i].entry;
else
@@ -722,6 +724,10 @@ static int msix_setup_entries(struct pci
entry->msi_attrib.default_irq = dev->irq;
entry->mask_base = base;
+ addr = pci_msix_desc_addr(entry);
+ if (addr)
+ entry->masked = readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
+
list_add_tail(&entry->list, dev_to_msi_list(&dev->dev));
if (masks)
curmsk++;
@@ -732,26 +738,25 @@ static int msix_setup_entries(struct pci
return ret;
}
-static void msix_program_entries(struct pci_dev *dev,
- struct msix_entry *entries)
+static void msix_update_entries(struct pci_dev *dev, struct msix_entry *entries)
{
struct msi_desc *entry;
- int i = 0;
- void __iomem *desc_addr;
for_each_pci_msi_entry(entry, dev) {
- if (entries)
- entries[i++].vector = entry->irq;
+ if (entries) {
+ entries->vector = entry->irq;
+ entries++;
+ }
+ }
+}
- desc_addr = pci_msix_desc_addr(entry);
- if (desc_addr)
- entry->masked = readl(desc_addr +
- PCI_MSIX_ENTRY_VECTOR_CTRL);
- else
- entry->masked = 0;
+static void msix_mask_all(void __iomem *base, int tsize)
+{
+ u32 ctrl = PCI_MSIX_ENTRY_CTRL_MASKBIT;
+ int i;
- msix_mask_irq(entry, 1);
- }
+ for (i = 0; i < tsize; i++, base += PCI_MSIX_ENTRY_SIZE)
+ writel(ctrl, base + PCI_MSIX_ENTRY_VECTOR_CTRL);
}
/**
@@ -768,9 +773,9 @@ static void msix_program_entries(struct
static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
int nvec, struct irq_affinity *affd)
{
- int ret;
- u16 control;
void __iomem *base;
+ int ret, tsize;
+ u16 control;
/*
* Some devices require MSI-X to be enabled before the MSI-X
@@ -782,12 +787,16 @@ static int msix_capability_init(struct p
pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
/* Request & Map MSI-X table region */
- base = msix_map_region(dev, msix_table_size(control));
+ tsize = msix_table_size(control);
+ base = msix_map_region(dev, tsize);
if (!base) {
ret = -ENOMEM;
goto out_disable;
}
+ /* Ensure that all table entries are masked. */
+ msix_mask_all(base, tsize);
+
ret = msix_setup_entries(dev, base, entries, nvec, affd);
if (ret)
goto out_disable;
@@ -801,7 +810,7 @@ static int msix_capability_init(struct p
if (ret)
goto out_free;
- msix_program_entries(dev, entries);
+ msix_update_entries(dev, entries);
ret = populate_msi_sysfs(dev);
if (ret)
next prev parent reply other threads:[~2021-07-29 22:34 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-29 21:51 [patch V2 00/19] PCI/MSI, x86: Cure a couple of inconsistencies Thomas Gleixner
2021-07-29 21:51 ` [patch V2 01/19] PCI/MSI: Enable and mask MSI-X early Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` Thomas Gleixner [this message]
2021-08-10 9:07 ` [tip: irq/core] PCI/MSI: Mask all unused MSI-X entries tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 03/19] PCI/MSI: Enforce that MSI-X table entry is masked for update Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 04/19] PCI/MSI: Enforce MSI[X] entry updates to be visible Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 05/19] PCI/MSI: Do not set invalid bits in MSI mask Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 06/19] PCI/MSI: Correct misleading comments Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 07/19] PCI/MSI: Use msi_mask_irq() in pci_msi_shutdown() Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 08/19] PCI/MSI: Protect msi_desc::masked for multi-MSI Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 09/19] genirq: Provide IRQCHIP_AFFINITY_PRE_STARTUP Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 10/19] x86/ioapic: Force affinity setup before startup Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 11/19] x86/msi: " Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 12/19] s390/pci: Do not mask MSI[-X] entries on teardown Thomas Gleixner
2021-08-03 12:48 ` Niklas Schnelle
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 13/19] PCI/MSI: Simplify msi_verify_entries() Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 14/19] PCI/MSI: Rename msi_desc::masked Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 15/19] PCI/MSI: Consolidate error handling in msi_capability_init() Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 16/19] PCI/MSI: Deobfuscate virtual MSI-X Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 17/19] PCI/MSI: Cleanup msi_mask() Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 18/19] PCI/MSI: Provide a new set of mask and unmask functions Thomas Gleixner
[not found] ` <87r1f6bpt7.wl-maz@kernel.org>
2021-08-09 18:56 ` Thomas Gleixner
2021-08-09 19:08 ` [patch V3 " Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-07-29 21:51 ` [patch V2 19/19] PCI/MSI: Use new mask/unmask functions Thomas Gleixner
2021-08-10 9:07 ` [tip: irq/core] " tip-bot2 for Thomas Gleixner
2021-08-10 7:49 ` [patch V2 00/19] PCI/MSI, x86: Cure a couple of inconsistencies Marc Zyngier
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=20210729222542.403833459@linutronix.de \
--to=tglx@linutronix.de \
--cc=alex.williamson@redhat.com \
--cc=ashok.raj@intel.com \
--cc=bhelgaas@google.com \
--cc=borntraeger@de.ibm.com \
--cc=davem@davemloft.net \
--cc=gerald.schaefer@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=maz@kernel.org \
--cc=mingo@kernel.org \
--cc=schnelle@linux.ibm.com \
--cc=x86@kernel.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).