diff mbox series

[v5,1/2] cxl/core/regs: Add rcd_regs initialization at __rcrb_to_component()

Message ID 20240412070715.16160-2-kobayashi.da-06@fujitsu.com
State Superseded
Headers show
Series cxl: Export cxl1.1 device link status to sysfs | expand

Commit Message

Daisuke Kobayashi (Fujitsu) April 12, 2024, 7:07 a.m. UTC 
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(+)

Comments

Dave Jiang April 12, 2024, 5:47 p.m. UTC | #1 
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;
>  };
>  
>  /**
Daisuke Kobayashi (Fujitsu) April 23, 2024, 8:33 a.m. UTC | #2 
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.
Dave Jiang April 23, 2024, 3:24 p.m. UTC | #3 
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!
diff mbox series

Patch

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;
 };
 
 /**