diff mbox series

[2/3] cxl: Move identify and partition query from pci probe to port probe

Message ID 168428296504.2205351.15006863208285686036.stgit@djiang5-mobl3
State Superseded
Headers show
Series cxl: Move operations after memory is ready | expand

Commit Message

Dave Jiang May 17, 2023, 12:22 a.m. UTC
Move the enumeration of device capacity to cxl_port_probe() from
cxl_pci_probe(). The size and capacity information should be read
after cxl_await_media_ready() so the data is valid.

Fixes: 5e2411ae8071 ("cxl/memdev: Change cxl_mem to a more descriptive name")
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/pci.c  |    8 --------
 drivers/cxl/port.c |    8 ++++++++
 2 files changed, 8 insertions(+), 8 deletions(-)

Comments

Ira Weiny May 18, 2023, 3:07 a.m. UTC | #1
Dave Jiang wrote:
> Move the enumeration of device capacity to cxl_port_probe() from
> cxl_pci_probe(). The size and capacity information should be read
> after cxl_await_media_ready() so the data is valid.
> 
> Fixes: 5e2411ae8071 ("cxl/memdev: Change cxl_mem to a more descriptive name")
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Dan Williams May 18, 2023, 3:40 p.m. UTC | #2
Dave Jiang wrote:
> Move the enumeration of device capacity to cxl_port_probe() from
> cxl_pci_probe(). The size and capacity information should be read
> after cxl_await_media_ready() so the data is valid.

This causes problems with our current sysfs ABI. What does
/sys/bus/cxl/devices/memX/ram/size show in the interim? Now userspace
needs to second guess when that attribute is ready for reading? While I
agree that it would have been nice to delay capacity determination until
after port probe, that ship has sailed.

Note that cxl_pci is already awaiting mailbox readiness which I expect
an implementation would want to put behind the media ready event just so
it does not return garbage for Identify commands, however that is not
required by the spec.

All signs point to doing the reverse of this patch and moving
await-media-ready before the first retrieval of capacity information.
The requirement is that any agent that registers memdevs
(devm_cxl_add_memdev()) must settle the capacity information prior to
registration.

> Fixes: 5e2411ae8071 ("cxl/memdev: Change cxl_mem to a more descriptive name")

No, you want the commit that first introduced cxl_dev_state_identify()
to cxl_pci.

> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/pci.c  |    8 --------
>  drivers/cxl/port.c |    8 ++++++++
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index ea38bd49b0cf..0f85e59eb60f 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -720,14 +720,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	if (rc)
>  		return rc;
>  
> -	rc = cxl_dev_state_identify(cxlds);
> -	if (rc)
> -		return rc;
> -
> -	rc = cxl_mem_create_range_info(cxlds);
> -	if (rc)
> -		return rc;
> -
>  	rc = cxl_alloc_irq_vectors(pdev);
>  	if (rc)
>  		return rc;
> diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
> index a49f5eb149f1..40e7b7de8bf6 100644
> --- a/drivers/cxl/port.c
> +++ b/drivers/cxl/port.c
> @@ -114,6 +114,14 @@ static int cxl_endpoint_port_probe(struct cxl_port *port)
>  		return rc;
>  	}
>  
> +	rc = cxl_dev_state_identify(cxlds);
> +	if (rc)
> +		return rc;
> +
> +	rc = cxl_mem_create_range_info(cxlds);
> +	if (rc)
> +		return rc;
> +
>  	rc = devm_cxl_enumerate_decoders(cxlhdm, &info);
>  	if (rc)
>  		return rc;
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index ea38bd49b0cf..0f85e59eb60f 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -720,14 +720,6 @@  static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (rc)
 		return rc;
 
-	rc = cxl_dev_state_identify(cxlds);
-	if (rc)
-		return rc;
-
-	rc = cxl_mem_create_range_info(cxlds);
-	if (rc)
-		return rc;
-
 	rc = cxl_alloc_irq_vectors(pdev);
 	if (rc)
 		return rc;
diff --git a/drivers/cxl/port.c b/drivers/cxl/port.c
index a49f5eb149f1..40e7b7de8bf6 100644
--- a/drivers/cxl/port.c
+++ b/drivers/cxl/port.c
@@ -114,6 +114,14 @@  static int cxl_endpoint_port_probe(struct cxl_port *port)
 		return rc;
 	}
 
+	rc = cxl_dev_state_identify(cxlds);
+	if (rc)
+		return rc;
+
+	rc = cxl_mem_create_range_info(cxlds);
+	if (rc)
+		return rc;
+
 	rc = devm_cxl_enumerate_decoders(cxlhdm, &info);
 	if (rc)
 		return rc;