| Message ID | 20241202171222.62595-5-alejandro.lucero-palau@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: add type2 device basic support | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Mon, 2 Dec 2024 17:11:58 +0000 <alejandro.lucero-palau@amd.com> wrote: > From: Alejandro Lucero <alucerop@amd.com> > > During CXL device initialization supported capabilities by the device > are discovered. Type3 and Type2 devices have different mandatory > capabilities and a Type2 expects a specific set including optional > capabilities. > > Add a function for checking expected capabilities against those found > during initialization and allow those mandatory/expected capabilities > to be a subset of the capabilities found. > > Rely on this function for validating capabilities instead of when CXL > regs are probed. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/pci.c | 16 ++++++++++++++++ > drivers/cxl/core/regs.c | 9 --------- > drivers/cxl/pci.c | 24 ++++++++++++++++++++++++ > include/cxl/cxl.h | 3 +++ > 4 files changed, 43 insertions(+), 9 deletions(-) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 7114d632be04..a85b96eebfd3 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -8,6 +8,7 @@ > #include <linux/pci.h> > #include <linux/pci-doe.h> > #include <linux/aer.h> > +#include <cxl/cxl.h> > #include <cxlpci.h> > #include <cxlmem.h> > #include <cxl.h> > @@ -1055,3 +1056,18 @@ int cxl_pci_get_bandwidth(struct pci_dev > *pdev, struct access_coordinate *c) > return 0; > } > + > +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, unsigned long > *expected_caps, > + unsigned long *current_caps) > +{ > + > + if (current_caps) > + bitmap_copy(current_caps, cxlds->capabilities, > CXL_MAX_CAPS); + > + dev_dbg(cxlds->dev, "Checking cxlds caps 0x%08lx vs expected > caps 0x%08lx\n", > + *cxlds->capabilities, *expected_caps); > + > + /* Checking a minimum of mandatory/expected capabilities */ > + return bitmap_subset(expected_caps, cxlds->capabilities, > CXL_MAX_CAPS); +} > +EXPORT_SYMBOL_NS_GPL(cxl_pci_check_caps, CXL); > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index fe835f6df866..70378bb80b33 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -444,15 +444,6 @@ static int cxl_probe_regs(struct > cxl_register_map *map, unsigned long *caps) case > CXL_REGLOC_RBI_MEMDEV: dev_map = &map->device_map; > cxl_probe_device_regs(host, base, dev_map, caps); > - if (!dev_map->status.valid || !dev_map->mbox.valid || > - !dev_map->memdev.valid) { > - dev_err(host, "registers not found: > %s%s%s\n", > - !dev_map->status.valid ? "status " : > "", > - !dev_map->mbox.valid ? "mbox " : "", > - !dev_map->memdev.valid ? "memdev " : > ""); > - return -ENXIO; > - } > - > dev_dbg(host, "Probing device registers...\n"); > break; > default: > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index f6071bde437b..822030843b2f 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -903,6 +903,8 @@ __ATTRIBUTE_GROUPS(cxl_rcd); > static int cxl_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *id) { > struct pci_host_bridge *host_bridge = > pci_find_host_bridge(pdev->bus); > + DECLARE_BITMAP(expected, CXL_MAX_CAPS); > + DECLARE_BITMAP(found, CXL_MAX_CAPS); > struct cxl_memdev_state *mds; > struct cxl_dev_state *cxlds; > struct cxl_register_map map; > @@ -964,6 +966,28 @@ static int cxl_pci_probe(struct pci_dev *pdev, > const struct pci_device_id *id) if (rc) > dev_dbg(&pdev->dev, "Failed to map RAS > capability.\n"); > + bitmap_clear(expected, 0, CXL_MAX_CAPS); > + > + /* > + * These are the mandatory capabilities for a Type3 device. > + * Only checking capabilities used by current Linux drivers. > + */ > + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); > + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); > + bitmap_set(expected, CXL_DEV_CAP_MAILBOX_PRIMARY, 1); > + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); > + I suppose this change is for type-3, looks the caps above is wrong. It has a duplicated one and I think what you mean is CXL_DEV_CAP_MEMDEV? Better we can find some folks who have a type-3 to test these series. Z. > + /* > + * Checking mandatory caps are there as, at least, a subset > of those > + * found. > + */ > + if (!cxl_pci_check_caps(cxlds, expected, found)) { > + dev_err(&pdev->dev, > + "Expected mandatory capabilities not found: > (%08lx - %08lx)\n", > + *expected, *found); > + return -ENXIO; > + } > + > rc = cxl_pci_type3_init_mailbox(cxlds); > if (rc) > return rc; > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > index f656fcd4945f..05f06bfd2c29 100644 > --- a/include/cxl/cxl.h > +++ b/include/cxl/cxl.h > @@ -37,4 +37,7 @@ void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 > dvsec); void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource > res, enum cxl_resource); > +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, > + unsigned long *expected_caps, > + unsigned long *current_caps); > #endif
On 12/3/24 18:37, Zhi Wang wrote: > On Mon, 2 Dec 2024 17:11:58 +0000 > <alejandro.lucero-palau@amd.com> wrote: > >> From: Alejandro Lucero <alucerop@amd.com> >> >> During CXL device initialization supported capabilities by the device >> are discovered. Type3 and Type2 devices have different mandatory >> capabilities and a Type2 expects a specific set including optional >> capabilities. >> >> Add a function for checking expected capabilities against those found >> during initialization and allow those mandatory/expected capabilities >> to be a subset of the capabilities found. >> >> Rely on this function for validating capabilities instead of when CXL >> regs are probed. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/pci.c | 16 ++++++++++++++++ >> drivers/cxl/core/regs.c | 9 --------- >> drivers/cxl/pci.c | 24 ++++++++++++++++++++++++ >> include/cxl/cxl.h | 3 +++ >> 4 files changed, 43 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 7114d632be04..a85b96eebfd3 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -8,6 +8,7 @@ >> #include <linux/pci.h> >> #include <linux/pci-doe.h> >> #include <linux/aer.h> >> +#include <cxl/cxl.h> >> #include <cxlpci.h> >> #include <cxlmem.h> >> #include <cxl.h> >> @@ -1055,3 +1056,18 @@ int cxl_pci_get_bandwidth(struct pci_dev >> *pdev, struct access_coordinate *c) >> return 0; >> } >> + >> +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, unsigned long >> *expected_caps, >> + unsigned long *current_caps) >> +{ >> + >> + if (current_caps) >> + bitmap_copy(current_caps, cxlds->capabilities, >> CXL_MAX_CAPS); + >> + dev_dbg(cxlds->dev, "Checking cxlds caps 0x%08lx vs expected >> caps 0x%08lx\n", >> + *cxlds->capabilities, *expected_caps); >> + >> + /* Checking a minimum of mandatory/expected capabilities */ >> + return bitmap_subset(expected_caps, cxlds->capabilities, >> CXL_MAX_CAPS); +} >> +EXPORT_SYMBOL_NS_GPL(cxl_pci_check_caps, CXL); >> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c >> index fe835f6df866..70378bb80b33 100644 >> --- a/drivers/cxl/core/regs.c >> +++ b/drivers/cxl/core/regs.c >> @@ -444,15 +444,6 @@ static int cxl_probe_regs(struct >> cxl_register_map *map, unsigned long *caps) case >> CXL_REGLOC_RBI_MEMDEV: dev_map = &map->device_map; >> cxl_probe_device_regs(host, base, dev_map, caps); >> - if (!dev_map->status.valid || !dev_map->mbox.valid || >> - !dev_map->memdev.valid) { >> - dev_err(host, "registers not found: >> %s%s%s\n", >> - !dev_map->status.valid ? "status " : >> "", >> - !dev_map->mbox.valid ? "mbox " : "", >> - !dev_map->memdev.valid ? "memdev " : >> ""); >> - return -ENXIO; >> - } >> - >> dev_dbg(host, "Probing device registers...\n"); >> break; >> default: >> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c >> index f6071bde437b..822030843b2f 100644 >> --- a/drivers/cxl/pci.c >> +++ b/drivers/cxl/pci.c >> @@ -903,6 +903,8 @@ __ATTRIBUTE_GROUPS(cxl_rcd); >> static int cxl_pci_probe(struct pci_dev *pdev, const struct >> pci_device_id *id) { >> struct pci_host_bridge *host_bridge = >> pci_find_host_bridge(pdev->bus); >> + DECLARE_BITMAP(expected, CXL_MAX_CAPS); >> + DECLARE_BITMAP(found, CXL_MAX_CAPS); >> struct cxl_memdev_state *mds; >> struct cxl_dev_state *cxlds; >> struct cxl_register_map map; >> @@ -964,6 +966,28 @@ static int cxl_pci_probe(struct pci_dev *pdev, >> const struct pci_device_id *id) if (rc) >> dev_dbg(&pdev->dev, "Failed to map RAS >> capability.\n"); >> + bitmap_clear(expected, 0, CXL_MAX_CAPS); >> + >> + /* >> + * These are the mandatory capabilities for a Type3 device. >> + * Only checking capabilities used by current Linux drivers. >> + */ >> + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); >> + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); >> + bitmap_set(expected, CXL_DEV_CAP_MAILBOX_PRIMARY, 1); >> + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); >> + > I suppose this change is for type-3, looks the caps above is wrong. > > It has a duplicated one and I think what you mean is CXL_DEV_CAP_MEMDEV? Oh, you are right. In fact is was like that in v3! I think I did tests changing the bits for checking the code and forgot to restore it. BTW, with the new implementation this would not be an error, although obviously not correct. What I mean is now the check is by default the expected by the driver being at least a subset of those discovered and not a full match. Anyway, good catch! I'll fix it. Thanks! > > Better we can find some folks who have a type-3 to test these series. > > Z. > >> + /* >> + * Checking mandatory caps are there as, at least, a subset >> of those >> + * found. >> + */ >> + if (!cxl_pci_check_caps(cxlds, expected, found)) { >> + dev_err(&pdev->dev, >> + "Expected mandatory capabilities not found: >> (%08lx - %08lx)\n", >> + *expected, *found); >> + return -ENXIO; >> + } >> + >> rc = cxl_pci_type3_init_mailbox(cxlds); >> if (rc) >> return rc; >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >> index f656fcd4945f..05f06bfd2c29 100644 >> --- a/include/cxl/cxl.h >> +++ b/include/cxl/cxl.h >> @@ -37,4 +37,7 @@ void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 >> dvsec); void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >> int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource >> res, enum cxl_resource); >> +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, >> + unsigned long *expected_caps, >> + unsigned long *current_caps); >> #endif
On Mon, Dec 02, 2024 at 05:11:58PM +0000, alejandro.lucero-palau@amd.com wrote: > From: Alejandro Lucero <alucerop@amd.com> > > During CXL device initialization supported capabilities by the device > are discovered. Type3 and Type2 devices have different mandatory > capabilities and a Type2 expects a specific set including optional > capabilities. > > Add a function for checking expected capabilities against those found > during initialization and allow those mandatory/expected capabilities to > be a subset of the capabilities found. > > Rely on this function for validating capabilities instead of when CXL > regs are probed. > > Signed-off-by: Alejandro Lucero <alucerop@amd.com> > --- > drivers/cxl/core/pci.c | 16 ++++++++++++++++ > drivers/cxl/core/regs.c | 9 --------- > drivers/cxl/pci.c | 24 ++++++++++++++++++++++++ > include/cxl/cxl.h | 3 +++ > 4 files changed, 43 insertions(+), 9 deletions(-) > > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c > index 7114d632be04..a85b96eebfd3 100644 > --- a/drivers/cxl/core/pci.c > +++ b/drivers/cxl/core/pci.c > @@ -8,6 +8,7 @@ > #include <linux/pci.h> > #include <linux/pci-doe.h> > #include <linux/aer.h> > +#include <cxl/cxl.h> > #include <cxlpci.h> > #include <cxlmem.h> > #include <cxl.h> > @@ -1055,3 +1056,18 @@ int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c) > > return 0; > } > + > +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, unsigned long *expected_caps, > + unsigned long *current_caps) It seems "current_caps" will always be cxlds->capabilities in this series, and used only for the error message. Do we expect a case where these two can be different? If not, I think we can get rid of it and just use cxlds->capabilities directly in the function and in the error message below. Fan > +{ > + > + if (current_caps) > + bitmap_copy(current_caps, cxlds->capabilities, CXL_MAX_CAPS); > + > + dev_dbg(cxlds->dev, "Checking cxlds caps 0x%08lx vs expected caps 0x%08lx\n", > + *cxlds->capabilities, *expected_caps); > + > + /* Checking a minimum of mandatory/expected capabilities */ > + return bitmap_subset(expected_caps, cxlds->capabilities, CXL_MAX_CAPS); > +} > +EXPORT_SYMBOL_NS_GPL(cxl_pci_check_caps, CXL); > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c > index fe835f6df866..70378bb80b33 100644 > --- a/drivers/cxl/core/regs.c > +++ b/drivers/cxl/core/regs.c > @@ -444,15 +444,6 @@ static int cxl_probe_regs(struct cxl_register_map *map, unsigned long *caps) > case CXL_REGLOC_RBI_MEMDEV: > dev_map = &map->device_map; > cxl_probe_device_regs(host, base, dev_map, caps); > - if (!dev_map->status.valid || !dev_map->mbox.valid || > - !dev_map->memdev.valid) { > - dev_err(host, "registers not found: %s%s%s\n", > - !dev_map->status.valid ? "status " : "", > - !dev_map->mbox.valid ? "mbox " : "", > - !dev_map->memdev.valid ? "memdev " : ""); > - return -ENXIO; > - } > - > dev_dbg(host, "Probing device registers...\n"); > break; > default: > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index f6071bde437b..822030843b2f 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -903,6 +903,8 @@ __ATTRIBUTE_GROUPS(cxl_rcd); > static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > { > struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); > + DECLARE_BITMAP(expected, CXL_MAX_CAPS); > + DECLARE_BITMAP(found, CXL_MAX_CAPS); > struct cxl_memdev_state *mds; > struct cxl_dev_state *cxlds; > struct cxl_register_map map; > @@ -964,6 +966,28 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > if (rc) > dev_dbg(&pdev->dev, "Failed to map RAS capability.\n"); > > + bitmap_clear(expected, 0, CXL_MAX_CAPS); > + > + /* > + * These are the mandatory capabilities for a Type3 device. > + * Only checking capabilities used by current Linux drivers. > + */ > + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); > + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); > + bitmap_set(expected, CXL_DEV_CAP_MAILBOX_PRIMARY, 1); > + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); > + > + /* > + * Checking mandatory caps are there as, at least, a subset of those > + * found. > + */ > + if (!cxl_pci_check_caps(cxlds, expected, found)) { > + dev_err(&pdev->dev, > + "Expected mandatory capabilities not found: (%08lx - %08lx)\n", > + *expected, *found); > + return -ENXIO; > + } > + > rc = cxl_pci_type3_init_mailbox(cxlds); > if (rc) > return rc; > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h > index f656fcd4945f..05f06bfd2c29 100644 > --- a/include/cxl/cxl.h > +++ b/include/cxl/cxl.h > @@ -37,4 +37,7 @@ void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); > void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); > int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, > enum cxl_resource); > +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, > + unsigned long *expected_caps, > + unsigned long *current_caps); > #endif > -- > 2.17.1 >
On 12/3/24 22:55, Fan Ni wrote: > On Mon, Dec 02, 2024 at 05:11:58PM +0000, alejandro.lucero-palau@amd.com wrote: >> From: Alejandro Lucero <alucerop@amd.com> >> >> During CXL device initialization supported capabilities by the device >> are discovered. Type3 and Type2 devices have different mandatory >> capabilities and a Type2 expects a specific set including optional >> capabilities. >> >> Add a function for checking expected capabilities against those found >> during initialization and allow those mandatory/expected capabilities to >> be a subset of the capabilities found. >> >> Rely on this function for validating capabilities instead of when CXL >> regs are probed. >> >> Signed-off-by: Alejandro Lucero <alucerop@amd.com> >> --- >> drivers/cxl/core/pci.c | 16 ++++++++++++++++ >> drivers/cxl/core/regs.c | 9 --------- >> drivers/cxl/pci.c | 24 ++++++++++++++++++++++++ >> include/cxl/cxl.h | 3 +++ >> 4 files changed, 43 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c >> index 7114d632be04..a85b96eebfd3 100644 >> --- a/drivers/cxl/core/pci.c >> +++ b/drivers/cxl/core/pci.c >> @@ -8,6 +8,7 @@ >> #include <linux/pci.h> >> #include <linux/pci-doe.h> >> #include <linux/aer.h> >> +#include <cxl/cxl.h> >> #include <cxlpci.h> >> #include <cxlmem.h> >> #include <cxl.h> >> @@ -1055,3 +1056,18 @@ int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c) >> >> return 0; >> } >> + >> +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, unsigned long *expected_caps, >> + unsigned long *current_caps) > It seems "current_caps" will always be cxlds->capabilities in this > series, and used only for the error message. Do we expect a case where > these two can be different? If not, I think we can get rid of it and > just use cxlds->capabilities directly in the function and in the error > message below. They are the same because current_caps is the way the client obtains the information, being a pointer to a capabilities bitmap allocated by the client. The point being, the client can not access the capabilities from the cxlds as it is opaque for him on purpose. If the capabilities check returns error, the client can show/compare those discovered by the CXL core code and those expected. I hope this clarifies it. Thanks > Fan >> +{ >> + >> + if (current_caps) >> + bitmap_copy(current_caps, cxlds->capabilities, CXL_MAX_CAPS); >> + >> + dev_dbg(cxlds->dev, "Checking cxlds caps 0x%08lx vs expected caps 0x%08lx\n", >> + *cxlds->capabilities, *expected_caps); >> + >> + /* Checking a minimum of mandatory/expected capabilities */ >> + return bitmap_subset(expected_caps, cxlds->capabilities, CXL_MAX_CAPS); >> +} >> +EXPORT_SYMBOL_NS_GPL(cxl_pci_check_caps, CXL); >> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c >> index fe835f6df866..70378bb80b33 100644 >> --- a/drivers/cxl/core/regs.c >> +++ b/drivers/cxl/core/regs.c >> @@ -444,15 +444,6 @@ static int cxl_probe_regs(struct cxl_register_map *map, unsigned long *caps) >> case CXL_REGLOC_RBI_MEMDEV: >> dev_map = &map->device_map; >> cxl_probe_device_regs(host, base, dev_map, caps); >> - if (!dev_map->status.valid || !dev_map->mbox.valid || >> - !dev_map->memdev.valid) { >> - dev_err(host, "registers not found: %s%s%s\n", >> - !dev_map->status.valid ? "status " : "", >> - !dev_map->mbox.valid ? "mbox " : "", >> - !dev_map->memdev.valid ? "memdev " : ""); >> - return -ENXIO; >> - } >> - >> dev_dbg(host, "Probing device registers...\n"); >> break; >> default: >> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c >> index f6071bde437b..822030843b2f 100644 >> --- a/drivers/cxl/pci.c >> +++ b/drivers/cxl/pci.c >> @@ -903,6 +903,8 @@ __ATTRIBUTE_GROUPS(cxl_rcd); >> static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> { >> struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); >> + DECLARE_BITMAP(expected, CXL_MAX_CAPS); >> + DECLARE_BITMAP(found, CXL_MAX_CAPS); >> struct cxl_memdev_state *mds; >> struct cxl_dev_state *cxlds; >> struct cxl_register_map map; >> @@ -964,6 +966,28 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> if (rc) >> dev_dbg(&pdev->dev, "Failed to map RAS capability.\n"); >> >> + bitmap_clear(expected, 0, CXL_MAX_CAPS); >> + >> + /* >> + * These are the mandatory capabilities for a Type3 device. >> + * Only checking capabilities used by current Linux drivers. >> + */ >> + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); >> + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); >> + bitmap_set(expected, CXL_DEV_CAP_MAILBOX_PRIMARY, 1); >> + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); >> + >> + /* >> + * Checking mandatory caps are there as, at least, a subset of those >> + * found. >> + */ >> + if (!cxl_pci_check_caps(cxlds, expected, found)) { >> + dev_err(&pdev->dev, >> + "Expected mandatory capabilities not found: (%08lx - %08lx)\n", >> + *expected, *found); >> + return -ENXIO; >> + } >> + >> rc = cxl_pci_type3_init_mailbox(cxlds); >> if (rc) >> return rc; >> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h >> index f656fcd4945f..05f06bfd2c29 100644 >> --- a/include/cxl/cxl.h >> +++ b/include/cxl/cxl.h >> @@ -37,4 +37,7 @@ void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); >> void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); >> int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, >> enum cxl_resource); >> +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, >> + unsigned long *expected_caps, >> + unsigned long *current_caps); >> #endif >> -- >> 2.17.1 >>
diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c index 7114d632be04..a85b96eebfd3 100644 --- a/drivers/cxl/core/pci.c +++ b/drivers/cxl/core/pci.c @@ -8,6 +8,7 @@ #include <linux/pci.h> #include <linux/pci-doe.h> #include <linux/aer.h> +#include <cxl/cxl.h> #include <cxlpci.h> #include <cxlmem.h> #include <cxl.h> @@ -1055,3 +1056,18 @@ int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c) return 0; } + +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, unsigned long *expected_caps, + unsigned long *current_caps) +{ + + if (current_caps) + bitmap_copy(current_caps, cxlds->capabilities, CXL_MAX_CAPS); + + dev_dbg(cxlds->dev, "Checking cxlds caps 0x%08lx vs expected caps 0x%08lx\n", + *cxlds->capabilities, *expected_caps); + + /* Checking a minimum of mandatory/expected capabilities */ + return bitmap_subset(expected_caps, cxlds->capabilities, CXL_MAX_CAPS); +} +EXPORT_SYMBOL_NS_GPL(cxl_pci_check_caps, CXL); diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c index fe835f6df866..70378bb80b33 100644 --- a/drivers/cxl/core/regs.c +++ b/drivers/cxl/core/regs.c @@ -444,15 +444,6 @@ static int cxl_probe_regs(struct cxl_register_map *map, unsigned long *caps) case CXL_REGLOC_RBI_MEMDEV: dev_map = &map->device_map; cxl_probe_device_regs(host, base, dev_map, caps); - if (!dev_map->status.valid || !dev_map->mbox.valid || - !dev_map->memdev.valid) { - dev_err(host, "registers not found: %s%s%s\n", - !dev_map->status.valid ? "status " : "", - !dev_map->mbox.valid ? "mbox " : "", - !dev_map->memdev.valid ? "memdev " : ""); - return -ENXIO; - } - dev_dbg(host, "Probing device registers...\n"); break; default: diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index f6071bde437b..822030843b2f 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -903,6 +903,8 @@ __ATTRIBUTE_GROUPS(cxl_rcd); static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus); + DECLARE_BITMAP(expected, CXL_MAX_CAPS); + DECLARE_BITMAP(found, CXL_MAX_CAPS); struct cxl_memdev_state *mds; struct cxl_dev_state *cxlds; struct cxl_register_map map; @@ -964,6 +966,28 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) if (rc) dev_dbg(&pdev->dev, "Failed to map RAS capability.\n"); + bitmap_clear(expected, 0, CXL_MAX_CAPS); + + /* + * These are the mandatory capabilities for a Type3 device. + * Only checking capabilities used by current Linux drivers. + */ + bitmap_set(expected, CXL_DEV_CAP_HDM, 1); + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); + bitmap_set(expected, CXL_DEV_CAP_MAILBOX_PRIMARY, 1); + bitmap_set(expected, CXL_DEV_CAP_DEV_STATUS, 1); + + /* + * Checking mandatory caps are there as, at least, a subset of those + * found. + */ + if (!cxl_pci_check_caps(cxlds, expected, found)) { + dev_err(&pdev->dev, + "Expected mandatory capabilities not found: (%08lx - %08lx)\n", + *expected, *found); + return -ENXIO; + } + rc = cxl_pci_type3_init_mailbox(cxlds); if (rc) return rc; diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h index f656fcd4945f..05f06bfd2c29 100644 --- a/include/cxl/cxl.h +++ b/include/cxl/cxl.h @@ -37,4 +37,7 @@ void cxl_set_dvsec(struct cxl_dev_state *cxlds, u16 dvsec); void cxl_set_serial(struct cxl_dev_state *cxlds, u64 serial); int cxl_set_resource(struct cxl_dev_state *cxlds, struct resource res, enum cxl_resource); +bool cxl_pci_check_caps(struct cxl_dev_state *cxlds, + unsigned long *expected_caps, + unsigned long *current_caps); #endif