commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
unnecessarily uses pci->link_gen in deriving the index to the
n_fts[] array also introducing the issue of accessing beyond the
boundaries of array for greater than Gen-2 speeds. This change fixes
that issue.
Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
Signed-off-by: Vidya Sagar <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index c6725c519a47..9e4d96e5a3f5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -641,7 +641,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
if (pci->n_fts[1]) {
val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
val &= ~PORT_LOGIC_N_FTS_MASK;
- val |= pci->n_fts[pci->link_gen - 1];
+ val |= pci->n_fts[1];
dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
}
--
2.17.1
In subject, "Fix" (not "Fixes") so it's an imperative verb. Maybe
"Fix n_fts[] array overrun" to be more specific.
On Mon, Sep 19, 2022 at 08:01:23PM +0530, Vidya Sagar wrote:
> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
> unnecessarily uses pci->link_gen in deriving the index to the
Not just "unnecessarily"; using pci->link_gen is *incorrect* since
only [0] and [1] are defined.
> n_fts[] array also introducing the issue of accessing beyond the
> boundaries of array for greater than Gen-2 speeds. This change fixes
> that issue.
>
> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
> Signed-off-by: Vidya Sagar <[email protected]>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c6725c519a47..9e4d96e5a3f5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -641,7 +641,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
> if (pci->n_fts[1]) {
> val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> val &= ~PORT_LOGIC_N_FTS_MASK;
> - val |= pci->n_fts[pci->link_gen - 1];
> + val |= pci->n_fts[1];
> dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> }
>
> --
> 2.17.1
>
On Mon, Sep 19, 2022, Vidya Sagar <[email protected]> wrote:
>
> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
> unnecessarily uses pci->link_gen in deriving the index to the
> n_fts[] array also introducing the issue of accessing beyond the
> boundaries of array for greater than Gen-2 speeds. This change fixes
> that issue.
>
> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
> Signed-off-by: Vidya Sagar <[email protected]>
Acked-by: Jingoo Han <[email protected]>
Please send a V2 patch that addresses Bjorn's feedback.
Thank you.
Best regards,
Jingoo Han
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index c6725c519a47..9e4d96e5a3f5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -641,7 +641,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
> if (pci->n_fts[1]) {
> val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> val &= ~PORT_LOGIC_N_FTS_MASK;
> - val |= pci->n_fts[pci->link_gen - 1];
> + val |= pci->n_fts[1];
> dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> }
>
> --
> 2.17.1
>
commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
incorrectly uses pci->link_gen in deriving the index to the
n_fts[] array also introducing the issue of accessing beyond the
boundaries of array for greater than Gen-2 speeds. This change fixes
that issue.
Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
Signed-off-by: Vidya Sagar <[email protected]>
Acked-by: Jingoo Han <[email protected]>
---
V2:
* Addressed review comments from Bjorn
* Added "Acked-by: Jingoo Han <[email protected]>"
drivers/pci/controller/dwc/pcie-designware.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 650a7f22f9d0..3df687667f27 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -649,7 +649,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
if (pci->n_fts[1]) {
val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
val &= ~PORT_LOGIC_N_FTS_MASK;
- val |= pci->n_fts[pci->link_gen - 1];
+ val |= pci->n_fts[1];
dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
}
--
2.17.1
On Mon, Sep 26, 2022, Vidya Sagar <[email protected]> wrote:
>
> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
> incorrectly uses pci->link_gen in deriving the index to the
> n_fts[] array also introducing the issue of accessing beyond the
> boundaries of array for greater than Gen-2 speeds. This change fixes
> that issue.
Dear Vidya,
I am just ok with this patch. By the way, do we need another patch to generalize
this setting? Currently, this code targets only Gen1 and Gen2. But, in
my opinion,
it is possible that this driver can be used for more than Gen2,
later. PCIe Gen6
standard is available and Synopsys has their solution for Gen6. So, would you
send a new patch to generalize this thing, later?
Thank you.
Best regards,
Jingoo Han
>
> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
> Signed-off-by: Vidya Sagar <[email protected]>
> Acked-by: Jingoo Han <[email protected]>
> ---
> V2:
> * Addressed review comments from Bjorn
> * Added "Acked-by: Jingoo Han <[email protected]>"
>
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 650a7f22f9d0..3df687667f27 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -649,7 +649,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
> if (pci->n_fts[1]) {
> val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> val &= ~PORT_LOGIC_N_FTS_MASK;
> - val |= pci->n_fts[pci->link_gen - 1];
> + val |= pci->n_fts[1];
> dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> }
>
> --
> 2.17.1
>
On 9/29/2022 7:34 AM, Han Jingoo wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Sep 26, 2022, Vidya Sagar <[email protected]> wrote:
>>
>> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
>> incorrectly uses pci->link_gen in deriving the index to the
>> n_fts[] array also introducing the issue of accessing beyond the
>> boundaries of array for greater than Gen-2 speeds. This change fixes
>> that issue.
>
> Dear Vidya,
>
> I am just ok with this patch. By the way, do we need another patch to generalize
> this setting? Currently, this code targets only Gen1 and Gen2. But, in
> my opinion,
> it is possible that this driver can be used for more than Gen2,
> later. PCIe Gen6
> standard is available and Synopsys has their solution for Gen6. So, would you
> send a new patch to generalize this thing, later?
> Thank you.
As I mentioned in the commit message, current code is incorrectly using
'gen' in the index and ideally n_fts programming has nothing to do with
the speed and hence I tried to remove the 'gen' reference in the current
patch. So, the same code (after applying this patch) should work for any
future Synopsys IP versions supporting higher speeds.
Thanks,
Vidya Sagar
>
> Best regards,
> Jingoo Han
>
>>
>> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
>> Signed-off-by: Vidya Sagar <[email protected]>
>> Acked-by: Jingoo Han <[email protected]>
>> ---
>> V2:
>> * Addressed review comments from Bjorn
>> * Added "Acked-by: Jingoo Han <[email protected]>"
>>
>> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
>> index 650a7f22f9d0..3df687667f27 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware.c
>> @@ -649,7 +649,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
>> if (pci->n_fts[1]) {
>> val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>> val &= ~PORT_LOGIC_N_FTS_MASK;
>> - val |= pci->n_fts[pci->link_gen - 1];
>> + val |= pci->n_fts[1];
>> dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
>> }
>>
>> --
>> 2.17.1
>>
On Wed, Sep 28, 2022 Vidya Sagar <[email protected]> wrote:
> On 9/29/2022 7:34 AM, Han Jingoo wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Mon, Sep 26, 2022, Vidya Sagar <[email protected]> wrote:
> >>
> >> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
> >> incorrectly uses pci->link_gen in deriving the index to the
> >> n_fts[] array also introducing the issue of accessing beyond the
> >> boundaries of array for greater than Gen-2 speeds. This change fixes
> >> that issue.
> >
> > Dear Vidya,
> >
> > I am just ok with this patch. By the way, do we need another patch to generalize
> > this setting? Currently, this code targets only Gen1 and Gen2. But, in
> > my opinion,
> > it is possible that this driver can be used for more than Gen2,
> > later. PCIe Gen6
> > standard is available and Synopsys has their solution for Gen6. So, would you
> > send a new patch to generalize this thing, later?
> > Thank you.
>
> As I mentioned in the commit message, current code is incorrectly using
> 'gen' in the index and ideally n_fts programming has nothing to do with
> the speed and hence I tried to remove the 'gen' reference in the current
> patch. So, the same code (after applying this patch) should work for any
> future Synopsys IP versions supporting higher speeds.
I see. Thank you for your confirmation.
Best regards,
Jingoo Han
>
> Thanks,
> Vidya Sagar
>
> >
> > Best regards,
> > Jingoo Han
> >
> >>
> >> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
> >> Signed-off-by: Vidya Sagar <[email protected]>
> >> Acked-by: Jingoo Han <[email protected]>
> >> ---
> >> V2:
> >> * Addressed review comments from Bjorn
> >> * Added "Acked-by: Jingoo Han <[email protected]>"
> >>
> >> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> >> index 650a7f22f9d0..3df687667f27 100644
> >> --- a/drivers/pci/controller/dwc/pcie-designware.c
> >> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> >> @@ -649,7 +649,7 @@ void dw_pcie_setup(struct dw_pcie *pci)
> >> if (pci->n_fts[1]) {
> >> val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
> >> val &= ~PORT_LOGIC_N_FTS_MASK;
> >> - val |= pci->n_fts[pci->link_gen - 1];
> >> + val |= pci->n_fts[1];
> >> dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);
> >> }
> >>
> >> --
> >> 2.17.1
> >>
On Mon, Sep 26, 2022 at 6:19 AM Vidya Sagar <[email protected]> wrote:
>
> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
> incorrectly uses pci->link_gen in deriving the index to the
> n_fts[] array also introducing the issue of accessing beyond the
> boundaries of array for greater than Gen-2 speeds. This change fixes
> that issue.
>
> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
> Signed-off-by: Vidya Sagar <[email protected]>
> Acked-by: Jingoo Han <[email protected]>
> ---
> V2:
> * Addressed review comments from Bjorn
> * Added "Acked-by: Jingoo Han <[email protected]>"
>
> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Rob Herring <[email protected]>
Hi Lorenzo,
Do you have any further comments for this patch?
Thanks,
Vidya Sagar
On 10/5/2022 3:20 AM, Rob Herring wrote:
> External email: Use caution opening links or attachments
>
>
> On Mon, Sep 26, 2022 at 6:19 AM Vidya Sagar <[email protected]> wrote:
>>
>> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
>> incorrectly uses pci->link_gen in deriving the index to the
>> n_fts[] array also introducing the issue of accessing beyond the
>> boundaries of array for greater than Gen-2 speeds. This change fixes
>> that issue.
>>
>> Fixes: aeaa0bfe8965 ("PCI: dwc: Move N_FTS setup to common setup")
>> Signed-off-by: Vidya Sagar <[email protected]>
>> Acked-by: Jingoo Han <[email protected]>
>> ---
>> V2:
>> * Addressed review comments from Bjorn
>> * Added "Acked-by: Jingoo Han <[email protected]>"
>>
>> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Rob Herring <[email protected]>
>
On Mon, 26 Sep 2022 16:49:23 +0530, Vidya Sagar wrote:
> commit aeaa0bfe89654 ("PCI: dwc: Move N_FTS setup to common setup")
> incorrectly uses pci->link_gen in deriving the index to the
> n_fts[] array also introducing the issue of accessing beyond the
> boundaries of array for greater than Gen-2 speeds. This change fixes
> that issue.
>
>
> [...]
Applied to pci/dwc, thanks!
[1/1] PCI: dwc: Fix n_fts[] array overrun
https://git.kernel.org/lpieralisi/pci/c/66110361281b
Thanks,
Lorenzo