diff mbox series

[V10,5/9] cxl/port: Find a DOE mailbox which supports CDAT

Message ID 20220605005049.2155874-6-ira.weiny@intel.com (mailing list archive)
State Superseded
Headers show
Series CXL: Read CDAT and DSMAS data | expand

Commit Message

Ira Weiny June 5, 2022, 12:50 a.m. UTC
From: Ira Weiny <ira.weiny@intel.com>

Each CXL device may have multiple DOE mailbox capabilities and each
mailbox may support multiple protocols.  CXL port devices need to query
the CDAT information specifically.

Search the DOE mailboxes for one which supports the CDAT protocol.
Cache that mailbox to be used for future queries.

Only support memory devices at this time.

Cc: Ben Widawsky <ben.widawsky@intel.com>
Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>

---
Changes from V9
	Ben Widawsky
		s/cxl_find_cdat_mb/cxl_cache_cdat_mb/; add kdoc
	Jonathan Cameron
		Move cache_cdat to port probe [Not 100% necessary but it
		goes along with reading the cdat data.]

Changes from V8
	Incorporate feedback from Jonathan
	Move all this to the cxl_port object

Changes from V7
	Minor code clean ups

Changes from V6
	Adjust for aux devices being a CXL only concept
	Update commit msg.
	Ensure devices iterated by auxiliary_find_device() are checked
		to be DOE devices prior to checking for the CDAT
		protocol
	From Ben
		Ensure reference from auxiliary_find_device() is dropped
---
 drivers/cxl/core/pci.c | 35 +++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h      |  2 ++
 drivers/cxl/cxlpci.h   |  1 +
 drivers/cxl/port.c     |  2 ++
 4 files changed, 40 insertions(+)

Comments

Ben Widawsky June 6, 2022, 5:48 p.m. UTC | #1
On 22-06-04 17:50:45, ira.weiny@intel.com wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Each CXL device may have multiple DOE mailbox capabilities and each
> mailbox may support multiple protocols.  CXL port devices need to query
> the CDAT information specifically.
> 
> Search the DOE mailboxes for one which supports the CDAT protocol.
> Cache that mailbox to be used for future queries.
> 
> Only support memory devices at this time.
> 
> Cc: Ben Widawsky <ben.widawsky@intel.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> 
> ---
> Changes from V9
> 	Ben Widawsky
> 		s/cxl_find_cdat_mb/cxl_cache_cdat_mb/; add kdoc
> 	Jonathan Cameron
> 		Move cache_cdat to port probe [Not 100% necessary but it
> 		goes along with reading the cdat data.]
> 
> Changes from V8
> 	Incorporate feedback from Jonathan
> 	Move all this to the cxl_port object
> 
> Changes from V7
> 	Minor code clean ups
> 
> Changes from V6
> 	Adjust for aux devices being a CXL only concept
> 	Update commit msg.
> 	Ensure devices iterated by auxiliary_find_device() are checked
> 		to be DOE devices prior to checking for the CDAT
> 		protocol
> 	From Ben
> 		Ensure reference from auxiliary_find_device() is dropped
> ---
>  drivers/cxl/core/pci.c | 35 +++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h      |  2 ++
>  drivers/cxl/cxlpci.h   |  1 +
>  drivers/cxl/port.c     |  2 ++
>  4 files changed, 40 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index c4c99ff7b55e..d814d8317975 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -4,11 +4,14 @@
>  #include <linux/device.h>
>  #include <linux/delay.h>
>  #include <linux/pci.h>
> +#include <linux/pci-doe.h>
>  #include <cxlpci.h>
>  #include <cxlmem.h>
>  #include <cxl.h>
>  #include "core.h"
>  
> +#define CXL_DOE_PROTOCOL_TABLE_ACCESS 2
> +
>  /**
>   * DOC: cxl core pci
>   *
> @@ -458,3 +461,35 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm)
>  	return 0;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_hdm_decode_init, CXL);
> +
> +/**
> + * cxl_cache_cdat_mb() -- cache the DOE mailbox which suports the CDAT protocol
> + *
> + * @port: Port to containing DOE Mailboxes
> + *
> + * Cache a pointer to the doe mailbox which supports CDAT.
> + */
> +void cxl_cache_cdat_mb(struct cxl_port *port)
> +{
> +	struct device *dev = port->uport;
> +	struct cxl_memdev *cxlmd;
> +	struct cxl_dev_state *cxlds;
> +	int i;
> +
> +	if (!is_cxl_memdev(dev))
> +		return;
> +
> +	cxlmd = to_cxl_memdev(dev);
> +	cxlds = cxlmd->cxlds;
> +
> +	for (i = 0; i < cxlds->num_mbs; i++) {
> +		struct pci_doe_mb *cur = cxlds->doe_mbs[i];
> +
> +		if (pci_doe_supports_prot(cur, PCI_DVSEC_VENDOR_ID_CXL,
> +					  CXL_DOE_PROTOCOL_TABLE_ACCESS)) {
> +			port->cdat_mb = cur;

What happens if cxl_pci is unloaded after this? Would it be better to copy out
the CDAT info? Otherwise, I think you need to hold a ref on the PCI device
(though I only took a quick look).

> +			return;
> +		}
> +	}
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_cache_cdat_mb, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 140dc3278cde..0a86be589ffc 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -267,6 +267,7 @@ struct cxl_nvdimm {
>   * @component_reg_phys: component register capability base address (optional)
>   * @dead: last ep has been removed, force port re-creation
>   * @depth: How deep this port is relative to the root. depth 0 is the root.
> + * @cdat_mb: Mailbox which supports the CDAT protocol
>   */
>  struct cxl_port {
>  	struct device dev;
> @@ -278,6 +279,7 @@ struct cxl_port {
>  	resource_size_t component_reg_phys;
>  	bool dead;
>  	unsigned int depth;
> +	struct pci_doe_mb *cdat_mb;
>  };
>  
>  /**
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index fce1c11729c2..ddbb8b77752e 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -74,4 +74,5 @@ static inline resource_size_t cxl_regmap_to_base(struct pci_dev *pdev,
>  int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>  struct cxl_dev_state;
>  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm);
> +void cxl_cache_cdat_mb(struct cxl_port *port);
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index 3cf308f114c4..04f3d1fc6e07 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -49,6 +49,8 @@ static int cxl_port_probe(struct device *dev)
>  	if (IS_ERR(cxlhdm))
>  		return PTR_ERR(cxlhdm);
>  
> +	cxl_cache_cdat_mb(port);
> +
>  	if (is_cxl_endpoint(port)) {
>  		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
>  		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> -- 
> 2.35.1
>
Ira Weiny June 8, 2022, 7:39 p.m. UTC | #2
On Mon, Jun 06, 2022 at 10:48:19AM -0700, Ben Widawsky wrote:
> On 22-06-04 17:50:45, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> > 
> > Each CXL device may have multiple DOE mailbox capabilities and each
> > mailbox may support multiple protocols.  CXL port devices need to query
> > the CDAT information specifically.
> > 
> > Search the DOE mailboxes for one which supports the CDAT protocol.
> > Cache that mailbox to be used for future queries.
> > 
> > Only support memory devices at this time.
> > 
> > Cc: Ben Widawsky <ben.widawsky@intel.com>
> > Cc: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
> > Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> > 
> > ---
> > Changes from V9
> > 	Ben Widawsky
> > 		s/cxl_find_cdat_mb/cxl_cache_cdat_mb/; add kdoc
> > 	Jonathan Cameron
> > 		Move cache_cdat to port probe [Not 100% necessary but it
> > 		goes along with reading the cdat data.]
> > 
> > Changes from V8
> > 	Incorporate feedback from Jonathan
> > 	Move all this to the cxl_port object
> > 
> > Changes from V7
> > 	Minor code clean ups
> > 
> > Changes from V6
> > 	Adjust for aux devices being a CXL only concept
> > 	Update commit msg.
> > 	Ensure devices iterated by auxiliary_find_device() are checked
> > 		to be DOE devices prior to checking for the CDAT
> > 		protocol
> > 	From Ben
> > 		Ensure reference from auxiliary_find_device() is dropped
> > ---
> >  drivers/cxl/core/pci.c | 35 +++++++++++++++++++++++++++++++++++
> >  drivers/cxl/cxl.h      |  2 ++
> >  drivers/cxl/cxlpci.h   |  1 +
> >  drivers/cxl/port.c     |  2 ++
> >  4 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> > index c4c99ff7b55e..d814d8317975 100644
> > --- a/drivers/cxl/core/pci.c
> > +++ b/drivers/cxl/core/pci.c
> > @@ -4,11 +4,14 @@
> >  #include <linux/device.h>
> >  #include <linux/delay.h>
> >  #include <linux/pci.h>
> > +#include <linux/pci-doe.h>
> >  #include <cxlpci.h>
> >  #include <cxlmem.h>
> >  #include <cxl.h>
> >  #include "core.h"
> >  
> > +#define CXL_DOE_PROTOCOL_TABLE_ACCESS 2
> > +
> >  /**
> >   * DOC: cxl core pci
> >   *
> > @@ -458,3 +461,35 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm)
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_NS_GPL(cxl_hdm_decode_init, CXL);
> > +
> > +/**
> > + * cxl_cache_cdat_mb() -- cache the DOE mailbox which suports the CDAT protocol
> > + *
> > + * @port: Port to containing DOE Mailboxes
> > + *
> > + * Cache a pointer to the doe mailbox which supports CDAT.
> > + */
> > +void cxl_cache_cdat_mb(struct cxl_port *port)
> > +{
> > +	struct device *dev = port->uport;
> > +	struct cxl_memdev *cxlmd;
> > +	struct cxl_dev_state *cxlds;
> > +	int i;
> > +
> > +	if (!is_cxl_memdev(dev))
> > +		return;
> > +
> > +	cxlmd = to_cxl_memdev(dev);
> > +	cxlds = cxlmd->cxlds;
> > +
> > +	for (i = 0; i < cxlds->num_mbs; i++) {
> > +		struct pci_doe_mb *cur = cxlds->doe_mbs[i];
> > +
> > +		if (pci_doe_supports_prot(cur, PCI_DVSEC_VENDOR_ID_CXL,
> > +					  CXL_DOE_PROTOCOL_TABLE_ACCESS)) {
> > +			port->cdat_mb = cur;
> 
> What happens if cxl_pci is unloaded after this? Would it be better to copy out
> the CDAT info? Otherwise, I think you need to hold a ref on the PCI device
> (though I only took a quick look).

<sigh> I thought that could not happen but I see I was wrong.

A reference will need to be taken for at least the duration of the query.
Originally I had this set up to try and make it easier to query later.  But I
think that is a waste ATM.

I'm going to rework this ownership.

Thanks for the review,
Ira

> 
> > +			return;
> > +		}
> > +	}
> > +}
> > +EXPORT_SYMBOL_NS_GPL(cxl_cache_cdat_mb, CXL);
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index 140dc3278cde..0a86be589ffc 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -267,6 +267,7 @@ struct cxl_nvdimm {
> >   * @component_reg_phys: component register capability base address (optional)
> >   * @dead: last ep has been removed, force port re-creation
> >   * @depth: How deep this port is relative to the root. depth 0 is the root.
> > + * @cdat_mb: Mailbox which supports the CDAT protocol
> >   */
> >  struct cxl_port {
> >  	struct device dev;
> > @@ -278,6 +279,7 @@ struct cxl_port {
> >  	resource_size_t component_reg_phys;
> >  	bool dead;
> >  	unsigned int depth;
> > +	struct pci_doe_mb *cdat_mb;
> >  };
> >  
> >  /**
> > diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> > index fce1c11729c2..ddbb8b77752e 100644
> > --- a/drivers/cxl/cxlpci.h
> > +++ b/drivers/cxl/cxlpci.h
> > @@ -74,4 +74,5 @@ static inline resource_size_t cxl_regmap_to_base(struct pci_dev *pdev,
> >  int devm_cxl_port_enumerate_dports(struct cxl_port *port);
> >  struct cxl_dev_state;
> >  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm);
> > +void cxl_cache_cdat_mb(struct cxl_port *port);
> >  #endif /* __CXL_PCI_H__ */
> > diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> > index 3cf308f114c4..04f3d1fc6e07 100644
> > --- a/drivers/cxl/port.c
> > +++ b/drivers/cxl/port.c
> > @@ -49,6 +49,8 @@ static int cxl_port_probe(struct device *dev)
> >  	if (IS_ERR(cxlhdm))
> >  		return PTR_ERR(cxlhdm);
> >  
> > +	cxl_cache_cdat_mb(port);
> > +
> >  	if (is_cxl_endpoint(port)) {
> >  		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
> >  		struct cxl_dev_state *cxlds = cxlmd->cxlds;
> > -- 
> > 2.35.1
> >
Ira Weiny June 8, 2022, 9:38 p.m. UTC | #3
On Wed, Jun 08, 2022 at 12:39:49PM -0700, Ira wrote:
> On Mon, Jun 06, 2022 at 10:48:19AM -0700, Ben Widawsky wrote:
> > On 22-06-04 17:50:45, ira.weiny@intel.com wrote:
> > > From: Ira Weiny <ira.weiny@intel.com>
> > > 

[snip]

> > > +
> > > +/**
> > > + * cxl_cache_cdat_mb() -- cache the DOE mailbox which suports the CDAT protocol
> > > + *
> > > + * @port: Port to containing DOE Mailboxes
> > > + *
> > > + * Cache a pointer to the doe mailbox which supports CDAT.
> > > + */
> > > +void cxl_cache_cdat_mb(struct cxl_port *port)
> > > +{
> > > +	struct device *dev = port->uport;
> > > +	struct cxl_memdev *cxlmd;
> > > +	struct cxl_dev_state *cxlds;
> > > +	int i;
> > > +
> > > +	if (!is_cxl_memdev(dev))
> > > +		return;
> > > +
> > > +	cxlmd = to_cxl_memdev(dev);
> > > +	cxlds = cxlmd->cxlds;
> > > +
> > > +	for (i = 0; i < cxlds->num_mbs; i++) {
> > > +		struct pci_doe_mb *cur = cxlds->doe_mbs[i];
> > > +
> > > +		if (pci_doe_supports_prot(cur, PCI_DVSEC_VENDOR_ID_CXL,
> > > +					  CXL_DOE_PROTOCOL_TABLE_ACCESS)) {
> > > +			port->cdat_mb = cur;
> > 
> > What happens if cxl_pci is unloaded after this? Would it be better to copy out
> > the CDAT info? Otherwise, I think you need to hold a ref on the PCI device
> > (though I only took a quick look).
> 
> <sigh> I thought that could not happen but I see I was wrong.
> 
> A reference will need to be taken for at least the duration of the query.
> Originally I had this set up to try and make it easier to query later.  But I
> think that is a waste ATM.
> 
> I'm going to rework this ownership.

BTW this means this patch is going away.  I think just searching for the
mailbox with CDAT support on each query is the way to go at this time.

Ira

> 
> Thanks for the review,
> Ira
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index c4c99ff7b55e..d814d8317975 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -4,11 +4,14 @@ 
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/pci.h>
+#include <linux/pci-doe.h>
 #include <cxlpci.h>
 #include <cxlmem.h>
 #include <cxl.h>
 #include "core.h"
 
+#define CXL_DOE_PROTOCOL_TABLE_ACCESS 2
+
 /**
  * DOC: cxl core pci
  *
@@ -458,3 +461,35 @@  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm)
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_hdm_decode_init, CXL);
+
+/**
+ * cxl_cache_cdat_mb() -- cache the DOE mailbox which suports the CDAT protocol
+ *
+ * @port: Port to containing DOE Mailboxes
+ *
+ * Cache a pointer to the doe mailbox which supports CDAT.
+ */
+void cxl_cache_cdat_mb(struct cxl_port *port)
+{
+	struct device *dev = port->uport;
+	struct cxl_memdev *cxlmd;
+	struct cxl_dev_state *cxlds;
+	int i;
+
+	if (!is_cxl_memdev(dev))
+		return;
+
+	cxlmd = to_cxl_memdev(dev);
+	cxlds = cxlmd->cxlds;
+
+	for (i = 0; i < cxlds->num_mbs; i++) {
+		struct pci_doe_mb *cur = cxlds->doe_mbs[i];
+
+		if (pci_doe_supports_prot(cur, PCI_DVSEC_VENDOR_ID_CXL,
+					  CXL_DOE_PROTOCOL_TABLE_ACCESS)) {
+			port->cdat_mb = cur;
+			return;
+		}
+	}
+}
+EXPORT_SYMBOL_NS_GPL(cxl_cache_cdat_mb, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 140dc3278cde..0a86be589ffc 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -267,6 +267,7 @@  struct cxl_nvdimm {
  * @component_reg_phys: component register capability base address (optional)
  * @dead: last ep has been removed, force port re-creation
  * @depth: How deep this port is relative to the root. depth 0 is the root.
+ * @cdat_mb: Mailbox which supports the CDAT protocol
  */
 struct cxl_port {
 	struct device dev;
@@ -278,6 +279,7 @@  struct cxl_port {
 	resource_size_t component_reg_phys;
 	bool dead;
 	unsigned int depth;
+	struct pci_doe_mb *cdat_mb;
 };
 
 /**
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index fce1c11729c2..ddbb8b77752e 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -74,4 +74,5 @@  static inline resource_size_t cxl_regmap_to_base(struct pci_dev *pdev,
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm);
+void cxl_cache_cdat_mb(struct cxl_port *port);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index 3cf308f114c4..04f3d1fc6e07 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -49,6 +49,8 @@  static int cxl_port_probe(struct device *dev)
 	if (IS_ERR(cxlhdm))
 		return PTR_ERR(cxlhdm);
 
+	cxl_cache_cdat_mb(port);
+
 	if (is_cxl_endpoint(port)) {
 		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport);
 		struct cxl_dev_state *cxlds = cxlmd->cxlds;