* [PATCH v10 0/2] Export cxl1.1 device link status register value to pci device sysfs.
@ 2024-06-11 5:52 Kobayashi,Daisuke
2024-06-11 5:52 ` [PATCH v10 1/2] cxl/core/regs: Add rcd_pcie_cap initialization at __rcrb_to_component() Kobayashi,Daisuke
2024-06-11 5:52 ` [PATCH v10 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
0 siblings, 2 replies; 5+ messages in thread
From: Kobayashi,Daisuke @ 2024-06-11 5:52 UTC (permalink / raw)
To: kobayashi.da-06, linux-cxl
Cc: y-goto, mj, dan.j.williams, jonathan.cameron, Kobayashi,Daisuke
CXL devices are extensions of PCIe. Therefore, from CXL2.0 onwards,
the link status can be output in the same way as traditional PCIe.
However, unlike devices from CXL2.0 onwards, CXL1.1 requires a
different method to obtain the link status from traditional PCIe.
This is because the link status of the CXL1.1 device is not mapped
in the configuration space (as per cxl3.0 specification 8.1).
Instead, the configuration space containing the link status is mapped
to the memory mapped register region (as per cxl3.0 specification 8.2,
Table 8-18). Therefore, the current lspci has a problem where it does
not display the link status of the CXL1.1 device.
Solve these issues with sysfs attributes to export the status
registers hidden in the RCRB.
The procedure is as follows:
First, obtain the RCRB address within the cxl driver, then access
the configuration space. Next, output the link status information from
the configuration space to sysfs. Ultimately, the expectation is that
this sysfs file will be consumed by PCI user tools to utilize link status
information.
Changes
v1[1] -> v2:
- Modified to perform rcrb access within the CXL driver.
- Added new attributes to the sysfs of the PCI device.
- Output the link status information to the sysfs of the PCI device.
- Retrieve information from sysfs as the source when displaying information in lspci.
v2[2] -> v3:
- Fix unnecessary initialization and wrong types (Bjohn).
- Create a helper function for getting a PCIe capability offset (Bjohn).
- Move platform-specific implementation to the lib directory in pciutils (Martin).
v3[3] -> v4:
- RCRB register values are read once and cached.
- Added a new attribute to the sysfs of the PCI device.
- Separate lspci implementation from this patch.
v4[4] -> v5:
- Use macros for bitwise operations
- Fix RCRB access to use cxl_memdev
v5[5] -> v6:
- Add and use masks for RCRB register values
v6[6] -> v7:
- Fix comments on white space inline
v7[7] -> v8:
- Change the cache value to offset
- Access memory map area in rcd_*_show() functions
v8[8] -> v9:
- Map the pcie cap in for all the time the driver is bound to the device
- Add mapping the pcie cap in cxl_rcd_component_reg_phys()
v9[9] -> v10:
- Change a utility function for getting PCIe capability
- Fix tab alignment issue, error handling, and apply suggestions from Jonathan
[1]
https://lore.kernel.org/linux-cxl/20231220050738.178481-1-kobayashi.da-06@fujitsu.com/
[2]
https://lore.kernel.org/linux-cxl/20240227083313.87699-1-kobayashi.da-06@fujitsu.com/
[3]
https://lore.kernel.org/linux-cxl/20240312080559.14904-1-kobayashi.da-06@fujitsu.com/
[4]
https://lore.kernel.org/linux-cxl/20240409073528.13214-1-kobayashi.da-06@fujitsu.com/
[5]
https://lore.kernel.org/linux-cxl/20240412070715.16160-1-kobayashi.da-06@fujitsu.com/
[6]
https://lore.kernel.org/linux-cxl/20240424050102.26788-1-kobayashi.da-06@fujitsu.com/
[7]
https://lore.kernel.org/linux-cxl/20240510073710.98953-1-kobayashi.da-06@fujitsu.com/
[8]
https://lore.kernel.org/linux-cxl/20240606074814.5633-1-kobayashi.da-06@fujitsu.com/
[9]
https://lore.kernel.org/linux-cxl/20240610082222.22772-1-kobayashi.da-06@fujitsu.com/
Kobayashi,Daisuke (2):
cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()
cxl/pci: Add sysfs attribute for CXL 1.1 device link status
drivers/cxl/core/core.h | 5 +++
drivers/cxl/core/regs.c | 27 +++++++++++-
drivers/cxl/cxl.h | 9 ++++
drivers/cxl/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 138 insertions(+), 1 deletion(-)
--
2.44.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v10 1/2] cxl/core/regs: Add rcd_pcie_cap initialization at __rcrb_to_component()
2024-06-11 5:52 [PATCH v10 0/2] Export cxl1.1 device link status register value to pci device sysfs Kobayashi,Daisuke
@ 2024-06-11 5:52 ` Kobayashi,Daisuke
2024-06-11 15:35 ` Jonathan Cameron
2024-06-11 5:52 ` [PATCH v10 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
1 sibling, 1 reply; 5+ messages in thread
From: Kobayashi,Daisuke @ 2024-06-11 5:52 UTC (permalink / raw)
To: kobayashi.da-06, linux-cxl
Cc: y-goto, mj, dan.j.williams, jonathan.cameron, Kobayashi,Daisuke
Add rcd_pcie_cap and its initialization at __rcrb_to_component() to cache
the offset of cxl1.1 device link status information. By caching it, avoid
the walking memory map area to find the offset when output the register
value.
Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
---
drivers/cxl/core/core.h | 5 +++++
drivers/cxl/core/regs.c | 27 ++++++++++++++++++++++++++-
drivers/cxl/cxl.h | 9 +++++++++
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 3b64fb1b9ed0..66778c3ce3b7 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -75,6 +75,11 @@ resource_size_t __rcrb_to_component(struct device *dev,
enum cxl_rcrb which);
u16 cxl_rcrb_to_aer(struct device *dev, resource_size_t rcrb);
+#define PCI_RCRB_CAP_LIST_ID_MASK GENMASK(7, 0)
+#define PCI_RCRB_CAP_HDR_ID_MASK GENMASK(7, 0)
+#define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8)
+#define RCRB_PCIECAP_LEN 0x3c
+
extern struct rw_semaphore cxl_dpa_rwsem;
extern struct rw_semaphore cxl_region_rwsem;
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 372786f80955..5ce831ca05ca 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -514,6 +514,8 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri
u32 bar0, bar1;
u16 cmd;
u32 id;
+ u32 cap_hdr;
+ u16 offset;
if (which == CXL_RCRB_UPSTREAM)
rcrb += SZ_4K;
@@ -537,6 +539,19 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri
cmd = readw(addr + PCI_COMMAND);
bar0 = readl(addr + PCI_BASE_ADDRESS_0);
bar1 = readl(addr + PCI_BASE_ADDRESS_1);
+ offset = FIELD_GET(PCI_RCRB_CAP_LIST_ID_MASK, readw(addr + PCI_CAPABILITY_LIST));
+ cap_hdr = readl(addr + offset);
+ while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr)) != PCI_CAP_ID_EXP) {
+ offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr);
+ if (offset == 0 || offset > SZ_4K) {
+ offset = 0;
+ break;
+ }
+ cap_hdr = readl(addr + offset);
+ }
+ if (offset)
+ ri->rcd_pcie_cap = offset;
+
iounmap(addr);
release_mem_region(rcrb, SZ_4K);
@@ -572,8 +587,18 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri
resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
struct cxl_dport *dport)
{
+ resource_size_t rcd_pcie_offset, ret;
+
if (!dport->rch)
return CXL_RESOURCE_NONE;
- return __rcrb_to_component(dev, &dport->rcrb, CXL_RCRB_UPSTREAM);
+
+ ret = __rcrb_to_component(dev, &dport->rcrb, CXL_RCRB_UPSTREAM);
+ if (dport->rcrb.rcd_pcie_cap) {
+ rcd_pcie_offset = dport->rcrb.base + dport->rcrb.rcd_pcie_cap;
+ dport->regs.rcd_pcie_cap = devm_cxl_iomap_block(dev, rcd_pcie_offset,
+ sizeof(u8) * RCRB_PCIECAP_LEN);
+ }
+
+ return ret;
}
EXPORT_SYMBOL_NS_GPL(cxl_rcd_component_reg_phys, CXL);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 003feebab79b..fc9e0dbd5932 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -230,6 +230,14 @@ struct cxl_regs {
struct_group_tagged(cxl_rch_regs, rch_regs,
void __iomem *dport_aer;
);
+
+ /*
+ * RCD upstream port specific PCIe cap register
+ * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB
+ */
+ struct_group_tagged(cxl_rcd_regs, rcd_regs,
+ void __iomem *rcd_pcie_cap;
+ );
};
struct cxl_reg_map {
@@ -646,6 +654,7 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
struct cxl_rcrb_info {
resource_size_t base;
+ u16 rcd_pcie_cap;
u16 aer_cap;
};
--
2.44.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v10 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status
2024-06-11 5:52 [PATCH v10 0/2] Export cxl1.1 device link status register value to pci device sysfs Kobayashi,Daisuke
2024-06-11 5:52 ` [PATCH v10 1/2] cxl/core/regs: Add rcd_pcie_cap initialization at __rcrb_to_component() Kobayashi,Daisuke
@ 2024-06-11 5:52 ` Kobayashi,Daisuke
2024-06-11 15:40 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Kobayashi,Daisuke @ 2024-06-11 5:52 UTC (permalink / raw)
To: kobayashi.da-06, linux-cxl
Cc: y-goto, mj, dan.j.williams, jonathan.cameron, Kobayashi,Daisuke
Add sysfs attribute for CXL 1.1 device link status to the cxl pci device.
In CXL1.1, the link status of the device is included in the RCRB mapped to
the memory mapped register area. Critically, that arrangement makes the
link status and control registers invisible to existing PCI user tooling.
Export those registers via sysfs with the expectation that PCI user
tooling will alternatively look for these sysfs files when attempting to
access to these CXL 1.1 endpoints registers.
Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
---
drivers/cxl/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 2ff361e756d6..655616a16892 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -786,6 +786,103 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
return 0;
}
+static ssize_t rcd_pcie_cap_emitl(struct device *dev, u16 offset, char *buf)
+{
+ struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
+ struct cxl_memdev *cxlmd = cxlds->cxlmd;
+ struct device *endpoint_parent;
+ struct cxl_dport *dport;
+ struct cxl_port *port;
+
+ port = cxl_mem_find_port(cxlmd, &dport);
+ if (!port)
+ return -EINVAL;
+
+ endpoint_parent = port->uport_dev;
+ if (!endpoint_parent)
+ return -ENXIO;
+
+ guard(device)(endpoint_parent);
+ if (!endpoint_parent->driver)
+ return -ENXIO;
+
+ if (dport->regs.rcd_pcie_cap == NULL)
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%x\n", readl(dport->regs.rcd_pcie_cap + offset));
+}
+
+static ssize_t rcd_pcie_cap_emitw(struct device *dev, u16 offset, char *buf)
+{
+ struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
+ struct cxl_memdev *cxlmd = cxlds->cxlmd;
+ struct device *endpoint_parent;
+ struct cxl_dport *dport;
+ struct cxl_port *port;
+
+ port = cxl_mem_find_port(cxlmd, &dport);
+ if (!port)
+ return -EINVAL;
+
+ endpoint_parent = port->uport_dev;
+ if (!endpoint_parent)
+ return -ENXIO;
+
+ guard(device)(endpoint_parent);
+ if (!endpoint_parent->driver)
+ return -ENXIO;
+
+ if (dport->regs.rcd_pcie_cap == NULL)
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%x\n", readw(dport->regs.rcd_pcie_cap + offset));
+}
+
+static ssize_t rcd_link_cap_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return rcd_pcie_cap_emitl(dev, PCI_EXP_LNKCAP, buf);
+}
+static DEVICE_ATTR_RO(rcd_link_cap);
+
+static ssize_t rcd_link_ctrl_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return rcd_pcie_cap_emitw(dev, PCI_EXP_LNKCTL, buf);
+}
+static DEVICE_ATTR_RO(rcd_link_ctrl);
+
+static ssize_t rcd_link_status_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return rcd_pcie_cap_emitw(dev, PCI_EXP_LNKSTA, buf);
+}
+static DEVICE_ATTR_RO(rcd_link_status);
+
+static struct attribute *cxl_rcd_attrs[] = {
+ &dev_attr_rcd_link_cap.attr,
+ &dev_attr_rcd_link_ctrl.attr,
+ &dev_attr_rcd_link_status.attr,
+ NULL
+};
+
+static umode_t cxl_rcd_visible(struct kobject *kobj, struct attribute *a, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct pci_dev *pdev = to_pci_dev(dev);
+
+ if (is_cxl_restricted(pdev))
+ return a->mode;
+
+ return 0;
+}
+
+static struct attribute_group cxl_rcd_group = {
+ .attrs = cxl_rcd_attrs,
+ .is_visible = cxl_rcd_visible,
+};
+__ATTRIBUTE_GROUPS(cxl_rcd);
+
static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
@@ -969,6 +1066,7 @@ static struct pci_driver cxl_pci_driver = {
.id_table = cxl_mem_pci_tbl,
.probe = cxl_pci_probe,
.err_handler = &cxl_error_handlers,
+ .dev_groups = cxl_rcd_groups,
.driver = {
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
--
2.44.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v10 1/2] cxl/core/regs: Add rcd_pcie_cap initialization at __rcrb_to_component()
2024-06-11 5:52 ` [PATCH v10 1/2] cxl/core/regs: Add rcd_pcie_cap initialization at __rcrb_to_component() Kobayashi,Daisuke
@ 2024-06-11 15:35 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2024-06-11 15:35 UTC (permalink / raw)
To: Kobayashi,Daisuke; +Cc: kobayashi.da-06, linux-cxl, y-goto, mj, dan.j.williams
On Tue, 11 Jun 2024 14:52:53 +0900
"Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com> wrote:
> Add rcd_pcie_cap and its initialization at __rcrb_to_component() to cache
> the offset of cxl1.1 device link status information. By caching it, avoid
> the walking memory map area to find the offset when output the register
> value.
>
> Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
Hi. The basic functionality now looks good, but I'm not convinced by
the 'where' of the calls. Even though it will require an additional
ioremap/iounmap() pair I don't think you should bury this in code
doing something largely unrelated.
> ---
> drivers/cxl/core/core.h | 5 +++++
> drivers/cxl/core/regs.c | 27 ++++++++++++++++++++++++++-
> drivers/cxl/cxl.h | 9 +++++++++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 3b64fb1b9ed0..66778c3ce3b7 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -75,6 +75,11 @@ resource_size_t __rcrb_to_component(struct device *dev,
> enum cxl_rcrb which);
> u16 cxl_rcrb_to_aer(struct device *dev, resource_size_t rcrb);
>
> +#define PCI_RCRB_CAP_LIST_ID_MASK GENMASK(7, 0)
> +#define PCI_RCRB_CAP_HDR_ID_MASK GENMASK(7, 0)
> +#define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8)
> +#define RCRB_PCIECAP_LEN 0x3c
> +
> extern struct rw_semaphore cxl_dpa_rwsem;
> extern struct rw_semaphore cxl_region_rwsem;
>
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 372786f80955..5ce831ca05ca 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -514,6 +514,8 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri
> u32 bar0, bar1;
> u16 cmd;
> u32 id;
> + u32 cap_hdr;
> + u16 offset;
>
> if (which == CXL_RCRB_UPSTREAM)
> rcrb += SZ_4K;
> @@ -537,6 +539,19 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri
> cmd = readw(addr + PCI_COMMAND);
> bar0 = readl(addr + PCI_BASE_ADDRESS_0);
> bar1 = readl(addr + PCI_BASE_ADDRESS_1);
> + offset = FIELD_GET(PCI_RCRB_CAP_LIST_ID_MASK, readw(addr + PCI_CAPABILITY_LIST));
Similarly to below, this is putting unrelated functionality in __rcrb_to_component()
I think you need to be more smilar to cxl_rcrb_to_aer() which does it's
own iomap of the rcrb. That allows it to keep to doing just one thing.
So I would have a cxl_rcrb_to_lnkcap() and call that from
cxl_pci_setup_regs()
> + cap_hdr = readl(addr + offset);
> + while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr)) != PCI_CAP_ID_EXP) {
> + offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr);
> + if (offset == 0 || offset > SZ_4K) {
> + offset = 0;
> + break;
> + }
> + cap_hdr = readl(addr + offset);
> + }
> + if (offset)
> + ri->rcd_pcie_cap = offset;
> +
> iounmap(addr);
> release_mem_region(rcrb, SZ_4K);
>
> @@ -572,8 +587,18 @@ resource_size_t __rcrb_to_component(struct device *dev, struct cxl_rcrb_info *ri
> resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
> struct cxl_dport *dport)
This has gone from being a 'find me one of these' function to one
that does rather more inside. Doesn't feel like the right place
to locate this capability.
I would wrap the mapping code up in a function similar to the aer_cap
code in cxl_dport_map_rch_aer() called something like
cxl_dport_map_rcd_lnkcap(), and call that directly from cxl_pci_setup_regs()
under the same check as is use for calling cxl_rcrb_get_comp_regs()
> {
> + resource_size_t rcd_pcie_offset, ret;
> +
> if (!dport->rch)
> return CXL_RESOURCE_NONE;
> - return __rcrb_to_component(dev, &dport->rcrb, CXL_RCRB_UPSTREAM);
> +
> + ret = __rcrb_to_component(dev, &dport->rcrb, CXL_RCRB_UPSTREAM);
> + if (dport->rcrb.rcd_pcie_cap) {
> + rcd_pcie_offset = dport->rcrb.base + dport->rcrb.rcd_pcie_cap;
> + dport->regs.rcd_pcie_cap = devm_cxl_iomap_block(dev, rcd_pcie_offset,
> + sizeof(u8) * RCRB_PCIECAP_LEN);
> + }
> +
> + return ret;
> }
> EXPORT_SYMBOL_NS_GPL(cxl_rcd_component_reg_phys, CXL);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 003feebab79b..fc9e0dbd5932 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -230,6 +230,14 @@ struct cxl_regs {
> struct_group_tagged(cxl_rch_regs, rch_regs,
> void __iomem *dport_aer;
> );
> +
> + /*
> + * RCD upstream port specific PCIe cap register
> + * @pcie_cap: CXL 3.0 8.2.1.2 RCD Upstream Port RCRB
> + */
> + struct_group_tagged(cxl_rcd_regs, rcd_regs,
> + void __iomem *rcd_pcie_cap;
> + );
> };
>
> struct cxl_reg_map {
> @@ -646,6 +654,7 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
>
> struct cxl_rcrb_info {
> resource_size_t base;
> + u16 rcd_pcie_cap;
> u16 aer_cap;
> };
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v10 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status
2024-06-11 5:52 ` [PATCH v10 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
@ 2024-06-11 15:40 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2024-06-11 15:40 UTC (permalink / raw)
To: Kobayashi,Daisuke; +Cc: kobayashi.da-06, linux-cxl, y-goto, mj, dan.j.williams
On Tue, 11 Jun 2024 14:52:54 +0900
"Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com> wrote:
> Add sysfs attribute for CXL 1.1 device link status to the cxl pci device.
>
> In CXL1.1, the link status of the device is included in the RCRB mapped to
> the memory mapped register area. Critically, that arrangement makes the
> link status and control registers invisible to existing PCI user tooling.
>
> Export those registers via sysfs with the expectation that PCI user
> tooling will alternatively look for these sysfs files when attempting to
> access to these CXL 1.1 endpoints registers.
>
> Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
Hi.
Can go one step further without hurting readability much.
Jonathan
> ---
> drivers/cxl/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 2ff361e756d6..655616a16892 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -786,6 +786,103 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
> return 0;
> }
>
> +static ssize_t rcd_pcie_cap_emitl(struct device *dev, u16 offset, char *buf)
> +{
> + struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
> + struct device *endpoint_parent;
> + struct cxl_dport *dport;
> + struct cxl_port *port;
> +
> + port = cxl_mem_find_port(cxlmd, &dport);
> + if (!port)
> + return -EINVAL;
> +
> + endpoint_parent = port->uport_dev;
> + if (!endpoint_parent)
> + return -ENXIO;
> +
> + guard(device)(endpoint_parent);
> + if (!endpoint_parent->driver)
> + return -ENXIO;
> +
> + if (dport->regs.rcd_pcie_cap == NULL)
> + return -EINVAL;
> +
> + return sysfs_emit(buf, "%x\n", readl(dport->regs.rcd_pcie_cap + offset));
I'd create an extra function called form both of these called from these with
a size parameter that select between the last two lines.
rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size_t width)
{
....
switch (width) {
case 2:
return sysfs_emit(buf, "%x\n",
readw(dport->regs.rcd_pcie_cap + offset));
case 4:
return sysfs_emit(buf, "%x\n",
readl(dport->regs.rcd_pcie_cap + offset));
default:
return -EINVAL;
}
}
Then call that from the wrappers with appropriate size parameter.
> +}
> +
> +static ssize_t rcd_pcie_cap_emitw(struct device *dev, u16 offset, char *buf)
> +{
> + struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
> + struct device *endpoint_parent;
> + struct cxl_dport *dport;
> + struct cxl_port *port;
> +
> + port = cxl_mem_find_port(cxlmd, &dport);
> + if (!port)
> + return -EINVAL;
> +
> + endpoint_parent = port->uport_dev;
> + if (!endpoint_parent)
> + return -ENXIO;
> +
> + guard(device)(endpoint_parent);
> + if (!endpoint_parent->driver)
> + return -ENXIO;
> +
> + if (dport->regs.rcd_pcie_cap == NULL)
> + return -EINVAL;
> +
> + return sysfs_emit(buf, "%x\n", readw(dport->regs.rcd_pcie_cap + offset));
> +}
> +
> +static ssize_t rcd_link_cap_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return rcd_pcie_cap_emitl(dev, PCI_EXP_LNKCAP, buf);
> +}
> +static DEVICE_ATTR_RO(rcd_link_cap);
> +
> +static ssize_t rcd_link_ctrl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return rcd_pcie_cap_emitw(dev, PCI_EXP_LNKCTL, buf);
> +}
> +static DEVICE_ATTR_RO(rcd_link_ctrl);
> +
> +static ssize_t rcd_link_status_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + return rcd_pcie_cap_emitw(dev, PCI_EXP_LNKSTA, buf);
> +}
> +static DEVICE_ATTR_RO(rcd_link_status);
> +
> +static struct attribute *cxl_rcd_attrs[] = {
> + &dev_attr_rcd_link_cap.attr,
> + &dev_attr_rcd_link_ctrl.attr,
> + &dev_attr_rcd_link_status.attr,
> + NULL
> +};
> +
> +static umode_t cxl_rcd_visible(struct kobject *kobj, struct attribute *a, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct pci_dev *pdev = to_pci_dev(dev);
> +
> + if (is_cxl_restricted(pdev))
> + return a->mode;
> +
> + return 0;
> +}
> +
> +static struct attribute_group cxl_rcd_group = {
> + .attrs = cxl_rcd_attrs,
> + .is_visible = cxl_rcd_visible,
> +};
> +__ATTRIBUTE_GROUPS(cxl_rcd);
> +
> static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> {
> struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
> @@ -969,6 +1066,7 @@ static struct pci_driver cxl_pci_driver = {
> .id_table = cxl_mem_pci_tbl,
> .probe = cxl_pci_probe,
> .err_handler = &cxl_error_handlers,
> + .dev_groups = cxl_rcd_groups,
> .driver = {
> .probe_type = PROBE_PREFER_ASYNCHRONOUS,
> },
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-11 15:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-11 5:52 [PATCH v10 0/2] Export cxl1.1 device link status register value to pci device sysfs Kobayashi,Daisuke
2024-06-11 5:52 ` [PATCH v10 1/2] cxl/core/regs: Add rcd_pcie_cap initialization at __rcrb_to_component() Kobayashi,Daisuke
2024-06-11 15:35 ` Jonathan Cameron
2024-06-11 5:52 ` [PATCH v10 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
2024-06-11 15:40 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox