diff mbox series

[2/2] cxl/mbox: Wire up irq support

Message ID 20221018030010.20913-3-dave@stgolabs.net
State New
Headers
Series cxl: Add general MSI-X/MSI irq support |

Commit Message

Davidlohr Bueso Oct. 18, 2022, 3 a.m. UTC 
  With enough vectors properly allocated, this adds support for
(the primary) mailbox interrupt, which is needed for background
completion handling, beyond polling.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 drivers/cxl/cxl.h |  1 +
 drivers/cxl/pci.c | 29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

Comments

Jonathan Cameron Oct. 18, 2022, 9:38 a.m. UTC | #1 
On Mon, 17 Oct 2022 20:00:10 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> With enough vectors properly allocated, this adds support for
> (the primary) mailbox interrupt, which is needed for background
> completion handling, beyond polling.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>

Whilst I get the need for an example, I'd rather this didn't
go in until we have that background handler in place.

Unused infrastructure tends to rot or not be quite what is
needed.

Follow up comment below.

Jonathan


>  static void cxl_pci_free_irq_vectors(void *data)
> @@ -562,6 +588,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		return rc;
>  
>  	if (!cxl_pci_alloc_irq_vectors(cxlds)) {
> +		cxl_pci_mbox_irqsetup(cxlds);

Ah. That rather wrecks my previous suggestion :)

>  		cxlds->has_irq = true;
>  	} else
>  		cxlds->has_irq = false;
Davidlohr Bueso Oct. 21, 2022, 5:23 p.m. UTC | #2 
On Tue, 18 Oct 2022, Jonathan Cameron wrote:

>Whilst I get the need for an example, I'd rather this didn't
>go in until we have that background handler in place.

One of the reasons why the bg stuff hasn't been re-posted is
because the only user currently is the sanitation work
(and hopefully maybe scan media soon), which in turn depends
on the cache-flushing thing to be picked up (unless someone
starts ranting again against wbinvd). And that is in Dave's
pmem security series which I'm hoping will be picked up for
v6.3 at some point.

So I guess we're a while away from the irq thing. I was mostly
suggesting sending an mbox-only just to layout the
pci_alloc_irq_vectors if we're not using the common table,
but per the above this might not be sooner than the pmu or
events stuff...

>Unused infrastructure tends to rot or not be quite what is
>needed.

No arguing there.

Thanks,
Davidlohr
Dan Williams Oct. 22, 2022, 10:06 p.m. UTC | #3 
Davidlohr Bueso wrote:
> With enough vectors properly allocated, this adds support for
> (the primary) mailbox interrupt, which is needed for background
> completion handling, beyond polling.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  drivers/cxl/cxl.h |  1 +
>  drivers/cxl/pci.c | 29 ++++++++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f680450f0b16..13a9743b0012 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -135,6 +135,7 @@ static inline int ways_to_cxl(unsigned int ways, u8 *iw)
>  /* CXL 2.0 8.2.8.4 Mailbox Registers */
>  #define CXLDEV_MBOX_CAPS_OFFSET 0x00
>  #define   CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
> +#define   CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
>  #define CXLDEV_MBOX_CTRL_OFFSET 0x04
>  #define   CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
>  #define CXLDEV_MBOX_CMD_OFFSET 0x08
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 9c3e95ebaa26..c3f3ee307d7a 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -274,6 +274,32 @@ static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
>  	return 0;
>  }
>  
> +static int cxl_pci_mbox_get_max_msgnum(struct cxl_dev_state *cxlds)
> +{
> +	int cap;
> +
> +	cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
> +	return FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap);
> +}
> +
> +static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
> +{
> +	/* TODO: handle completion of background commands */
> +	return IRQ_HANDLED;
> +}
> +
> +static void cxl_pci_mbox_irqsetup(struct cxl_dev_state *cxlds)
> +{
> +	struct device *dev = cxlds->dev;
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int irq;
> +
> +	irq = cxl_pci_mbox_get_max_msgnum(cxlds);

In this context cxl_pci_mbox_get_max_msgnum() looks misnamed. There is
no max, it's only *the* message number for the mailbox.

> +	if (!pci_request_irq(pdev, irq, cxl_pci_mbox_irq, NULL,
> +			     cxlds, "%s-mailbox", dev_name(dev)))
> +		dev_dbg(dev, "Mailbox irq (%d) supported", irq);
> +}
> +
>  static int cxl_map_regblock(struct pci_dev *pdev, struct cxl_register_map *map)
>  {
>  	void __iomem *addr;
> @@ -442,7 +468,7 @@ struct cxl_irq_cap {
>  };
>  
>  static const struct cxl_irq_cap cxl_irq_cap_table[] = {
> -	NULL
> +	{ "mailbox", cxl_pci_mbox_get_max_msgnum },
>  };
>  
>  static void cxl_pci_free_irq_vectors(void *data)
> @@ -562,6 +588,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		return rc;
>  
>  	if (!cxl_pci_alloc_irq_vectors(cxlds)) {
> +		cxl_pci_mbox_irqsetup(cxlds);
>  		cxlds->has_irq = true;
>  	} else
>  		cxlds->has_irq = false;
> -- 
> 2.38.0
>
Dan Williams Oct. 22, 2022, 10:14 p.m. UTC | #4 
Davidlohr Bueso wrote:
> On Tue, 18 Oct 2022, Jonathan Cameron wrote:
> 
> >Whilst I get the need for an example, I'd rather this didn't
> >go in until we have that background handler in place.
> 
> One of the reasons why the bg stuff hasn't been re-posted is
> because the only user currently is the sanitation work
> (and hopefully maybe scan media soon), which in turn depends
> on the cache-flushing thing to be picked up (unless someone
> starts ranting again against wbinvd). And that is in Dave's
> pmem security series which I'm hoping will be picked up for
> v6.3 at some point.
> 
> So I guess we're a while away from the irq thing. I was mostly
> suggesting sending an mbox-only just to layout the
> pci_alloc_irq_vectors if we're not using the common table,
> but per the above this might not be sooner than the pmu or
> events stuff...
> 
> >Unused infrastructure tends to rot or not be quite what is
> >needed.
> 
> No arguing there.

I do feel that event notifications are likely the nearest consumer
because the whole reason to do the RCD work is to support OS native
error handling for these CXL 1.1+ devices that people can actually get
their hands on today. Error handling and other RAS flows need to be able
to hear the device's screams for help.
diff mbox series

Patch

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index f680450f0b16..13a9743b0012 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -135,6 +135,7 @@  static inline int ways_to_cxl(unsigned int ways, u8 *iw)
 /* CXL 2.0 8.2.8.4 Mailbox Registers */
 #define CXLDEV_MBOX_CAPS_OFFSET 0x00
 #define   CXLDEV_MBOX_CAP_PAYLOAD_SIZE_MASK GENMASK(4, 0)
+#define   CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK GENMASK(10, 7)
 #define CXLDEV_MBOX_CTRL_OFFSET 0x04
 #define   CXLDEV_MBOX_CTRL_DOORBELL BIT(0)
 #define CXLDEV_MBOX_CMD_OFFSET 0x08
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 9c3e95ebaa26..c3f3ee307d7a 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -274,6 +274,32 @@  static int cxl_pci_setup_mailbox(struct cxl_dev_state *cxlds)
 	return 0;
 }
 
+static int cxl_pci_mbox_get_max_msgnum(struct cxl_dev_state *cxlds)
+{
+	int cap;
+
+	cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET);
+	return FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap);
+}
+
+static irqreturn_t cxl_pci_mbox_irq(int irq, void *id)
+{
+	/* TODO: handle completion of background commands */
+	return IRQ_HANDLED;
+}
+
+static void cxl_pci_mbox_irqsetup(struct cxl_dev_state *cxlds)
+{
+	struct device *dev = cxlds->dev;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int irq;
+
+	irq = cxl_pci_mbox_get_max_msgnum(cxlds);
+	if (!pci_request_irq(pdev, irq, cxl_pci_mbox_irq, NULL,
+			     cxlds, "%s-mailbox", dev_name(dev)))
+		dev_dbg(dev, "Mailbox irq (%d) supported", irq);
+}
+
 static int cxl_map_regblock(struct pci_dev *pdev, struct cxl_register_map *map)
 {
 	void __iomem *addr;
@@ -442,7 +468,7 @@  struct cxl_irq_cap {
 };
 
 static const struct cxl_irq_cap cxl_irq_cap_table[] = {
-	NULL
+	{ "mailbox", cxl_pci_mbox_get_max_msgnum },
 };
 
 static void cxl_pci_free_irq_vectors(void *data)
@@ -562,6 +588,7 @@  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return rc;
 
 	if (!cxl_pci_alloc_irq_vectors(cxlds)) {
+		cxl_pci_mbox_irqsetup(cxlds);
 		cxlds->has_irq = true;
 	} else
 		cxlds->has_irq = false;