* [PATCH v5 0/2] cxl: Export cxl1.1 device link status to sysfs
@ 2024-04-12 7:07 Kobayashi,Daisuke
2024-04-12 7:07 ` [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component() Kobayashi,Daisuke
2024-04-12 7:07 ` [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
0 siblings, 2 replies; 7+ messages in thread
From: Kobayashi,Daisuke @ 2024-04-12 7:07 UTC (permalink / raw
To: kobayashi.da-06, linux-cxl
Cc: y-goto, linux-pci, mj, dan.j.williams, Kobayashi,Daisuke
Export cxl1.1 device link status register value to pci device sysfs.
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
[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/
Kobayashi,Daisuke (2):
cxl: Add rcd_regs to cxl_rcrb_info
cxl/pci: Add sysfs attribute for CXL 1.1 device link status
drivers/cxl/core/regs.c | 16 +++++++
drivers/cxl/cxl.h | 3 ++
drivers/cxl/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 117 insertions(+)
--
2.44.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()
2024-04-12 7:07 [PATCH v5 0/2] cxl: Export cxl1.1 device link status to sysfs Kobayashi,Daisuke
@ 2024-04-12 7:07 ` Kobayashi,Daisuke
2024-04-12 17:47 ` Dave Jiang
2024-04-12 7:07 ` [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
1 sibling, 1 reply; 7+ messages in thread
From: Kobayashi,Daisuke @ 2024-04-12 7:07 UTC (permalink / raw
To: kobayashi.da-06, linux-cxl
Cc: y-goto, linux-pci, mj, dan.j.williams, Kobayashi,Daisuke
Add rcd_regs and its initialization at __rcrb_to_component() to cache
the cxl1.1 device link status information. Reduce access to the memory
map area where the RCRB is located by caching the cxl1.1 device
link status information.
Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
---
drivers/cxl/core/regs.c | 16 ++++++++++++++++
drivers/cxl/cxl.h | 3 +++
2 files changed, 19 insertions(+)
diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 372786f80955..e0e96be0ca7d 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;
+ u16 offset;
+ u32 cap_hdr;
if (which == CXL_RCRB_UPSTREAM)
rcrb += SZ_4K;
@@ -537,6 +539,20 @@ 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(GENMASK(7, 0), readw(addr + PCI_CAPABILITY_LIST));
+ cap_hdr = readl(addr + offset);
+ while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) {
+ offset = (cap_hdr >> 8) & GENMASK(7, 0);
+ if (offset == 0)
+ break;
+ cap_hdr = readl(addr + offset);
+ }
+ if (offset) {
+ ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP);
+ ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL);
+ ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA);
+ }
+
iounmap(addr);
release_mem_region(rcrb, SZ_4K);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 003feebab79b..2dc827c301a1 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
struct cxl_rcrb_info {
resource_size_t base;
u16 aer_cap;
+ u16 rcd_lnkctrl;
+ u16 rcd_lnkstatus;
+ u32 rcd_lnkcap;
};
/**
--
2.44.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status
2024-04-12 7:07 [PATCH v5 0/2] cxl: Export cxl1.1 device link status to sysfs Kobayashi,Daisuke
2024-04-12 7:07 ` [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component() Kobayashi,Daisuke
@ 2024-04-12 7:07 ` Kobayashi,Daisuke
2024-04-12 17:51 ` Dave Jiang
1 sibling, 1 reply; 7+ messages in thread
From: Kobayashi,Daisuke @ 2024-04-12 7:07 UTC (permalink / raw
To: kobayashi.da-06, linux-cxl
Cc: y-goto, linux-pci, mj, dan.j.williams, 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..b2d8198ab532 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_link_cap_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cxl_port *port;
+ struct cxl_dport *dport;
+ struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
+ struct cxl_memdev *cxlmd = cxlds->cxlmd;
+ struct device *endpoint_parent;
+
+ 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;
+
+ return sysfs_emit(buf, "%x\n", dport->rcrb.rcd_lnkcap);
+}
+static DEVICE_ATTR_RO(rcd_link_cap);
+
+static ssize_t rcd_link_ctrl_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cxl_port *port;
+ struct cxl_dport *dport;
+ struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
+ struct cxl_memdev *cxlmd = cxlds->cxlmd;
+ struct device *endpoint_parent;
+
+ 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;
+
+ return sysfs_emit(buf, "%x\n", dport->rcrb.rcd_lnkctrl);
+}
+static DEVICE_ATTR_RO(rcd_link_ctrl);
+
+static ssize_t rcd_link_status_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct cxl_port *port;
+ struct cxl_dport *dport;
+ struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
+ struct cxl_memdev *cxlmd = cxlds->cxlmd;
+ struct device *endpoint_parent;
+
+ 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;
+
+ return sysfs_emit(buf, "%x\n", dport->rcrb.rcd_lnkstatus);
+}
+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] 7+ messages in thread
* Re: [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()
2024-04-12 7:07 ` [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component() Kobayashi,Daisuke
@ 2024-04-12 17:47 ` Dave Jiang
2024-04-23 8:33 ` Daisuke Kobayashi (Fujitsu)
0 siblings, 1 reply; 7+ messages in thread
From: Dave Jiang @ 2024-04-12 17:47 UTC (permalink / raw
To: Kobayashi,Daisuke, kobayashi.da-06, linux-cxl
Cc: y-goto, linux-pci, mj, dan.j.williams
On 4/12/24 12:07 AM, Kobayashi,Daisuke wrote:
> Add rcd_regs and its initialization at __rcrb_to_component() to cache
> the cxl1.1 device link status information. Reduce access to the memory
> map area where the RCRB is located by caching the cxl1.1 device
> link status information.
>
> Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
> ---
> drivers/cxl/core/regs.c | 16 ++++++++++++++++
> drivers/cxl/cxl.h | 3 +++
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 372786f80955..e0e96be0ca7d 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;
> + u16 offset;
> + u32 cap_hdr;
>
> if (which == CXL_RCRB_UPSTREAM)
> rcrb += SZ_4K;
> @@ -537,6 +539,20 @@ 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(GENMASK(7, 0), readw(addr + PCI_CAPABILITY_LIST));
Maybe
#define PCI_RCRB_CAPABILITY_LIST_ID_MASK GENMASK(7, 0)
> + cap_hdr = readl(addr + offset);
> + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) {
while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr) != PCI_CAP_ID_EXP) {
Also I think you need to add a check and see if the loop went beyond SZ_4K that was mapped.
> + offset = (cap_hdr >> 8) & GENMASK(7, 0);
#define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8);
offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr);
> + if (offset == 0)
> + break;
> + cap_hdr = readl(addr + offset);
> + }
> + if (offset) {
> + ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP);
> + ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL);
> + ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA);
> + }
> +
> iounmap(addr);
> release_mem_region(rcrb, SZ_4K);
>
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 003feebab79b..2dc827c301a1 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
> struct cxl_rcrb_info {
> resource_size_t base;
> u16 aer_cap;
> + u16 rcd_lnkctrl;
> + u16 rcd_lnkstatus;
> + u32 rcd_lnkcap;
> };
>
> /**
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status
2024-04-12 7:07 ` [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
@ 2024-04-12 17:51 ` Dave Jiang
0 siblings, 0 replies; 7+ messages in thread
From: Dave Jiang @ 2024-04-12 17:51 UTC (permalink / raw
To: Kobayashi,Daisuke, kobayashi.da-06, linux-cxl
Cc: y-goto, linux-pci, mj, dan.j.williams
On 4/12/24 12:07 AM, Kobayashi,Daisuke 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>
Minor nit. Maybe arrange the variable declaration in reverse xmas tree format. Otherwise LGTM.
> ---
> drivers/cxl/pci.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 98 insertions(+)
>
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 2ff361e756d6..b2d8198ab532 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_link_cap_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct cxl_port *port;
> + struct cxl_dport *dport;
> + struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
> + struct device *endpoint_parent;
> +
> + 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;
> +
> + return sysfs_emit(buf, "%x\n", dport->rcrb.rcd_lnkcap);
> +}
> +static DEVICE_ATTR_RO(rcd_link_cap);
> +
> +static ssize_t rcd_link_ctrl_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct cxl_port *port;
> + struct cxl_dport *dport;
> + struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
> + struct device *endpoint_parent;
> +
> + 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;
> +
> + return sysfs_emit(buf, "%x\n", dport->rcrb.rcd_lnkctrl);
> +}
> +static DEVICE_ATTR_RO(rcd_link_ctrl);
> +
> +static ssize_t rcd_link_status_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct cxl_port *port;
> + struct cxl_dport *dport;
> + struct cxl_dev_state *cxlds = dev_get_drvdata(dev);
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
> + struct device *endpoint_parent;
> +
> + 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;
> +
> + return sysfs_emit(buf, "%x\n", dport->rcrb.rcd_lnkstatus);
> +}
> +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] 7+ messages in thread
* RE: [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()
2024-04-12 17:47 ` Dave Jiang
@ 2024-04-23 8:33 ` Daisuke Kobayashi (Fujitsu)
2024-04-23 15:24 ` Dave Jiang
0 siblings, 1 reply; 7+ messages in thread
From: Daisuke Kobayashi (Fujitsu) @ 2024-04-23 8:33 UTC (permalink / raw
To: 'Dave Jiang', linux-cxl@vger.kernel.org
Cc: Yasunori Gotou (Fujitsu), linux-pci@vger.kernel.org, mj@ucw.cz,
dan.j.williams@intel.com
Dave Jiang wrote:
> On 4/12/24 12:07 AM, Kobayashi,Daisuke wrote:
> > Add rcd_regs and its initialization at __rcrb_to_component() to cache
> > the cxl1.1 device link status information. Reduce access to the memory
> > map area where the RCRB is located by caching the cxl1.1 device
> > link status information.
> >
> > Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
> > ---
> > drivers/cxl/core/regs.c | 16 ++++++++++++++++
> > drivers/cxl/cxl.h | 3 +++
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> > index 372786f80955..e0e96be0ca7d 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;
> > + u16 offset;
> > + u32 cap_hdr;
> >
> > if (which == CXL_RCRB_UPSTREAM)
> > rcrb += SZ_4K;
> > @@ -537,6 +539,20 @@ 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(GENMASK(7, 0), readw(addr +
> PCI_CAPABILITY_LIST));
>
> Maybe
> #define PCI_RCRB_CAPABILITY_LIST_ID_MASK GENMASK(7, 0)
>
> > + cap_hdr = readl(addr + offset);
> > + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) {
>
> while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr) !=
> PCI_CAP_ID_EXP) {
>
> Also I think you need to add a check and see if the loop went beyond SZ_4K that
> was mapped.
>
> > + offset = (cap_hdr >> 8) & GENMASK(7, 0);
>
> #define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8);
> offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr);
Thank you for your comment. In the next patch I will define and use additional masks.
> > + if (offset == 0)
> > + break;
> > + cap_hdr = readl(addr + offset);
> > + }
> > + if (offset) {
> > + ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP);
> > + ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL);
> > + ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA);
> > + }
> > +
> > iounmap(addr);
> > release_mem_region(rcrb, SZ_4K);
> >
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index 003feebab79b..2dc827c301a1 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const
> struct device *dport_dev)
> > struct cxl_rcrb_info {
> > resource_size_t base;
> > u16 aer_cap;
> > + u16 rcd_lnkctrl;
> > + u16 rcd_lnkstatus;
> > + u32 rcd_lnkcap;
> > };
> >
> > /**
Please let me know if any revisions are necessary for merging this patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()
2024-04-23 8:33 ` Daisuke Kobayashi (Fujitsu)
@ 2024-04-23 15:24 ` Dave Jiang
0 siblings, 0 replies; 7+ messages in thread
From: Dave Jiang @ 2024-04-23 15:24 UTC (permalink / raw
To: Daisuke Kobayashi (Fujitsu), linux-cxl@vger.kernel.org
Cc: Yasunori Gotou (Fujitsu), linux-pci@vger.kernel.org, mj@ucw.cz,
dan.j.williams@intel.com
On 4/23/24 1:33 AM, Daisuke Kobayashi (Fujitsu) wrote:
> Dave Jiang wrote:
>> On 4/12/24 12:07 AM, Kobayashi,Daisuke wrote:
>>> Add rcd_regs and its initialization at __rcrb_to_component() to cache
>>> the cxl1.1 device link status information. Reduce access to the memory
>>> map area where the RCRB is located by caching the cxl1.1 device
>>> link status information.
>>>
>>> Signed-off-by: "Kobayashi,Daisuke" <kobayashi.da-06@fujitsu.com>
>>> ---
>>> drivers/cxl/core/regs.c | 16 ++++++++++++++++
>>> drivers/cxl/cxl.h | 3 +++
>>> 2 files changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
>>> index 372786f80955..e0e96be0ca7d 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;
>>> + u16 offset;
>>> + u32 cap_hdr;
>>>
>>> if (which == CXL_RCRB_UPSTREAM)
>>> rcrb += SZ_4K;
>>> @@ -537,6 +539,20 @@ 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(GENMASK(7, 0), readw(addr +
>> PCI_CAPABILITY_LIST));
>>
>> Maybe
>> #define PCI_RCRB_CAPABILITY_LIST_ID_MASK GENMASK(7, 0)
>>
>>> + cap_hdr = readl(addr + offset);
>>> + while ((cap_hdr & GENMASK(7, 0)) != PCI_CAP_ID_EXP) {
>>
>> while ((FIELD_GET(PCI_RCRB_CAP_HDR_ID_MASK, cap_hdr) !=
>> PCI_CAP_ID_EXP) {
>>
>> Also I think you need to add a check and see if the loop went beyond SZ_4K that
>> was mapped.
>>
>>> + offset = (cap_hdr >> 8) & GENMASK(7, 0);
>>
>> #define PCI_RCRB_CAP_HDR_NEXT_MASK GENMASK(15, 8);
>> offset = FIELD_GET(PCI_RCRB_CAP_HDR_NEXT_MASK, cap_hdr);
>
> Thank you for your comment. In the next patch I will define and use additional masks.
>
>>> + if (offset == 0)
>>> + break;
>>> + cap_hdr = readl(addr + offset);
>>> + }
>>> + if (offset) {
>>> + ri->rcd_lnkcap = readl(addr + offset + PCI_EXP_LNKCAP);
>>> + ri->rcd_lnkctrl = readl(addr + offset + PCI_EXP_LNKCTL);
>>> + ri->rcd_lnkstatus = readl(addr + offset + PCI_EXP_LNKSTA);
>>> + }
>>> +
>>> iounmap(addr);
>>> release_mem_region(rcrb, SZ_4K);
>>>
>>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>>> index 003feebab79b..2dc827c301a1 100644
>>> --- a/drivers/cxl/cxl.h
>>> +++ b/drivers/cxl/cxl.h
>>> @@ -647,6 +647,9 @@ cxl_find_dport_by_dev(struct cxl_port *port, const
>> struct device *dport_dev)
>>> struct cxl_rcrb_info {
>>> resource_size_t base;
>>> u16 aer_cap;
>>> + u16 rcd_lnkctrl;
>>> + u16 rcd_lnkstatus;
>>> + u32 rcd_lnkcap;
>>> };
>>>
>>> /**
>
> Please let me know if any revisions are necessary for merging this patch.
Yes please send out a revision with updates. I'd also like to see a review tag from one of the other maintainers before merging. Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-04-23 15:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-12 7:07 [PATCH v5 0/2] cxl: Export cxl1.1 device link status to sysfs Kobayashi,Daisuke
2024-04-12 7:07 ` [PATCH v5 1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component() Kobayashi,Daisuke
2024-04-12 17:47 ` Dave Jiang
2024-04-23 8:33 ` Daisuke Kobayashi (Fujitsu)
2024-04-23 15:24 ` Dave Jiang
2024-04-12 7:07 ` [PATCH v5 2/2] cxl/pci: Add sysfs attribute for CXL 1.1 device link status Kobayashi,Daisuke
2024-04-12 17:51 ` Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).