On Wed, 30 Jul 2025 16:47:14 -0500
Ben Cheatham <Benjamin.Cheatham@xxxxxxx> wrote:
> Add sysfs attributes to enable/disable CXL isolation and transaction
> timeout. The intended use for these attributes is to disable isolation
> and/or timeout as part of device maintenance or hotplug.
>
> The attributes are added under a new "cxl_isolation" group on the PCIe
> Root Port device.
>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
Comments below apply in quite a few of these functions, I just
picked one example to talk about.
Thanks,
Jonathan
> diff --git a/drivers/pci/pcie/cxl_isolation.c b/drivers/pci/pcie/cxl_isolation.c
> index 5a56a327b599..9d2ad14810e8 100644
> --- a/drivers/pci/pcie/cxl_isolation.c
> +++ b/drivers/pci/pcie/cxl_isolation.c
> +static ssize_t timeout_ctrl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct cxl_port *port;
> + u32 ctrl;
> +
> + struct cxl_dport **dport __free(kfree) =
> + kzalloc(sizeof(*dport), GFP_KERNEL);
> + if (!dport)
> + return -ENOMEM;
> +
> + port = cxl_find_pcie_rp(pdev, dport);
struct cxl_port *port __free(put_cxl_port) = cxl_find_pcie_rp();
Same for other cases above.
> + if (!port || !(*dport))
leaks device reference if port is set and dport isn't.
> + return -ENODEV;
> +
> + if (!(*dport)->regs.isolation)
leaks device reference.
> + return -ENXIO;
> +
> + ctrl = readl((*dport)->regs.isolation + CXL_ISOLATION_CTRL_OFFSET);
> + put_device(&port->dev);
and no need to do this by hand.
> +
> + return sysfs_emit(buf, "%lu\n",
> + FIELD_GET(CXL_ISOLATION_CTRL_MEM_TIME_ENABLE, ctrl));
> +}
> +DEVICE_ATTR_RW(timeout_ctrl);
> +
> +static struct attribute *isolation_attrs[] = {
> + &dev_attr_timeout_ctrl.attr,
> + &dev_attr_isolation_ctrl.attr,
> + NULL,
My favorite trivial comment :) No need for that trailing comma.
> +};
