Drop a few of the subject line prefixes and mention something about
sysfs. This has nothing to do with portdrv. Following sibling "port
service drivers," I guess the prefix would be something like
"PCI/CXL:" or "PCI/CXL_ISO:"
On Wed, Jul 30, 2025 at 04:47:15PM -0500, Ben Cheatham wrote:
> Add functions to enable programming the CXL.mem transaction timeout
> range, if supported. Add a sysfs attribute to the "cxl_isolation" group
> to allow programming the timeout from userspace.
Include a sample path with at least the last 2-3 components. Maybe
even an example, e.g.,
# echo B2 > /sys/.../cxl_isolation/timeout_range
Probably also some doc in Documentation/ABI/testing/?
> The attribute can take either the CXL spec-defined hex value for the
> associated timeout range (CXL 3.2 8.2.4.24.2 field 3:0) or a
> string with the range. The range string is formatted as the range letter
> in uppercase or lowercase, with an optional "2" to specify the second
> range in the aforementioned spec ref.
>
> For example, to program the port with a timeout of 65ms to 210ms (range B)
> the following strings could be specified: "b2"/"B2". Picking the first
> portion of range B (16ms to 55ms) would be: "b"/"B".
What's the value of accepting either upper- or lower-case? It doubles
the size of ranges[], and I think timeout_range_show() always shows
the lower-case one. The spec uses upper-case.
> +/* CXL 3.2 8.2.4.24.2 CXL Timeout & Isolation Control Register, field 3:0 */
> +const struct timeout_ranges {
> + char *str;
> + u8 val;
> +} ranges[] = {
> + { .str = "a", .val = 0x1 },
> + { .str = "A", .val = 0x1 },
> + { .str = "a2", .val = 0x2 },
> + { .str = "A2", .val = 0x2 },
> + { .str = "b", .val = 0x5 },
> + { .str = "B", .val = 0x5 },
> + { .str = "b2", .val = 0x6 },
> + { .str = "B2", .val = 0x6 },
> + { .str = "c", .val = 0x9 },
> + { .str = "C", .val = 0x9 },
> + { .str = "c2", .val = 0xA },
> + { .str = "C2", .val = 0xA },
> + { .str = "d", .val = 0xD },
> + { .str = "D", .val = 0xD },
> + { .str = "d2", .val = 0xE },
> + { .str = "D2", .val = 0xE },
> +};
> +
> +static int timeout_range_str_to_val(const char *str, u8 *val)
> +{
> + char val_buf[32] = { 0 };
> + char *start;
> +
> + strscpy(val_buf, str, ARRAY_SIZE(val_buf) - 1);
> + start = strim(val_buf);
> + if (!start)
> + return -EINVAL;
> +
> + for (int i = 0; i < ARRAY_SIZE(ranges); i++)
> + if (strcmp(start, ranges[i].str) == 0)
> + return ranges[i].val;
> +
> + return -EINVAL;
> +}
> +
> +static ssize_t timeout_range_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t n)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct cxl_port *port;
> + u8 val;
> + int rc;
> +
> + rc = kstrtou8(buf, 0, &val);
> + if (rc && timeout_range_str_to_val(buf, &val) < 0)
> + return -EINVAL;
> +
> + struct cxl_dport **dport __free(kfree) =
> + kzalloc(sizeof(*dport), GFP_KERNEL);
> + if (!dport)
> + return -ENOMEM;
> +
> + port = cxl_find_pcie_rp(pdev, dport);
> + if (!port || !(*dport))
> + return -ENODEV;
> +
> + if (!(*dport)->regs.isolation)
> + return -ENXIO;
> +
> + rc = cxl_set_timeout_range(*dport, val);
> + put_device(&port->dev);
> + return rc ? rc : n;
> +}
> +
> +static ssize_t timeout_range_show(struct device *dev,
> + struct device_attribute *attr, char * buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct cxl_port *port;
> + u32 ctrl, val;
> +
> + struct cxl_dport **dport __free(kfree) =
> + kzalloc(sizeof(*dport), GFP_KERNEL);
> + if (!dport)
> + return -ENOMEM;
> +
> + port = cxl_find_pcie_rp(pdev, dport);
> + if (!port || !(*dport))
> + return -ENODEV;
> +
> + if (!(*dport)->regs.isolation)
> + return -ENXIO;
> +
> + ctrl = readl((*dport)->regs.isolation + CXL_ISOLATION_CTRL_OFFSET);
> + put_device(&port->dev);
> +
> + val = FIELD_GET(CXL_ISOLATION_CTRL_MEM_TIME_MASK, ctrl);
> + for (int i = 0; i < ARRAY_SIZE(ranges); i++)
> + if (ranges[i].val == val)
> + return sysfs_emit(buf, "%s\n", ranges[i].str);
> +
> + return -ENXIO;
> +}
> +DEVICE_ATTR_RW(timeout_range);
> +
> static struct attribute *isolation_attrs[] = {
> &dev_attr_timeout_ctrl.attr,
> &dev_attr_isolation_ctrl.attr,
> + &dev_attr_timeout_range.attr,
> NULL,
> };
>
> diff --git a/include/cxl/isolation.h b/include/cxl/isolation.h
> index 0b6e4f0160a8..f2c4feb5a42b 100644
> --- a/include/cxl/isolation.h
> +++ b/include/cxl/isolation.h
> @@ -30,6 +30,7 @@ void cxl_enable_isolation(struct cxl_dport *dport);
> int cxl_disable_isolation(struct cxl_dport *dport);
> void cxl_enable_timeout(struct cxl_dport *dport);
> void cxl_disable_timeout(struct cxl_dport *dport);
> +int cxl_set_timeout_range(struct cxl_dport *dport, u8 val);
>
> struct cxl_port *cxl_find_pcie_rp(struct pci_dev *pdev,
> struct cxl_dport **dport);
> @@ -39,6 +40,8 @@ static inline int cxl_disable_isolation(struct cxl_dport *dport)
> { return -ENXIO; }
> static inline void cxl_enable_timeout(struct cxl_dport *dport) {}
> static inline void cxl_disable_timeout(struct cxl_dport *dport) {}
> +static inline int cxl_set_timeout_range(struct cxl_dport *dport, u8 val)
> +{ return -ENXIO; }
>
> static inline struct cxl_port *cxl_find_pcie_rp(struct pci_dev *pdev,
> struct cxl_dport **dport);
> --
> 2.34.1
>
