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@xxxxxxx> 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@xxxxxxx> > 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@xxxxxxxxxx> > >> --- >> 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@xxxxxxxxxxxxxxx >> +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); >