* [PATCH V4 1/2] PCI: rcar: Move the inbound index check
@ 2019-10-26 18:26 marek.vasut
2019-10-26 18:26 ` [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry marek.vasut
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: marek.vasut @ 2019-10-26 18:26 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Andrew Murray, Geert Uytterhoeven,
Lorenzo Pieralisi, Wolfram Sang, linux-renesas-soc
From: Marek Vasut <marek.vasut+renesas@gmail.com>
Since the $idx variable value is stored across multiple calls to
rcar_pcie_inbound_ranges() function, and the $idx value is used to
index registers which are written, subsequent calls might cause
the $idx value to be high enough to trigger writes into nonexistent
registers.
Fix this by moving the $idx value check to the beginning of the loop.
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
Reviewed-by: Andrew Murray <andrew.murray@arm.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-renesas-soc@vger.kernel.org
To: linux-pci@vger.kernel.org
---
V2: New patch
V3: Adjust the check to idx >= MAX_NR_INBOUND_MAPS - 1
V4: Rebase on next/master
---
drivers/pci/controller/pcie-rcar.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c
index e45bb2a7bfa5..b2a5c3e94245 100644
--- a/drivers/pci/controller/pcie-rcar.c
+++ b/drivers/pci/controller/pcie-rcar.c
@@ -1049,6 +1049,10 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
mask &= ~0xf;
while (cpu_addr < cpu_end) {
+ if (idx >= MAX_NR_INBOUND_MAPS - 1) {
+ dev_err(pcie->dev, "Failed to map inbound regions!\n");
+ return -EINVAL;
+ }
/*
* Set up 64-bit inbound regions as the range parser doesn't
* distinguish between 32 and 64-bit types.
@@ -1068,11 +1072,6 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie,
pci_addr += size;
cpu_addr += size;
idx += 2;
-
- if (idx > MAX_NR_INBOUND_MAPS) {
- dev_err(pcie->dev, "Failed to map inbound regions!\n");
- return -EINVAL;
- }
}
*index = idx;
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry 2019-10-26 18:26 [PATCH V4 1/2] PCI: rcar: Move the inbound index check marek.vasut @ 2019-10-26 18:26 ` marek.vasut 2019-10-28 8:35 ` Yoshihiro Shimoda 2019-10-28 8:24 ` [PATCH V4 1/2] PCI: rcar: Move the inbound index check Yoshihiro Shimoda 2019-10-29 11:37 ` Lorenzo Pieralisi 2 siblings, 1 reply; 9+ messages in thread From: marek.vasut @ 2019-10-26 18:26 UTC (permalink / raw) To: linux-pci Cc: Marek Vasut, Andrew Murray, Simon Horman, Geert Uytterhoeven, Lorenzo Pieralisi, Wolfram Sang, linux-renesas-soc From: Marek Vasut <marek.vasut+renesas@gmail.com> Due to hardware constraints, the size of each inbound range entry populated into the controller cannot be larger than the alignment of the entry's start address. Currently, the alignment for each "dma-ranges" inbound range is calculated only once for each range and the increment for programming the controller is also derived from it only once. Thus, a "dma-ranges" entry describing a memory at 0x48000000 and size 0x38000000 would lead to multiple controller entries, each 0x08000000 long. This is inefficient, especially considering that by adding the size to the start address, the alignment increases. This patch moves the alignment calculation into the loop populating the controller entries, thus updating the alignment for each controller entry. Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Reviewed-by: Andrew Murray <andrew.murray@arm.com> Reviewed-by: Simon Horman <horms+renesas@verge.net.au> Cc: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Wolfram Sang <wsa@the-dreams.de> Cc: linux-renesas-soc@vger.kernel.org To: linux-pci@vger.kernel.org --- V2: Update on top of 1/3 V3: No change V4: Rebase on next/master and dropped 2/3 patch --- drivers/pci/controller/pcie-rcar.c | 37 +++++++++++++++--------------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c index b2a5c3e94245..0dadccb61051 100644 --- a/drivers/pci/controller/pcie-rcar.c +++ b/drivers/pci/controller/pcie-rcar.c @@ -1030,29 +1030,30 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, if (restype & IORESOURCE_PREFETCH) flags |= LAM_PREFETCH; - /* - * If the size of the range is larger than the alignment of the start - * address, we have to use multiple entries to perform the mapping. - */ - if (cpu_addr > 0) { - unsigned long nr_zeros = __ffs64(cpu_addr); - u64 alignment = 1ULL << nr_zeros; - - size = min(range->size, alignment); - } else { - size = range->size; - } - /* Hardware supports max 4GiB inbound region */ - size = min(size, 1ULL << 32); - - mask = roundup_pow_of_two(size) - 1; - mask &= ~0xf; - while (cpu_addr < cpu_end) { if (idx >= MAX_NR_INBOUND_MAPS - 1) { dev_err(pcie->dev, "Failed to map inbound regions!\n"); return -EINVAL; } + /* + * If the size of the range is larger than the alignment of + * the start address, we have to use multiple entries to + * perform the mapping. + */ + if (cpu_addr > 0) { + unsigned long nr_zeros = __ffs64(cpu_addr); + u64 alignment = 1ULL << nr_zeros; + + size = min(range->size, alignment); + } else { + size = range->size; + } + /* Hardware supports max 4GiB inbound region */ + size = min(size, 1ULL << 32); + + mask = roundup_pow_of_two(size) - 1; + mask &= ~0xf; + /* * Set up 64-bit inbound regions as the range parser doesn't * distinguish between 32 and 64-bit types. -- 2.23.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* RE: [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry 2019-10-26 18:26 ` [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry marek.vasut @ 2019-10-28 8:35 ` Yoshihiro Shimoda 2019-10-28 10:20 ` Lorenzo Pieralisi 0 siblings, 1 reply; 9+ messages in thread From: Yoshihiro Shimoda @ 2019-10-28 8:35 UTC (permalink / raw) To: marek.vasut, linux-pci Cc: Marek Vasut, Andrew Murray, Simon Horman, Geert Uytterhoeven, Lorenzo Pieralisi, Wolfram Sang, linux-renesas-soc Hi Marek-san. > From: Marek Vasut, Sent: Sunday, October 27, 2019 3:27 AM > > Due to hardware constraints, the size of each inbound range entry > populated into the controller cannot be larger than the alignment > of the entry's start address. Currently, the alignment for each > "dma-ranges" inbound range is calculated only once for each range > and the increment for programming the controller is also derived > from it only once. Thus, a "dma-ranges" entry describing a memory > at 0x48000000 and size 0x38000000 would lead to multiple controller > entries, each 0x08000000 long. I added a debug code [1] and I confirmed that each entry is not 0x08000000 long [2]. After fixed the commit log above, Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> And I tested on r8a7795-salvator-xs with my debug code. So, Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda --- [1] Based on next-20191025 with this patch series: diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index fde6ec1..9bdd39e 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -2684,7 +2684,7 @@ 0x02000000 0 0x30000000 0 0x30000000 0 0x08000000 0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>; /* Map all possible DDR as inbound ranges */ - dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>; + dma-ranges = <0x42000000 0 0x48000000 0 0x48000000 0 0x38000000>; interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c index 0dadccb..54ad977 100644 --- a/drivers/pci/controller/pcie-rcar.c +++ b/drivers/pci/controller/pcie-rcar.c @@ -11,6 +11,8 @@ * Author: Phil Edworthy <phil.edworthy@renesas.com> */ +#define DEBUG + #include <linux/bitops.h> #include <linux/clk.h> #include <linux/delay.h> @@ -1054,6 +1056,8 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, mask = roundup_pow_of_two(size) - 1; mask &= ~0xf; + dev_dbg(pcie->dev, "idx%d: 0x%016llx..0x%016llx -> 0x%016llx\n", + idx, cpu_addr, size, pci_addr); /* * Set up 64-bit inbound regions as the range parser doesn't * distinguish between 32 and 64-bit types. --- [2] [ 0.374771] rcar-pcie fe000000.pcie: idx0: 0x0000000048000000..0x0000000008000000 -> 0x0000000048000000 [ 0.374777] rcar-pcie fe000000.pcie: idx2: 0x0000000050000000..0x0000000010000000 -> 0x0000000050000000 [ 0.374782] rcar-pcie fe000000.pcie: idx4: 0x0000000060000000..0x0000000020000000 -> 0x0000000060000000 --- ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry 2019-10-28 8:35 ` Yoshihiro Shimoda @ 2019-10-28 10:20 ` Lorenzo Pieralisi 2019-10-29 1:18 ` Yoshihiro Shimoda 0 siblings, 1 reply; 9+ messages in thread From: Lorenzo Pieralisi @ 2019-10-28 10:20 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: marek.vasut, linux-pci, Marek Vasut, Andrew Murray, Simon Horman, Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc On Mon, Oct 28, 2019 at 08:35:32AM +0000, Yoshihiro Shimoda wrote: > Hi Marek-san. > > > From: Marek Vasut, Sent: Sunday, October 27, 2019 3:27 AM > > > > Due to hardware constraints, the size of each inbound range entry > > populated into the controller cannot be larger than the alignment > > of the entry's start address. Currently, the alignment for each > > "dma-ranges" inbound range is calculated only once for each range > > and the increment for programming the controller is also derived > > from it only once. Thus, a "dma-ranges" entry describing a memory > > at 0x48000000 and size 0x38000000 would lead to multiple controller > > entries, each 0x08000000 long. > > I added a debug code [1] and I confirmed that each entry is not 0x08000000 long [2]. > > After fixed the commit log above, So what does this mean in practice ? Does it mean that the commit log is wrong or that the issue is not present as described, in the mainline code ? Please clarify and send a v5 accordingly. Thanks, Lorenzo > Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > And I tested on r8a7795-salvator-xs with my debug code. So, > > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > Best regards, > Yoshihiro Shimoda > > --- > [1] Based on next-20191025 with this patch series: > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > index fde6ec1..9bdd39e 100644 > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > @@ -2684,7 +2684,7 @@ > 0x02000000 0 0x30000000 0 0x30000000 0 0x08000000 > 0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>; > /* Map all possible DDR as inbound ranges */ > - dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>; > + dma-ranges = <0x42000000 0 0x48000000 0 0x48000000 0 0x38000000>; > interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>, > <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; > diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c > index 0dadccb..54ad977 100644 > --- a/drivers/pci/controller/pcie-rcar.c > +++ b/drivers/pci/controller/pcie-rcar.c > @@ -11,6 +11,8 @@ > * Author: Phil Edworthy <phil.edworthy@renesas.com> > */ > > +#define DEBUG > + > #include <linux/bitops.h> > #include <linux/clk.h> > #include <linux/delay.h> > @@ -1054,6 +1056,8 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, > mask = roundup_pow_of_two(size) - 1; > mask &= ~0xf; > > + dev_dbg(pcie->dev, "idx%d: 0x%016llx..0x%016llx -> 0x%016llx\n", > + idx, cpu_addr, size, pci_addr); > /* > * Set up 64-bit inbound regions as the range parser doesn't > * distinguish between 32 and 64-bit types. > --- > [2] > [ 0.374771] rcar-pcie fe000000.pcie: idx0: 0x0000000048000000..0x0000000008000000 -> 0x0000000048000000 > [ 0.374777] rcar-pcie fe000000.pcie: idx2: 0x0000000050000000..0x0000000010000000 -> 0x0000000050000000 > [ 0.374782] rcar-pcie fe000000.pcie: idx4: 0x0000000060000000..0x0000000020000000 -> 0x0000000060000000 > --- > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry 2019-10-28 10:20 ` Lorenzo Pieralisi @ 2019-10-29 1:18 ` Yoshihiro Shimoda 2019-10-29 10:34 ` Lorenzo Pieralisi 0 siblings, 1 reply; 9+ messages in thread From: Yoshihiro Shimoda @ 2019-10-29 1:18 UTC (permalink / raw) To: Lorenzo Pieralisi, marek.vasut Cc: linux-pci, Marek Vasut, Andrew Murray, Simon Horman, Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc Hi Lorenzo, > From: Lorenzo Pieralisi, Sent: Monday, October 28, 2019 7:21 PM > > On Mon, Oct 28, 2019 at 08:35:32AM +0000, Yoshihiro Shimoda wrote: > > Hi Marek-san. > > > > > From: Marek Vasut, Sent: Sunday, October 27, 2019 3:27 AM > > > > > > Due to hardware constraints, the size of each inbound range entry > > > populated into the controller cannot be larger than the alignment > > > of the entry's start address. Currently, the alignment for each > > > "dma-ranges" inbound range is calculated only once for each range > > > and the increment for programming the controller is also derived > > > from it only once. Thus, a "dma-ranges" entry describing a memory > > > at 0x48000000 and size 0x38000000 would lead to multiple controller > > > entries, each 0x08000000 long. > > > > I added a debug code [1] and I confirmed that each entry is not 0x08000000 long [2]. > > > > After fixed the commit log above, > > So what does this mean in practice ? Does it mean that the commit log is > wrong or that the issue is not present as described, in the mainline > code ? I meant the commit log is wrong. In such the case, the multiple controller entries has 3 kind of size like below. > > + dev_dbg(pcie->dev, "idx%d: 0x%016llx..0x%016llx -> 0x%016llx\n", > > + idx, cpu_addr, size, pci_addr); <snip> > > [ 0.374771] rcar-pcie fe000000.pcie: idx0: 0x0000000048000000..0x0000000008000000 -> 0x0000000048000000 The first entry's size is 0x08000000. > > [ 0.374777] rcar-pcie fe000000.pcie: idx2: 0x0000000050000000..0x0000000010000000 -> 0x0000000050000000 The second one's size is 0x10000000. > > [ 0.374782] rcar-pcie fe000000.pcie: idx4: 0x0000000060000000..0x0000000020000000 -> 0x0000000060000000 The third one's size is 0x20000000. > Please clarify and send a v5 accordingly. Marek-san, would you send a v5 patch series? Best regards, Yoshihiro Shimoda > Thanks, > Lorenzo > > > Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > And I tested on r8a7795-salvator-xs with my debug code. So, > > > > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > Best regards, > > Yoshihiro Shimoda > > > > --- > > [1] Based on next-20191025 with this patch series: > > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > index fde6ec1..9bdd39e 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > @@ -2684,7 +2684,7 @@ > > 0x02000000 0 0x30000000 0 0x30000000 0 0x08000000 > > 0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>; > > /* Map all possible DDR as inbound ranges */ > > - dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>; > > + dma-ranges = <0x42000000 0 0x48000000 0 0x48000000 0 0x38000000>; > > interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>, > > <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; > > diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c > > index 0dadccb..54ad977 100644 > > --- a/drivers/pci/controller/pcie-rcar.c > > +++ b/drivers/pci/controller/pcie-rcar.c > > @@ -11,6 +11,8 @@ > > * Author: Phil Edworthy <phil.edworthy@renesas.com> > > */ > > > > +#define DEBUG > > + > > #include <linux/bitops.h> > > #include <linux/clk.h> > > #include <linux/delay.h> > > @@ -1054,6 +1056,8 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, > > mask = roundup_pow_of_two(size) - 1; > > mask &= ~0xf; > > > > + dev_dbg(pcie->dev, "idx%d: 0x%016llx..0x%016llx -> 0x%016llx\n", > > + idx, cpu_addr, size, pci_addr); > > /* > > * Set up 64-bit inbound regions as the range parser doesn't > > * distinguish between 32 and 64-bit types. > > --- > > [2] > > [ 0.374771] rcar-pcie fe000000.pcie: idx0: 0x0000000048000000..0x0000000008000000 -> 0x0000000048000000 > > [ 0.374777] rcar-pcie fe000000.pcie: idx2: 0x0000000050000000..0x0000000010000000 -> 0x0000000050000000 > > [ 0.374782] rcar-pcie fe000000.pcie: idx4: 0x0000000060000000..0x0000000020000000 -> 0x0000000060000000 > > --- > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry 2019-10-29 1:18 ` Yoshihiro Shimoda @ 2019-10-29 10:34 ` Lorenzo Pieralisi 2019-10-29 11:12 ` Yoshihiro Shimoda 0 siblings, 1 reply; 9+ messages in thread From: Lorenzo Pieralisi @ 2019-10-29 10:34 UTC (permalink / raw) To: Yoshihiro Shimoda Cc: marek.vasut, linux-pci, Marek Vasut, Andrew Murray, Simon Horman, Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc On Tue, Oct 29, 2019 at 01:18:04AM +0000, Yoshihiro Shimoda wrote: > Hi Lorenzo, > > > From: Lorenzo Pieralisi, Sent: Monday, October 28, 2019 7:21 PM > > > > On Mon, Oct 28, 2019 at 08:35:32AM +0000, Yoshihiro Shimoda wrote: > > > Hi Marek-san. > > > > > > > From: Marek Vasut, Sent: Sunday, October 27, 2019 3:27 AM > > > > > > > > Due to hardware constraints, the size of each inbound range entry > > > > populated into the controller cannot be larger than the alignment > > > > of the entry's start address. Currently, the alignment for each > > > > "dma-ranges" inbound range is calculated only once for each range > > > > and the increment for programming the controller is also derived > > > > from it only once. Thus, a "dma-ranges" entry describing a memory > > > > at 0x48000000 and size 0x38000000 would lead to multiple controller > > > > entries, each 0x08000000 long. > > > > > > I added a debug code [1] and I confirmed that each entry is not 0x08000000 long [2]. > > > > > > After fixed the commit log above, > > > > So what does this mean in practice ? Does it mean that the commit log is > > wrong or that the issue is not present as described, in the mainline > > code ? > > I meant the commit log is wrong. In such the case, the multiple controller > entries has 3 kind of size like below. OK, that's confusing. The commit log is describing the issue it is fixing and you are reporting that's not what happens in practice, so in short my question is, is it possible to describe the issue you are fixing with an example representative of what's happening and explaining why we need to apply this patch please ? Thanks, Lorenzo > > > + dev_dbg(pcie->dev, "idx%d: 0x%016llx..0x%016llx -> 0x%016llx\n", > > > + idx, cpu_addr, size, pci_addr); > <snip> > > > [ 0.374771] rcar-pcie fe000000.pcie: idx0: 0x0000000048000000..0x0000000008000000 -> 0x0000000048000000 > > The first entry's size is 0x08000000. > > > > [ 0.374777] rcar-pcie fe000000.pcie: idx2: 0x0000000050000000..0x0000000010000000 -> 0x0000000050000000 > > The second one's size is 0x10000000. > > > > [ 0.374782] rcar-pcie fe000000.pcie: idx4: 0x0000000060000000..0x0000000020000000 -> 0x0000000060000000 > > The third one's size is 0x20000000. > > > Please clarify and send a v5 accordingly. > > Marek-san, would you send a v5 patch series? > > Best regards, > Yoshihiro Shimoda > > > Thanks, > > Lorenzo > > > > > Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > And I tested on r8a7795-salvator-xs with my debug code. So, > > > > > > Tested-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > > Best regards, > > > Yoshihiro Shimoda > > > > > > --- > > > [1] Based on next-20191025 with this patch series: > > > diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > > index fde6ec1..9bdd39e 100644 > > > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > > @@ -2684,7 +2684,7 @@ > > > 0x02000000 0 0x30000000 0 0x30000000 0 0x08000000 > > > 0x42000000 0 0x38000000 0 0x38000000 0 0x08000000>; > > > /* Map all possible DDR as inbound ranges */ > > > - dma-ranges = <0x42000000 0 0x40000000 0 0x40000000 0 0x40000000>; > > > + dma-ranges = <0x42000000 0 0x48000000 0 0x48000000 0 0x38000000>; > > > interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>, > > > <GIC_SPI 117 IRQ_TYPE_LEVEL_HIGH>, > > > <GIC_SPI 118 IRQ_TYPE_LEVEL_HIGH>; > > > diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c > > > index 0dadccb..54ad977 100644 > > > --- a/drivers/pci/controller/pcie-rcar.c > > > +++ b/drivers/pci/controller/pcie-rcar.c > > > @@ -11,6 +11,8 @@ > > > * Author: Phil Edworthy <phil.edworthy@renesas.com> > > > */ > > > > > > +#define DEBUG > > > + > > > #include <linux/bitops.h> > > > #include <linux/clk.h> > > > #include <linux/delay.h> > > > @@ -1054,6 +1056,8 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, > > > mask = roundup_pow_of_two(size) - 1; > > > mask &= ~0xf; > > > > > > + dev_dbg(pcie->dev, "idx%d: 0x%016llx..0x%016llx -> 0x%016llx\n", > > > + idx, cpu_addr, size, pci_addr); > > > /* > > > * Set up 64-bit inbound regions as the range parser doesn't > > > * distinguish between 32 and 64-bit types. > > > --- > > > [2] > > > [ 0.374771] rcar-pcie fe000000.pcie: idx0: 0x0000000048000000..0x0000000008000000 -> 0x0000000048000000 > > > [ 0.374777] rcar-pcie fe000000.pcie: idx2: 0x0000000050000000..0x0000000010000000 -> 0x0000000050000000 > > > [ 0.374782] rcar-pcie fe000000.pcie: idx4: 0x0000000060000000..0x0000000020000000 -> 0x0000000060000000 > > > --- > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry 2019-10-29 10:34 ` Lorenzo Pieralisi @ 2019-10-29 11:12 ` Yoshihiro Shimoda 0 siblings, 0 replies; 9+ messages in thread From: Yoshihiro Shimoda @ 2019-10-29 11:12 UTC (permalink / raw) To: Lorenzo Pieralisi Cc: marek.vasut, linux-pci, Marek Vasut, Andrew Murray, Simon Horman, Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc Hi Lorenzo, > From: Lorenzo Pieralisi, Sent: Tuesday, October 29, 2019 7:35 PM > > On Tue, Oct 29, 2019 at 01:18:04AM +0000, Yoshihiro Shimoda wrote: > > Hi Lorenzo, > > > > > From: Lorenzo Pieralisi, Sent: Monday, October 28, 2019 7:21 PM > > > > > > On Mon, Oct 28, 2019 at 08:35:32AM +0000, Yoshihiro Shimoda wrote: > > > > Hi Marek-san. > > > > > > > > > From: Marek Vasut, Sent: Sunday, October 27, 2019 3:27 AM > > > > > > > > > > Due to hardware constraints, the size of each inbound range entry > > > > > populated into the controller cannot be larger than the alignment > > > > > of the entry's start address. Currently, the alignment for each > > > > > "dma-ranges" inbound range is calculated only once for each range > > > > > and the increment for programming the controller is also derived > > > > > from it only once. Thus, a "dma-ranges" entry describing a memory > > > > > at 0x48000000 and size 0x38000000 would lead to multiple controller > > > > > entries, each 0x08000000 long. > > > > > > > > I added a debug code [1] and I confirmed that each entry is not 0x08000000 long [2]. > > > > > > > > After fixed the commit log above, > > > > > > So what does this mean in practice ? Does it mean that the commit log is > > > wrong or that the issue is not present as described, in the mainline > > > code ? > > > > I meant the commit log is wrong. In such the case, the multiple controller > > entries has 3 kind of size like below. > > OK, that's confusing. The commit log is describing the issue it is > fixing and you are reporting that's not what happens in practice, so in > short my question is, is it possible to describe the issue you > are fixing with an example representative of what's happening and > explaining why we need to apply this patch please ? I'm very sorry, I completely misunderstood the original commit description. I misunderstood "each 0x08000000 log" was a behavior of after we applied this patch... So, the description is no problem. In other words, we don't need to fix any description on this patch. Best regards, Yoshihiro Shimoda ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH V4 1/2] PCI: rcar: Move the inbound index check 2019-10-26 18:26 [PATCH V4 1/2] PCI: rcar: Move the inbound index check marek.vasut 2019-10-26 18:26 ` [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry marek.vasut @ 2019-10-28 8:24 ` Yoshihiro Shimoda 2019-10-29 11:37 ` Lorenzo Pieralisi 2 siblings, 0 replies; 9+ messages in thread From: Yoshihiro Shimoda @ 2019-10-28 8:24 UTC (permalink / raw) To: marek.vasut, linux-pci Cc: Marek Vasut, Andrew Murray, Geert Uytterhoeven, Lorenzo Pieralisi, Wolfram Sang, linux-renesas-soc Hi Marek-san, > From: Marek Vasut, Sent: Sunday, October 27, 2019 3:27 AM > > Since the $idx variable value is stored across multiple calls to > rcar_pcie_inbound_ranges() function, and the $idx value is used to > index registers which are written, subsequent calls might cause > the $idx value to be high enough to trigger writes into nonexistent > registers. > > Fix this by moving the $idx value check to the beginning of the loop. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Wolfram Sang <wsa@the-dreams.de> > Cc: linux-renesas-soc@vger.kernel.org > To: linux-pci@vger.kernel.org > --- Thank you for the patch! Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V4 1/2] PCI: rcar: Move the inbound index check 2019-10-26 18:26 [PATCH V4 1/2] PCI: rcar: Move the inbound index check marek.vasut 2019-10-26 18:26 ` [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry marek.vasut 2019-10-28 8:24 ` [PATCH V4 1/2] PCI: rcar: Move the inbound index check Yoshihiro Shimoda @ 2019-10-29 11:37 ` Lorenzo Pieralisi 2 siblings, 0 replies; 9+ messages in thread From: Lorenzo Pieralisi @ 2019-10-29 11:37 UTC (permalink / raw) To: marek.vasut Cc: linux-pci, Marek Vasut, Andrew Murray, Geert Uytterhoeven, Wolfram Sang, linux-renesas-soc On Sat, Oct 26, 2019 at 08:26:58PM +0200, marek.vasut@gmail.com wrote: > From: Marek Vasut <marek.vasut+renesas@gmail.com> > > Since the $idx variable value is stored across multiple calls to > rcar_pcie_inbound_ranges() function, and the $idx value is used to > index registers which are written, subsequent calls might cause > the $idx value to be high enough to trigger writes into nonexistent > registers. > > Fix this by moving the $idx value check to the beginning of the loop. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Reviewed-by: Andrew Murray <andrew.murray@arm.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Wolfram Sang <wsa@the-dreams.de> > Cc: linux-renesas-soc@vger.kernel.org > To: linux-pci@vger.kernel.org > --- > V2: New patch > V3: Adjust the check to idx >= MAX_NR_INBOUND_MAPS - 1 > V4: Rebase on next/master > --- > drivers/pci/controller/pcie-rcar.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) Applied both patches to pci/rcar, thanks ! Lorenzo > diff --git a/drivers/pci/controller/pcie-rcar.c b/drivers/pci/controller/pcie-rcar.c > index e45bb2a7bfa5..b2a5c3e94245 100644 > --- a/drivers/pci/controller/pcie-rcar.c > +++ b/drivers/pci/controller/pcie-rcar.c > @@ -1049,6 +1049,10 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, > mask &= ~0xf; > > while (cpu_addr < cpu_end) { > + if (idx >= MAX_NR_INBOUND_MAPS - 1) { > + dev_err(pcie->dev, "Failed to map inbound regions!\n"); > + return -EINVAL; > + } > /* > * Set up 64-bit inbound regions as the range parser doesn't > * distinguish between 32 and 64-bit types. > @@ -1068,11 +1072,6 @@ static int rcar_pcie_inbound_ranges(struct rcar_pcie *pcie, > pci_addr += size; > cpu_addr += size; > idx += 2; > - > - if (idx > MAX_NR_INBOUND_MAPS) { > - dev_err(pcie->dev, "Failed to map inbound regions!\n"); > - return -EINVAL; > - } > } > *index = idx; > > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-10-29 11:37 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-26 18:26 [PATCH V4 1/2] PCI: rcar: Move the inbound index check marek.vasut 2019-10-26 18:26 ` [PATCH V4 2/2] PCI: rcar: Recalculate inbound range alignment for each controller entry marek.vasut 2019-10-28 8:35 ` Yoshihiro Shimoda 2019-10-28 10:20 ` Lorenzo Pieralisi 2019-10-29 1:18 ` Yoshihiro Shimoda 2019-10-29 10:34 ` Lorenzo Pieralisi 2019-10-29 11:12 ` Yoshihiro Shimoda 2019-10-28 8:24 ` [PATCH V4 1/2] PCI: rcar: Move the inbound index check Yoshihiro Shimoda 2019-10-29 11:37 ` Lorenzo Pieralisi
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 NNTP newsgroup(s).