| Message ID | 20230925200127.504256-2-Benjamin.Cheatham@amd.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | CXL, ACPI, APEI, EINJ: Update EINJ for CXL 1.1 error types | expand |
On Mon, 25 Sep 2023 15:01:25 -0500 Ben Cheatham <Benjamin.Cheatham@amd.com> wrote: > Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie > device) for CXL RCH root ports. The file will print the RCRB base > MMIO address of the root port when read and will be used by > users looking to inject CXL EINJ error types for RCH hosts. > > Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com> Hi Ben, I'm still not totally convinced that injecting the group via the link onto the PCI device is necessarily a good idea, but if Bjorn is fine with that I don't mind too much. There is a question on whether this should also be added to the sysfs-bus-pci docs given it turns up on a device where people might look there first. So with that in mind Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 9 ++++ > drivers/cxl/acpi.c | 2 + > drivers/cxl/core/port.c | 58 +++++++++++++++++++++++++ > drivers/cxl/cxl.h | 2 + > 4 files changed, 71 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 087f762ebfd5..85621da69296 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -177,6 +177,15 @@ Description: > integer reflects the hardware port unique-id used in the > hardware decoder target list. > > +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr > +What: /sys/devices/pciX/cxl_rcrb_addr > +Date: August, 2023 > +KernelVersion: v6.6 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address > + of the RCRB of the corresponding CXL 1.1 downstream port. Only > + present for CXL 1.1 dports. > > What: /sys/bus/cxl/devices/decoderX.Y > Date: June, 2021 > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index d1c559879dcc..3e2ca946bf47 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) > if (IS_ERR(root_port)) > return PTR_ERR(root_port); > > + set_cxl_root(root_port); > + > rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, > add_host_bridge_dport); > if (rc < 0) > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 724be8448eb4..c3914e73f67e 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port) > } > EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL); > > +static struct cxl_port *cxl_root; > + > +void set_cxl_root(struct cxl_port *root_port) > +{ > + cxl_root = root_port; > +} > +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL); > + > static struct cxl_dport *find_dport(struct cxl_port *port, int id) > { > struct cxl_dport *dport; > @@ -930,11 +938,56 @@ static void cond_cxl_root_unlock(struct cxl_port *port) > device_unlock(&port->dev); > } > > +static ssize_t cxl_rcrb_addr_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct cxl_dport *dport; > + > + if (!cxl_root) > + return -ENODEV; > + > + dport = cxl_find_dport_by_dev(cxl_root, dev); > + if (!dport) > + return -ENODEV; > + > + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base); > +} > +DEVICE_ATTR_RO(cxl_rcrb_addr); > + > +static umode_t cxl_rcrb_addr_is_visible(struct kobject *kobj, > + struct attribute *a, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct cxl_dport *dport; > + > + if (!IS_ENABLED(CONFIG_ACPI_APEI_EINJ) || !cxl_root) > + return 0; > + > + dport = cxl_find_dport_by_dev(cxl_root, dev); > + if (!dport || !dport->rch || dport->rcrb.base == CXL_RESOURCE_NONE) > + return 0; > + > + return a->mode; > +} > + > +static struct attribute *cxl_rcrb_addr_attrs[] = { > + &dev_attr_cxl_rcrb_addr.attr, > + NULL, > +}; > + > +static const struct attribute_group cxl_rcrb_addr_group = { > + .attrs = cxl_rcrb_addr_attrs, > + .is_visible = cxl_rcrb_addr_is_visible, > +}; > + > static void cxl_dport_remove(void *data) > { > struct cxl_dport *dport = data; > struct cxl_port *port = dport->port; > > + if (dport->rch) > + sysfs_remove_group(&dport->dport_dev->kobj, &cxl_rcrb_addr_group); > + > xa_erase(&port->dports, (unsigned long) dport->dport_dev); > put_device(dport->dport_dev); > } > @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, > if (rc) > return ERR_PTR(rc); > > + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); > + if (rc) > + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", > + rc); > + > return dport; > } > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h > index 76d92561af29..4d5bce4bae7e 100644 > --- a/drivers/cxl/cxl.h > +++ b/drivers/cxl/cxl.h > @@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host, > resource_size_t component_reg_phys, > struct cxl_dport *parent_dport); > struct cxl_port *find_cxl_root(struct cxl_port *port); > +void set_cxl_root(struct cxl_port *root_port); > + > int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); > void cxl_bus_rescan(void); > void cxl_bus_drain(void);
Hi Jonathan, thanks for the review! Responses inline. On 9/26/23 5:50 AM, Jonathan Cameron wrote: > On Mon, 25 Sep 2023 15:01:25 -0500 > Ben Cheatham <Benjamin.Cheatham@amd.com> wrote: > >> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie >> device) for CXL RCH root ports. The file will print the RCRB base >> MMIO address of the root port when read and will be used by >> users looking to inject CXL EINJ error types for RCH hosts. >> >> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com> > Hi Ben, > > I'm still not totally convinced that injecting the group via the link > onto the PCI device is necessarily a good idea, but if Bjorn is fine with > that I don't mind too much. > I'm not 100% convinced it's great either, but it was the cleanest implementation I could come up with at the time. I'll try and see if I can come up with something better if I get some extra time in the near future, otherwise I'll leave it as is if Bjorn approves. > There is a question on whether this should also be added to the > sysfs-bus-pci docs given it turns up on a device where people might look there > first. > I agree, I'll move it to sysfs-bus-pci. I didn't move it this revision because the device the file was on (pciX) wasn't under sys/bus/pci on my test system. > So with that in mind > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> --- >> Documentation/ABI/testing/sysfs-bus-cxl | 9 ++++ >> drivers/cxl/acpi.c | 2 + >> drivers/cxl/core/port.c | 58 +++++++++++++++++++++++++ >> drivers/cxl/cxl.h | 2 + >> 4 files changed, 71 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl >> index 087f762ebfd5..85621da69296 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-cxl >> +++ b/Documentation/ABI/testing/sysfs-bus-cxl >> @@ -177,6 +177,15 @@ Description: >> integer reflects the hardware port unique-id used in the >> hardware decoder target list. >> >> +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr >> +What: /sys/devices/pciX/cxl_rcrb_addr >> +Date: August, 2023 >> +KernelVersion: v6.6 >> +Contact: linux-cxl@vger.kernel.org >> +Description: >> + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address >> + of the RCRB of the corresponding CXL 1.1 downstream port. Only >> + present for CXL 1.1 dports. >> >> What: /sys/bus/cxl/devices/decoderX.Y >> Date: June, 2021 >> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c >> index d1c559879dcc..3e2ca946bf47 100644 >> --- a/drivers/cxl/acpi.c >> +++ b/drivers/cxl/acpi.c >> @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) >> if (IS_ERR(root_port)) >> return PTR_ERR(root_port); >> >> + set_cxl_root(root_port); >> + >> rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, >> add_host_bridge_dport); >> if (rc < 0) >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >> index 724be8448eb4..c3914e73f67e 100644 >> --- a/drivers/cxl/core/port.c >> +++ b/drivers/cxl/core/port.c >> @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port) >> } >> EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL); >> >> +static struct cxl_port *cxl_root; >> + >> +void set_cxl_root(struct cxl_port *root_port) >> +{ >> + cxl_root = root_port; >> +} >> +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL); >> + >> static struct cxl_dport *find_dport(struct cxl_port *port, int id) >> { >> struct cxl_dport *dport; >> @@ -930,11 +938,56 @@ static void cond_cxl_root_unlock(struct cxl_port *port) >> device_unlock(&port->dev); >> } >> >> +static ssize_t cxl_rcrb_addr_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct cxl_dport *dport; >> + >> + if (!cxl_root) >> + return -ENODEV; >> + >> + dport = cxl_find_dport_by_dev(cxl_root, dev); >> + if (!dport) >> + return -ENODEV; >> + >> + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base); >> +} >> +DEVICE_ATTR_RO(cxl_rcrb_addr); >> + >> +static umode_t cxl_rcrb_addr_is_visible(struct kobject *kobj, >> + struct attribute *a, int n) >> +{ >> + struct device *dev = kobj_to_dev(kobj); >> + struct cxl_dport *dport; >> + >> + if (!IS_ENABLED(CONFIG_ACPI_APEI_EINJ) || !cxl_root) >> + return 0; >> + >> + dport = cxl_find_dport_by_dev(cxl_root, dev); >> + if (!dport || !dport->rch || dport->rcrb.base == CXL_RESOURCE_NONE) >> + return 0; >> + >> + return a->mode; >> +} >> + >> +static struct attribute *cxl_rcrb_addr_attrs[] = { >> + &dev_attr_cxl_rcrb_addr.attr, >> + NULL, >> +}; >> + >> +static const struct attribute_group cxl_rcrb_addr_group = { >> + .attrs = cxl_rcrb_addr_attrs, >> + .is_visible = cxl_rcrb_addr_is_visible, >> +}; >> + >> static void cxl_dport_remove(void *data) >> { >> struct cxl_dport *dport = data; >> struct cxl_port *port = dport->port; >> >> + if (dport->rch) >> + sysfs_remove_group(&dport->dport_dev->kobj, &cxl_rcrb_addr_group); >> + >> xa_erase(&port->dports, (unsigned long) dport->dport_dev); >> put_device(dport->dport_dev); >> } >> @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, >> if (rc) >> return ERR_PTR(rc); >> >> + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); >> + if (rc) >> + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", >> + rc); >> + >> return dport; >> } >> >> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h >> index 76d92561af29..4d5bce4bae7e 100644 >> --- a/drivers/cxl/cxl.h >> +++ b/drivers/cxl/cxl.h >> @@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host, >> resource_size_t component_reg_phys, >> struct cxl_dport *parent_dport); >> struct cxl_port *find_cxl_root(struct cxl_port *port); >> +void set_cxl_root(struct cxl_port *root_port); >> + >> int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); >> void cxl_bus_rescan(void); >> void cxl_bus_drain(void); >
On Mon, Sep 25, 2023 at 03:01:25PM -0500, Ben Cheatham wrote: > Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie > device) for CXL RCH root ports. The file will print the RCRB base > MMIO address of the root port when read and will be used by > users looking to inject CXL EINJ error types for RCH hosts. I guess this is talking about a sysfs file? If so, maybe mention that explicitly in the subject and commit log. I don't know how you decided to capitalize CXL initialisms and not "pcie", but I usually use "PCIe". > +static struct cxl_port *cxl_root; > + > +void set_cxl_root(struct cxl_port *root_port) > +{ > + cxl_root = root_port; > +} Is there always at most one cxl_root? Seems worth a one-line comment at the static data item, since in the world of devices, data is usually in a per-device struct, not in a single static item. > @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, > if (rc) > return ERR_PTR(rc); > > + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); > + if (rc) > + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", > + rc); Is there any way to create this with an attribute group that the sysfs infrastructure adds automatically? I'm not suggesting you have a race condition here, but using the sysfs infrastructure avoids a lot of potential problems. Bjorn
Ben Cheatham wrote: > Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie > device) for CXL RCH root ports. The file will print the RCRB base > MMIO address of the root port when read and will be used by > users looking to inject CXL EINJ error types for RCH hosts. RCRB is an implementation detail of RCH topologies, I don't see why userspace needs this information, maybe it becomes clearer in the follow on patches, but I would hope this detail could be hidden. > Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com> > --- > Documentation/ABI/testing/sysfs-bus-cxl | 9 ++++ > drivers/cxl/acpi.c | 2 + > drivers/cxl/core/port.c | 58 +++++++++++++++++++++++++ > drivers/cxl/cxl.h | 2 + > 4 files changed, 71 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl > index 087f762ebfd5..85621da69296 100644 > --- a/Documentation/ABI/testing/sysfs-bus-cxl > +++ b/Documentation/ABI/testing/sysfs-bus-cxl > @@ -177,6 +177,15 @@ Description: > integer reflects the hardware port unique-id used in the > hardware decoder target list. > > +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr > +What: /sys/devices/pciX/cxl_rcrb_addr > +Date: August, 2023 > +KernelVersion: v6.6 > +Contact: linux-cxl@vger.kernel.org > +Description: > + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address > + of the RCRB of the corresponding CXL 1.1 downstream port. Only > + present for CXL 1.1 dports. > > What: /sys/bus/cxl/devices/decoderX.Y > Date: June, 2021 > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c > index d1c559879dcc..3e2ca946bf47 100644 > --- a/drivers/cxl/acpi.c > +++ b/drivers/cxl/acpi.c > @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) > if (IS_ERR(root_port)) > return PTR_ERR(root_port); > > + set_cxl_root(root_port); > + The cxl_root is not a singleton and the way to determine this linkage is by walking up the port hierarchy. See find_cxl_root(). > rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, > add_host_bridge_dport); > if (rc < 0) > diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c > index 724be8448eb4..c3914e73f67e 100644 > --- a/drivers/cxl/core/port.c > +++ b/drivers/cxl/core/port.c > @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port) > } > EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL); > > +static struct cxl_port *cxl_root; > + > +void set_cxl_root(struct cxl_port *root_port) > +{ > + cxl_root = root_port; > +} > +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL); > + > static struct cxl_dport *find_dport(struct cxl_port *port, int id) > { > struct cxl_dport *dport; > @@ -930,11 +938,56 @@ static void cond_cxl_root_unlock(struct cxl_port *port) > device_unlock(&port->dev); > } > > +static ssize_t cxl_rcrb_addr_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct cxl_dport *dport; > + > + if (!cxl_root) > + return -ENODEV; > + > + dport = cxl_find_dport_by_dev(cxl_root, dev); > + if (!dport) > + return -ENODEV; > + > + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base); > +} > +DEVICE_ATTR_RO(cxl_rcrb_addr); > + > +static umode_t cxl_rcrb_addr_is_visible(struct kobject *kobj, > + struct attribute *a, int n) > +{ > + struct device *dev = kobj_to_dev(kobj); > + struct cxl_dport *dport; > + > + if (!IS_ENABLED(CONFIG_ACPI_APEI_EINJ) || !cxl_root) > + return 0; > + > + dport = cxl_find_dport_by_dev(cxl_root, dev); > + if (!dport || !dport->rch || dport->rcrb.base == CXL_RESOURCE_NONE) > + return 0; > + > + return a->mode; > +} > + > +static struct attribute *cxl_rcrb_addr_attrs[] = { > + &dev_attr_cxl_rcrb_addr.attr, > + NULL, > +}; > + > +static const struct attribute_group cxl_rcrb_addr_group = { > + .attrs = cxl_rcrb_addr_attrs, > + .is_visible = cxl_rcrb_addr_is_visible, > +}; > + > static void cxl_dport_remove(void *data) > { > struct cxl_dport *dport = data; > struct cxl_port *port = dport->port; > > + if (dport->rch) > + sysfs_remove_group(&dport->dport_dev->kobj, &cxl_rcrb_addr_group); > + > xa_erase(&port->dports, (unsigned long) dport->dport_dev); > put_device(dport->dport_dev); > } > @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, > if (rc) > return ERR_PTR(rc); > > + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); > + if (rc) > + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", > + rc); Please no dynamic sysfs attribute registration. If this attribute is needed it should be static.
Hi Bjorn, thanks for taking a look! On 9/26/23 3:23 PM, Bjorn Helgaas wrote: > On Mon, Sep 25, 2023 at 03:01:25PM -0500, Ben Cheatham wrote: >> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie >> device) for CXL RCH root ports. The file will print the RCRB base >> MMIO address of the root port when read and will be used by >> users looking to inject CXL EINJ error types for RCH hosts. > > I guess this is talking about a sysfs file? If so, maybe mention that > explicitly in the subject and commit log. > > I don't know how you decided to capitalize CXL initialisms and not > "pcie", but I usually use "PCIe". > Sorry about that, it's a typo. I should've written PCIE/PCIe. >> +static struct cxl_port *cxl_root; >> + >> +void set_cxl_root(struct cxl_port *root_port) >> +{ >> + cxl_root = root_port; >> +} > > Is there always at most one cxl_root? Seems worth a one-line comment > at the static data item, since in the world of devices, data is > usually in a per-device struct, not in a single static item. > I thought that the root was a singleton, but per Dan's comment it seems that isn't the case. >> @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, >> if (rc) >> return ERR_PTR(rc); >> >> + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); >> + if (rc) >> + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", >> + rc); > > Is there any way to create this with an attribute group that the sysfs > infrastructure adds automatically? I'm not suggesting you have a race > condition here, but using the sysfs infrastructure avoids a lot of > potential problems. > I would need to look into it more. I did it this way because I wasn't sure there was a way to set it up statically since the dport_dev can match a couple of different device types. After looking at Dan's comments I will probably move away from making this file altogether and move the functionality under sysfs/debug/cxl. > Bjorn
Thanks for the review Dan, responses inline. On 9/26/23 4:15 PM, Dan Williams wrote: > Ben Cheatham wrote: >> Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie >> device) for CXL RCH root ports. The file will print the RCRB base >> MMIO address of the root port when read and will be used by >> users looking to inject CXL EINJ error types for RCH hosts. > > RCRB is an implementation detail of RCH topologies, I don't see why > userspace needs this information, maybe it becomes clearer in the follow > on patches, but I would hope this detail could be hidden. > It doesn't, I'll rename the file (if it stays at all). >> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com> >> --- >> Documentation/ABI/testing/sysfs-bus-cxl | 9 ++++ >> drivers/cxl/acpi.c | 2 + >> drivers/cxl/core/port.c | 58 +++++++++++++++++++++++++ >> drivers/cxl/cxl.h | 2 + >> 4 files changed, 71 insertions(+) >> >> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl >> index 087f762ebfd5..85621da69296 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-cxl >> +++ b/Documentation/ABI/testing/sysfs-bus-cxl >> @@ -177,6 +177,15 @@ Description: >> integer reflects the hardware port unique-id used in the >> hardware decoder target list. >> >> +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr >> +What: /sys/devices/pciX/cxl_rcrb_addr >> +Date: August, 2023 >> +KernelVersion: v6.6 >> +Contact: linux-cxl@vger.kernel.org >> +Description: >> + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address >> + of the RCRB of the corresponding CXL 1.1 downstream port. Only >> + present for CXL 1.1 dports. >> >> What: /sys/bus/cxl/devices/decoderX.Y >> Date: June, 2021 >> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c >> index d1c559879dcc..3e2ca946bf47 100644 >> --- a/drivers/cxl/acpi.c >> +++ b/drivers/cxl/acpi.c >> @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) >> if (IS_ERR(root_port)) >> return PTR_ERR(root_port); >> >> + set_cxl_root(root_port); >> + > > The cxl_root is not a singleton and the way to determine this linkage is > by walking up the port hierarchy. See find_cxl_root(). > Ok, I was under the impression it was and couldn't find anything definitive in the CXL spec about it. The reason I did that was that I couldn't get access to the CXL port hierarchy from the EINJ model and needed an access point. >> rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, >> add_host_bridge_dport); >> if (rc < 0) >> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >> index 724be8448eb4..c3914e73f67e 100644 >> --- a/drivers/cxl/core/port.c >> +++ b/drivers/cxl/core/port.c >> @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port) >> } >> EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL); >> >> +static struct cxl_port *cxl_root; >> + >> +void set_cxl_root(struct cxl_port *root_port) >> +{ >> + cxl_root = root_port; >> +} >> +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL); >> + >> static struct cxl_dport *find_dport(struct cxl_port *port, int id) >> { >> struct cxl_dport *dport; >> @@ -930,11 +938,56 @@ static void cond_cxl_root_unlock(struct cxl_port *port) >> device_unlock(&port->dev); >> } >> >> +static ssize_t cxl_rcrb_addr_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct cxl_dport *dport; >> + >> + if (!cxl_root) >> + return -ENODEV; >> + >> + dport = cxl_find_dport_by_dev(cxl_root, dev); >> + if (!dport) >> + return -ENODEV; >> + >> + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base); >> +} >> +DEVICE_ATTR_RO(cxl_rcrb_addr); >> + >> +static umode_t cxl_rcrb_addr_is_visible(struct kobject *kobj, >> + struct attribute *a, int n) >> +{ >> + struct device *dev = kobj_to_dev(kobj); >> + struct cxl_dport *dport; >> + >> + if (!IS_ENABLED(CONFIG_ACPI_APEI_EINJ) || !cxl_root) >> + return 0; >> + >> + dport = cxl_find_dport_by_dev(cxl_root, dev); >> + if (!dport || !dport->rch || dport->rcrb.base == CXL_RESOURCE_NONE) >> + return 0; >> + >> + return a->mode; >> +} >> + >> +static struct attribute *cxl_rcrb_addr_attrs[] = { >> + &dev_attr_cxl_rcrb_addr.attr, >> + NULL, >> +}; >> + >> +static const struct attribute_group cxl_rcrb_addr_group = { >> + .attrs = cxl_rcrb_addr_attrs, >> + .is_visible = cxl_rcrb_addr_is_visible, >> +}; >> + >> static void cxl_dport_remove(void *data) >> { >> struct cxl_dport *dport = data; >> struct cxl_port *port = dport->port; >> >> + if (dport->rch) >> + sysfs_remove_group(&dport->dport_dev->kobj, &cxl_rcrb_addr_group); >> + >> xa_erase(&port->dports, (unsigned long) dport->dport_dev); >> put_device(dport->dport_dev); >> } >> @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, >> if (rc) >> return ERR_PTR(rc); >> >> + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); >> + if (rc) >> + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", >> + rc); > > Please no dynamic sysfs attribute registration. If this attribute is > needed it should be static. Understood. The file probably won't stay in v6, but I'll keep everything static.
diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl index 087f762ebfd5..85621da69296 100644 --- a/Documentation/ABI/testing/sysfs-bus-cxl +++ b/Documentation/ABI/testing/sysfs-bus-cxl @@ -177,6 +177,15 @@ Description: integer reflects the hardware port unique-id used in the hardware decoder target list. +What: /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr +What: /sys/devices/pciX/cxl_rcrb_addr +Date: August, 2023 +KernelVersion: v6.6 +Contact: linux-cxl@vger.kernel.org +Description: + (RO) The 'cxl_rcrb_addr' device file gives the MMIO base address + of the RCRB of the corresponding CXL 1.1 downstream port. Only + present for CXL 1.1 dports. What: /sys/bus/cxl/devices/decoderX.Y Date: June, 2021 diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c index d1c559879dcc..3e2ca946bf47 100644 --- a/drivers/cxl/acpi.c +++ b/drivers/cxl/acpi.c @@ -676,6 +676,8 @@ static int cxl_acpi_probe(struct platform_device *pdev) if (IS_ERR(root_port)) return PTR_ERR(root_port); + set_cxl_root(root_port); + rc = bus_for_each_dev(adev->dev.bus, NULL, root_port, add_host_bridge_dport); if (rc < 0) diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c index 724be8448eb4..c3914e73f67e 100644 --- a/drivers/cxl/core/port.c +++ b/drivers/cxl/core/port.c @@ -875,6 +875,14 @@ struct cxl_port *find_cxl_root(struct cxl_port *port) } EXPORT_SYMBOL_NS_GPL(find_cxl_root, CXL); +static struct cxl_port *cxl_root; + +void set_cxl_root(struct cxl_port *root_port) +{ + cxl_root = root_port; +} +EXPORT_SYMBOL_NS_GPL(set_cxl_root, CXL); + static struct cxl_dport *find_dport(struct cxl_port *port, int id) { struct cxl_dport *dport; @@ -930,11 +938,56 @@ static void cond_cxl_root_unlock(struct cxl_port *port) device_unlock(&port->dev); } +static ssize_t cxl_rcrb_addr_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct cxl_dport *dport; + + if (!cxl_root) + return -ENODEV; + + dport = cxl_find_dport_by_dev(cxl_root, dev); + if (!dport) + return -ENODEV; + + return sysfs_emit(buf, "0x%llx\n", (u64) dport->rcrb.base); +} +DEVICE_ATTR_RO(cxl_rcrb_addr); + +static umode_t cxl_rcrb_addr_is_visible(struct kobject *kobj, + struct attribute *a, int n) +{ + struct device *dev = kobj_to_dev(kobj); + struct cxl_dport *dport; + + if (!IS_ENABLED(CONFIG_ACPI_APEI_EINJ) || !cxl_root) + return 0; + + dport = cxl_find_dport_by_dev(cxl_root, dev); + if (!dport || !dport->rch || dport->rcrb.base == CXL_RESOURCE_NONE) + return 0; + + return a->mode; +} + +static struct attribute *cxl_rcrb_addr_attrs[] = { + &dev_attr_cxl_rcrb_addr.attr, + NULL, +}; + +static const struct attribute_group cxl_rcrb_addr_group = { + .attrs = cxl_rcrb_addr_attrs, + .is_visible = cxl_rcrb_addr_is_visible, +}; + static void cxl_dport_remove(void *data) { struct cxl_dport *dport = data; struct cxl_port *port = dport->port; + if (dport->rch) + sysfs_remove_group(&dport->dport_dev->kobj, &cxl_rcrb_addr_group); + xa_erase(&port->dports, (unsigned long) dport->dport_dev); put_device(dport->dport_dev); } @@ -1021,6 +1074,11 @@ __devm_cxl_add_dport(struct cxl_port *port, struct device *dport_dev, if (rc) return ERR_PTR(rc); + rc = sysfs_create_group(&dport_dev->kobj, &cxl_rcrb_addr_group); + if (rc) + dev_dbg(dport_dev, "Couldn't create cxl_rcrb_addr group: %d\n", + rc); + return dport; } diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h index 76d92561af29..4d5bce4bae7e 100644 --- a/drivers/cxl/cxl.h +++ b/drivers/cxl/cxl.h @@ -690,6 +690,8 @@ struct cxl_port *devm_cxl_add_port(struct device *host, resource_size_t component_reg_phys, struct cxl_dport *parent_dport); struct cxl_port *find_cxl_root(struct cxl_port *port); +void set_cxl_root(struct cxl_port *root_port); + int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd); void cxl_bus_rescan(void); void cxl_bus_drain(void);
Add cxl_rcrb_addr to the dport_dev (normally represented by a pcie device) for CXL RCH root ports. The file will print the RCRB base MMIO address of the root port when read and will be used by users looking to inject CXL EINJ error types for RCH hosts. Signed-off-by: Ben Cheatham <Benjamin.Cheatham@amd.com> --- Documentation/ABI/testing/sysfs-bus-cxl | 9 ++++ drivers/cxl/acpi.c | 2 + drivers/cxl/core/port.c | 58 +++++++++++++++++++++++++ drivers/cxl/cxl.h | 2 + 4 files changed, 71 insertions(+)