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);
>
