| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20190131223932.GR229773@google.com>
Date: Thu, 31 Jan 2019 16:39:32 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Logan Gunthorpe <logang@...tatee.com>
Cc: linux-kernel@...r.kernel.org, linux-ntb@...glegroups.com,
linux-pci@...r.kernel.org, iommu@...ts.linux-foundation.org,
linux-kselftest@...r.kernel.org, Jon Mason <jdmason@...zu.us>,
Joerg Roedel <joro@...tes.org>,
Allen Hubbe <allenbh@...il.com>,
Dave Jiang <dave.jiang@...el.com>,
Serge Semin <fancer.lancer@...il.com>,
Eric Pilmore <epilmore@...aio.com>,
Thomas Gleixner <tglx@...utronix.de>,
Marc Zyngier <marc.zyngier@....com>
Subject: Re: [PATCH 2/9] PCI/MSI: Support allocating virtual MSI interrupts
[+cc Thomas, Marc]
On Thu, Jan 31, 2019 at 11:56:49AM -0700, Logan Gunthorpe wrote:
> For NTB devices, we want to be able to trigger MSI interrupts
> through a memory window. In these cases we may want to use
> more interrupts than the NTB PCI device has available in its MSI-X
> table.
>
> We allow for this by creating a new 'virtual' interrupt. These
> interrupts are allocated as usual but are not programmed into the
> MSI-X table (as there may not be space for them).
>
> The MSI address and data will then handled through an NTB MSI library
> introduced later in this series.
>
> Signed-off-by: Logan Gunthorpe <logang@...tatee.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
I assume you'll merge this along with the rest of the series, so:
Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>
Minor question and typo below.
> ---
> drivers/pci/msi.c | 51 +++++++++++++++++++++++++++++++++++++--------
> include/linux/msi.h | 1 +
> include/linux/pci.h | 9 ++++++++
> 3 files changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
> index 4c0b47867258..145587da686c 100644
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -192,6 +192,9 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
>
> static void __iomem *pci_msix_desc_addr(struct msi_desc *desc)
> {
> + if (desc->msi_attrib.is_virtual)
> + return NULL;
> +
> return desc->mask_base +
> desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE;
> }
> @@ -206,14 +209,19 @@ static void __iomem *pci_msix_desc_addr(struct msi_desc *desc)
> u32 __pci_msix_desc_mask_irq(struct msi_desc *desc, u32 flag)
> {
> u32 mask_bits = desc->masked;
> + void __iomem *desc_addr;
>
> if (pci_msi_ignore_mask)
> return 0;
> + desc_addr = pci_msix_desc_addr(desc);
> + if (!desc_addr)
> + return 0;
>
> mask_bits &= ~PCI_MSIX_ENTRY_CTRL_MASKBIT;
> if (flag)
> mask_bits |= PCI_MSIX_ENTRY_CTRL_MASKBIT;
> - writel(mask_bits, pci_msix_desc_addr(desc) + PCI_MSIX_ENTRY_VECTOR_CTRL);
> +
> + writel(mask_bits, desc_addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
>
> return mask_bits;
> }
> @@ -273,6 +281,11 @@ void __pci_read_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> if (entry->msi_attrib.is_msix) {
> void __iomem *base = pci_msix_desc_addr(entry);
>
> + if (!base) {
> + WARN_ON(1);
> + return;
> + }
> +
> msg->address_lo = readl(base + PCI_MSIX_ENTRY_LOWER_ADDR);
> msg->address_hi = readl(base + PCI_MSIX_ENTRY_UPPER_ADDR);
> msg->data = readl(base + PCI_MSIX_ENTRY_DATA);
> @@ -303,6 +316,9 @@ void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> } else if (entry->msi_attrib.is_msix) {
> void __iomem *base = pci_msix_desc_addr(entry);
>
> + if (!base)
> + goto skip;
> +
> writel(msg->address_lo, base + PCI_MSIX_ENTRY_LOWER_ADDR);
> writel(msg->address_hi, base + PCI_MSIX_ENTRY_UPPER_ADDR);
> writel(msg->data, base + PCI_MSIX_ENTRY_DATA);
> @@ -327,6 +343,8 @@ void __pci_write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> msg->data);
> }
> }
> +
> +skip:
> entry->msg = *msg;
> }
>
> @@ -550,6 +568,7 @@ msi_setup_entry(struct pci_dev *dev, int nvec, const struct irq_affinity *affd)
>
> entry->msi_attrib.is_msix = 0;
> entry->msi_attrib.is_64 = !!(control & PCI_MSI_FLAGS_64BIT);
> + entry->msi_attrib.is_virtual = 0;
> entry->msi_attrib.entry_nr = 0;
> entry->msi_attrib.maskbit = !!(control & PCI_MSI_FLAGS_MASKBIT);
> entry->msi_attrib.default_irq = dev->irq; /* Save IOAPIC IRQ */
> @@ -674,6 +693,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
> struct irq_affinity_desc *curmsk, *masks = NULL;
> struct msi_desc *entry;
> int ret, i;
> + int vec_count = pci_msix_vec_count(dev);
>
> if (affd)
> masks = irq_create_affinity_masks(nvec, affd);
> @@ -696,6 +716,10 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
> entry->msi_attrib.entry_nr = entries[i].entry;
> else
> entry->msi_attrib.entry_nr = i;
> +
> + entry->msi_attrib.is_virtual =
> + entry->msi_attrib.entry_nr >= vec_count;
> +
> entry->msi_attrib.default_irq = dev->irq;
> entry->mask_base = base;
>
> @@ -714,12 +738,19 @@ static void msix_program_entries(struct pci_dev *dev,
> {
> 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;
> - entry->masked = readl(pci_msix_desc_addr(entry) +
> - PCI_MSIX_ENTRY_VECTOR_CTRL);
> +
> + desc_addr = pci_msix_desc_addr(entry);
> + if (desc_addr)
> + entry->masked = readl(desc_addr +
> + PCI_MSIX_ENTRY_VECTOR_CTRL);
> + else
> + entry->masked = 0;
> +
> msix_mask_irq(entry, 1);
> }
> }
> @@ -932,7 +963,8 @@ int pci_msix_vec_count(struct pci_dev *dev)
> EXPORT_SYMBOL(pci_msix_vec_count);
>
> static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
> - int nvec, const struct irq_affinity *affd)
> + int nvec, const struct irq_affinity *affd,
> + int flags)
> {
> int nr_entries;
> int i, j;
> @@ -943,7 +975,7 @@ static int __pci_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
> nr_entries = pci_msix_vec_count(dev);
> if (nr_entries < 0)
> return nr_entries;
> - if (nvec > nr_entries)
> + if (nvec > nr_entries && !(flags & PCI_IRQ_VIRTUAL))
> return nr_entries;
>
> if (entries) {
> @@ -1086,7 +1118,8 @@ EXPORT_SYMBOL(pci_enable_msi);
>
> static int __pci_enable_msix_range(struct pci_dev *dev,
> struct msix_entry *entries, int minvec,
> - int maxvec, const struct irq_affinity *affd)
> + int maxvec, const struct irq_affinity *affd,
> + int flags)
> {
> int rc, nvec = maxvec;
>
> @@ -1110,7 +1143,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
> return -ENOSPC;
> }
>
> - rc = __pci_enable_msix(dev, entries, nvec, affd);
> + rc = __pci_enable_msix(dev, entries, nvec, affd, flags);
> if (rc == 0)
> return nvec;
>
> @@ -1141,7 +1174,7 @@ static int __pci_enable_msix_range(struct pci_dev *dev,
> int pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries,
> int minvec, int maxvec)
> {
> - return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL);
> + return __pci_enable_msix_range(dev, entries, minvec, maxvec, NULL, 0);
> }
> EXPORT_SYMBOL(pci_enable_msix_range);
>
> @@ -1181,7 +1214,7 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
>
> if (flags & PCI_IRQ_MSIX) {
> msix_vecs = __pci_enable_msix_range(dev, NULL, min_vecs,
> - max_vecs, affd);
> + max_vecs, affd, flags);
> if (msix_vecs > 0)
> return msix_vecs;
> }
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 784fb52b9900..6458ab049852 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -88,6 +88,7 @@ struct msi_desc {
> __u8 multi_cap : 3;
> __u8 maskbit : 1;
> __u8 is_64 : 1;
> + __u8 is_virtual : 1;
You did the right thing by using the same style as what's already
here, but does anybody know why are we using __u8 and __u16 here?
Those typedefs are in include/uapi/asm-generic/int-l64.h, which
suggests they're for things exported to user space, but I don't think
that's the case here, so I'm wondering if we could someday replace
these with u8 and u16. Obviously that wouldn't be part of *this*
series.
> __u16 entry_nr;
> unsigned default_irq;
> } msi_attrib;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 65f1d8c2f082..ce0815c2c498 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1352,6 +1352,15 @@ int pci_set_vga_state(struct pci_dev *pdev, bool decode,
> #define PCI_IRQ_MSI (1 << 1) /* Allow MSI interrupts */
> #define PCI_IRQ_MSIX (1 << 2) /* Allow MSI-X interrupts */
> #define PCI_IRQ_AFFINITY (1 << 3) /* Auto-assign affinity */
> +
> +/*
> + * Virtual interrupts allow for more interrupts to be allocated
> + * than the device has interrupts for. These are not programmed
> + * into the devices MSI-X table and must be handled by some
s/devices/device's/
> + * other driver means.
> + */
> +#define PCI_IRQ_VIRTUAL (1 << 4)
> +
> #define PCI_IRQ_ALL_TYPES \
> (PCI_IRQ_LEGACY | PCI_IRQ_MSI | PCI_IRQ_MSIX)
>
> --
> 2.19.0
>
Powered by blists - more mailing lists