Message ID | 168428296504.2205351.15006863208285686036.stgit@djiang5-mobl3 |
---|---|
State | Superseded |
Headers | show |
Series | cxl: Move operations after memory is ready | expand |
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>
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 --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;