| 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: <Y2xwbYEKKc9KXUDy@iweiny-desk3>
Date: Wed, 9 Nov 2022 19:30:53 -0800
From: Ira Weiny <ira.weiny@...el.com>
To: Jonathan Cameron <Jonathan.Cameron@...wei.com>
CC: Davidlohr Bueso <dave@...olabs.net>,
Bjorn Helgaas <helgaas@...nel.org>,
Dan Williams <dan.j.williams@...el.com>,
<dave.jiang@...el.com>, <alison.schofield@...el.com>,
<bwidawsk@...nel.org>, <vishal.l.verma@...el.com>,
<a.manzanares@...sung.com>, <linux-cxl@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
<linux-pci@...r.kernel.org>, Christoph Hellwig <hch@....de>
Subject: Re: [PATCH 1/2] cxl/pci: Add generic MSI-X/MSI irq support
On Thu, Nov 03, 2022 at 06:09:16PM +0000, Jonathan Cameron wrote:
> On Wed, 2 Nov 2022 17:18:33 -0700
> Davidlohr Bueso <dave@...olabs.net> wrote:
>
> > On Wed, 02 Nov 2022, Ira Weiny wrote:
> >
> > >On Wed, Nov 02, 2022 at 10:15:24AM -0700, Davidlohr Bueso wrote:
> > >> Most CXL features that can have irqs will normally use only the first 16,
> > >> with the exception of isolation (cxl 3.0), which per the spec is up to 32.
> > >
> > >Dan, Dave, and I were discussing this and we agree. For now the only things
> > >people are working on are within the first 16 so why not just request 16 as the
> > >max for now?
> >
> > It is a fair compromise, yes.
>
> works for me.
I made what I thought would be a simple change to your patch and built this
into my series.
Unfortunately the following does not work with the current Qemu.
/*
* NOTE: Currently all the functions which are enabled for CXL require their
* vectors to be in the first 16. Allocate this number as the min/max.
*/
#define CXL_PCI_REQUIRED_VECTORS 16
...
rc = pci_alloc_irq_vectors(pdev, CXL_PCI_REQUIRED_VECTORS,
CXL_PCI_REQUIRED_VECTORS,
PCI_IRQ_MSIX | PCI_IRQ_MSI);
This is because Qemu CXL devices only support (with the event changes I have
made) 8 msg numbers. So the code fails to allocate any vectors.
I guess I should have known better. But allocating something less than 16 I
guess needs to be allowed.
But that also means that beyond knowing _if_ irq's have been enabled I think
each CXL feature needs to know the number of vectors allocated so they can
ensure their msg numbers are going to work.
So how about the following as a diff to this patch?
In the event code I have then used the nr_irq_vecs field to determine if I
should enable the irq for each log.
If you are ok with it I'm going to squash it into your patch and send out a new
version of the event log series.
Thanks,
Ira
>From 105561243c800442a2b7ff39b931e73b0a89bc34 Mon Sep 17 00:00:00 2001
From: Ira Weiny <ira.weiny@...el.com>
Date: Wed, 9 Nov 2022 12:35:07 -0800
Subject: [PATCH] squash: Allocate up to a static 16 vectors.
This covers the current desired features which CXL needs now.
---
drivers/cxl/cxlmem.h | 4 +--
drivers/cxl/cxlpci.h | 6 ++++
drivers/cxl/pci.c | 68 +++++++++-----------------------------------
3 files changed, 22 insertions(+), 56 deletions(-)
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 78ff6dca3c4b..03da4f8f74d3 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -212,7 +212,7 @@ struct cxl_endpoint_dvsec_info {
* @info: Cached DVSEC information about the device.
* @serial: PCIe Device Serial Number
* @doe_mbs: PCI DOE mailbox array
- * @has_irq: PCIe MSI-X/MSI support
+ * @nr_irq_vecs: Number of MSI-X/MSI vectors available
* @mbox_send: @dev specific transport for transmitting mailbox commands
*
* See section 8.2.9.5.2 Capacity Configuration and Label Storage for
@@ -249,7 +249,7 @@ struct cxl_dev_state {
struct xarray doe_mbs;
- bool has_irq;
+ int nr_irq_vecs;
int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
};
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index eec597dbe763..b7f4e2f417d3 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -53,6 +53,12 @@
#define CXL_DVSEC_REG_LOCATOR_BLOCK_ID_MASK GENMASK(15, 8)
#define CXL_DVSEC_REG_LOCATOR_BLOCK_OFF_LOW_MASK GENMASK(31, 16)
+/*
+ * NOTE: Currently all the functions which are enabled for CXL require their
+ * vectors to be in the first 16. Use this as the max.
+ */
+#define CXL_PCI_REQUIRED_VECTORS 16
+
/* Register Block Identifier (RBI) */
enum cxl_regloc_type {
CXL_REGLOC_RBI_EMPTY = 0,
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 9dc32b802594..e0d511575b45 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -428,71 +428,34 @@ static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
}
}
-/**
- * struct cxl_irq_cap - CXL feature that is capable of receiving MSI-X/MSI irqs.
- *
- * @name: Name of the device/component generating this interrupt.
- * @get_max_msgnum: Get the feature's largest interrupt message number. If the
- * feature does not have the Interrupt Supported bit set, then
- * return -1.
- */
-struct cxl_irq_cap {
- const char *name;
- int (*get_max_msgnum)(struct cxl_dev_state *cxlds);
-};
-
-static const struct cxl_irq_cap cxl_irq_cap_table[] = {
- NULL
-};
-
static void cxl_pci_free_irq_vectors(void *data)
{
pci_free_irq_vectors(data);
}
-/*
- * Attempt to allocate the largest amount of necessary vectors.
- *
- * Returns 0 upon a successful allocation of *all* vectors, or a
- * negative value otherwise.
- */
-static int cxl_pci_alloc_irq_vectors(struct cxl_dev_state *cxlds)
+static void cxl_pci_alloc_irq_vectors(struct cxl_dev_state *cxlds)
{
struct device *dev = cxlds->dev;
struct pci_dev *pdev = to_pci_dev(dev);
- int rc, i, vectors = -1;
-
- for (i = 0; i < ARRAY_SIZE(cxl_irq_cap_table); i++) {
- int irq;
-
- if (!cxl_irq_cap_table[i].get_max_msgnum)
- continue;
-
- irq = cxl_irq_cap_table[i].get_max_msgnum(cxlds);
- vectors = max_t(int, irq, vectors);
- }
-
- /*
- * Semantically lack of irq support is not an error, but we
- * still fail to allocate, so return negative.
- */
- if (vectors == -1)
- return -1;
+ int nvecs;
+ int rc;
- vectors++;
- rc = pci_alloc_irq_vectors(pdev, vectors, vectors,
+ nvecs = pci_alloc_irq_vectors(pdev, 1, CXL_PCI_REQUIRED_VECTORS,
PCI_IRQ_MSIX | PCI_IRQ_MSI);
- if (rc < 0)
- return rc;
-
- if (rc != vectors) {
+ if (nvecs < 0) {
dev_dbg(dev, "Not enough interrupts; use polling instead.\n");
+ return;
+ }
+
+ rc = devm_add_action_or_reset(dev, cxl_pci_free_irq_vectors, pdev);
+ if (rc) {
+ dev_dbg(dev, "Device managed call failed; interrupts disabled.\n");
/* some got allocated, clean them up */
cxl_pci_free_irq_vectors(pdev);
- return -ENOSPC;
+ return;
}
- return devm_add_action_or_reset(dev, cxl_pci_free_irq_vectors, pdev);
+ cxlds->nr_irq_vecs = nvecs;
}
static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -561,10 +524,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (rc)
return rc;
- if (!cxl_pci_alloc_irq_vectors(cxlds)) {
- cxlds->has_irq = true;
- } else
- cxlds->has_irq = false;
+ cxl_pci_alloc_irq_vectors(cxlds);
cxlmd = devm_cxl_add_memdev(cxlds);
if (IS_ERR(cxlmd))
base-commit: aae703b02f92bde9264366c545e87cec451de471
prerequisite-patch-id: e3c882f3fce0872c2259538d00ad798236ec251f
prerequisite-patch-id: a8faa71e6d79cb30eae9b863349f0cb5ffa55b05
prerequisite-patch-id: f8e6edeb4a1d8bc4b34a509cb3d4a625becdf1b3
prerequisite-patch-id: 665a2b5af761a3f50e20da2fa8e5fdc9df13969d
prerequisite-patch-id: 5cd9f56597f9c8637201193849f68811a94d2309
prerequisite-patch-id: 89be9f2bd84118682b9b37e4f3a6e057fd4ad0d6
prerequisite-patch-id: 3de731a18a28c32572bf65f38e8eb2fb927e4f56
--
2.37.2
Powered by blists - more mailing lists