* [PATCH] PCI: dwc: Program outbound ATU upper limit register
@ 2020-04-01 23:58 Alan Mikhak
2020-05-05 10:29 ` Lorenzo Pieralisi
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Alan Mikhak @ 2020-04-01 23:58 UTC (permalink / raw)
To: linux-kernel, linux-pci, jingoohan1, gustavo.pimentel,
lorenzo.pieralisi, amurray, bhelgaas, kishon, paul.walmsley
Cc: Alan Mikhak
From: Alan Mikhak <alan.mikhak@sifive.com>
Function dw_pcie_prog_outbound_atu_unroll() does not program the upper
32-bit ATU limit register. Since ATU programming functions limit the
size of the translated region to 4GB by using a u32 size parameter,
these issues may combine into undefined behavior for resource sizes
with non-zero upper 32-bits.
For example, a 128GB address space starting at physical CPU address of
0x2000000000 with size of 0x2000000000 needs the following values
programmed into the lower and upper 32-bit limit registers:
0x3fffffff in the upper 32-bit limit register
0xffffffff in the lower 32-bit limit register
Currently, only the lower 32-bit limit register is programmed with a
value of 0xffffffff but the upper 32-bit limit register is not being
programmed. As a result, the upper 32-bit limit register remains at its
default value after reset of 0x0.
These issues may combine to produce undefined behavior since the ATU
limit address may be lower than the ATU base address. Programming the
upper ATU limit address register prevents such undefined behavior despite
the region size getting truncated due to the 32-bit size limit.
Signed-off-by: Alan Mikhak <alan.mikhak@sifive.com>
---
drivers/pci/controller/dwc/pcie-designware.c | 7 +++++--
drivers/pci/controller/dwc/pcie-designware.h | 3 ++-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
index 681548c88282..c92496e36fd5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.c
+++ b/drivers/pci/controller/dwc/pcie-designware.c
@@ -244,13 +244,16 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
u64 pci_addr, u32 size)
{
u32 retries, val;
+ u64 limit_addr = cpu_addr + size - 1;
dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
lower_32_bits(cpu_addr));
dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
upper_32_bits(cpu_addr));
- dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
- lower_32_bits(cpu_addr + size - 1));
+ dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_LIMIT,
+ lower_32_bits(limit_addr));
+ dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_LIMIT,
+ upper_32_bits(limit_addr));
dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
lower_32_bits(pci_addr));
dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index a22ea5982817..5ce1aef706c5 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -112,9 +112,10 @@
#define PCIE_ATU_UNR_REGION_CTRL2 0x04
#define PCIE_ATU_UNR_LOWER_BASE 0x08
#define PCIE_ATU_UNR_UPPER_BASE 0x0C
-#define PCIE_ATU_UNR_LIMIT 0x10
+#define PCIE_ATU_UNR_LOWER_LIMIT 0x10
#define PCIE_ATU_UNR_LOWER_TARGET 0x14
#define PCIE_ATU_UNR_UPPER_TARGET 0x18
+#define PCIE_ATU_UNR_UPPER_LIMIT 0x20
/*
* The default address offset between dbi_base and atu_base. Root controller
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: dwc: Program outbound ATU upper limit register
2020-04-01 23:58 [PATCH] PCI: dwc: Program outbound ATU upper limit register Alan Mikhak
@ 2020-05-05 10:29 ` Lorenzo Pieralisi
2020-05-05 11:24 ` Gustavo Pimentel
2020-05-05 11:16 ` Gustavo Pimentel
2020-05-12 10:52 ` Lorenzo Pieralisi
2 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Pieralisi @ 2020-05-05 10:29 UTC (permalink / raw)
To: Alan Mikhak, jingoohan1, gustavo.pimentel
Cc: linux-kernel, linux-pci, amurray, bhelgaas, kishon, paul.walmsley
On Wed, Apr 01, 2020 at 04:58:13PM -0700, Alan Mikhak wrote:
> From: Alan Mikhak <alan.mikhak@sifive.com>
>
> Function dw_pcie_prog_outbound_atu_unroll() does not program the upper
> 32-bit ATU limit register. Since ATU programming functions limit the
> size of the translated region to 4GB by using a u32 size parameter,
> these issues may combine into undefined behavior for resource sizes
> with non-zero upper 32-bits.
>
> For example, a 128GB address space starting at physical CPU address of
> 0x2000000000 with size of 0x2000000000 needs the following values
> programmed into the lower and upper 32-bit limit registers:
> 0x3fffffff in the upper 32-bit limit register
> 0xffffffff in the lower 32-bit limit register
>
> Currently, only the lower 32-bit limit register is programmed with a
> value of 0xffffffff but the upper 32-bit limit register is not being
> programmed. As a result, the upper 32-bit limit register remains at its
> default value after reset of 0x0.
>
> These issues may combine to produce undefined behavior since the ATU
> limit address may be lower than the ATU base address. Programming the
> upper ATU limit address register prevents such undefined behavior despite
> the region size getting truncated due to the 32-bit size limit.
>
> Signed-off-by: Alan Mikhak <alan.mikhak@sifive.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 7 +++++--
> drivers/pci/controller/dwc/pcie-designware.h | 3 ++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
I would appreciate some feedback and possibly and ACK from DWC
maintainers. Should this go to stable kernels ? It seems so,
let me know if we want to add a stable tag.
I will merge it, along with:
https://patchwork.kernel.org/patch/11468465/
Lorenzo
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 681548c88282..c92496e36fd5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -244,13 +244,16 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> u64 pci_addr, u32 size)
> {
> u32 retries, val;
> + u64 limit_addr = cpu_addr + size - 1;
>
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
> lower_32_bits(cpu_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
> upper_32_bits(cpu_addr));
> - dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> - lower_32_bits(cpu_addr + size - 1));
> + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_LIMIT,
> + lower_32_bits(limit_addr));
> + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_LIMIT,
> + upper_32_bits(limit_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
> lower_32_bits(pci_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index a22ea5982817..5ce1aef706c5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -112,9 +112,10 @@
> #define PCIE_ATU_UNR_REGION_CTRL2 0x04
> #define PCIE_ATU_UNR_LOWER_BASE 0x08
> #define PCIE_ATU_UNR_UPPER_BASE 0x0C
> -#define PCIE_ATU_UNR_LIMIT 0x10
> +#define PCIE_ATU_UNR_LOWER_LIMIT 0x10
> #define PCIE_ATU_UNR_LOWER_TARGET 0x14
> #define PCIE_ATU_UNR_UPPER_TARGET 0x18
> +#define PCIE_ATU_UNR_UPPER_LIMIT 0x20
>
> /*
> * The default address offset between dbi_base and atu_base. Root controller
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] PCI: dwc: Program outbound ATU upper limit register
2020-04-01 23:58 [PATCH] PCI: dwc: Program outbound ATU upper limit register Alan Mikhak
2020-05-05 10:29 ` Lorenzo Pieralisi
@ 2020-05-05 11:16 ` Gustavo Pimentel
2020-05-12 10:52 ` Lorenzo Pieralisi
2 siblings, 0 replies; 5+ messages in thread
From: Gustavo Pimentel @ 2020-05-05 11:16 UTC (permalink / raw)
To: Alan Mikhak, linux-kernel, linux-pci, jingoohan1,
lorenzo.pieralisi, amurray, bhelgaas, kishon, paul.walmsley
On Thu, Apr 2, 2020 at 0:58:13, Alan Mikhak <alan.mikhak@sifive.com>
wrote:
> From: Alan Mikhak <alan.mikhak@sifive.com>
>
> Function dw_pcie_prog_outbound_atu_unroll() does not program the upper
> 32-bit ATU limit register. Since ATU programming functions limit the
> size of the translated region to 4GB by using a u32 size parameter,
> these issues may combine into undefined behavior for resource sizes
> with non-zero upper 32-bits.
>
> For example, a 128GB address space starting at physical CPU address of
> 0x2000000000 with size of 0x2000000000 needs the following values
> programmed into the lower and upper 32-bit limit registers:
> 0x3fffffff in the upper 32-bit limit register
> 0xffffffff in the lower 32-bit limit register
>
> Currently, only the lower 32-bit limit register is programmed with a
> value of 0xffffffff but the upper 32-bit limit register is not being
> programmed. As a result, the upper 32-bit limit register remains at its
> default value after reset of 0x0.
>
> These issues may combine to produce undefined behavior since the ATU
> limit address may be lower than the ATU base address. Programming the
> upper ATU limit address register prevents such undefined behavior despite
> the region size getting truncated due to the 32-bit size limit.
>
> Signed-off-by: Alan Mikhak <alan.mikhak@sifive.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 7 +++++--
> drivers/pci/controller/dwc/pcie-designware.h | 3 ++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 681548c88282..c92496e36fd5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -244,13 +244,16 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> u64 pci_addr, u32 size)
> {
> u32 retries, val;
> + u64 limit_addr = cpu_addr + size - 1;
>
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
> lower_32_bits(cpu_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
> upper_32_bits(cpu_addr));
> - dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> - lower_32_bits(cpu_addr + size - 1));
> + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_LIMIT,
> + lower_32_bits(limit_addr));
> + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_LIMIT,
> + upper_32_bits(limit_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
> lower_32_bits(pci_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index a22ea5982817..5ce1aef706c5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -112,9 +112,10 @@
> #define PCIE_ATU_UNR_REGION_CTRL2 0x04
> #define PCIE_ATU_UNR_LOWER_BASE 0x08
> #define PCIE_ATU_UNR_UPPER_BASE 0x0C
> -#define PCIE_ATU_UNR_LIMIT 0x10
> +#define PCIE_ATU_UNR_LOWER_LIMIT 0x10
> #define PCIE_ATU_UNR_LOWER_TARGET 0x14
> #define PCIE_ATU_UNR_UPPER_TARGET 0x18
> +#define PCIE_ATU_UNR_UPPER_LIMIT 0x20
>
> /*
> * The default address offset between dbi_base and atu_base. Root controller
> --
> 2.7.4
Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] PCI: dwc: Program outbound ATU upper limit register
2020-05-05 10:29 ` Lorenzo Pieralisi
@ 2020-05-05 11:24 ` Gustavo Pimentel
0 siblings, 0 replies; 5+ messages in thread
From: Gustavo Pimentel @ 2020-05-05 11:24 UTC (permalink / raw)
To: Lorenzo Pieralisi, Alan Mikhak, jingoohan1
Cc: linux-kernel, linux-pci, amurray, bhelgaas, kishon, paul.walmsley
Hi Lorenzo,
On Tue, May 5, 2020 at 11:29:33, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Apr 01, 2020 at 04:58:13PM -0700, Alan Mikhak wrote:
> > From: Alan Mikhak <alan.mikhak@sifive.com>
> >
> > Function dw_pcie_prog_outbound_atu_unroll() does not program the upper
> > 32-bit ATU limit register. Since ATU programming functions limit the
> > size of the translated region to 4GB by using a u32 size parameter,
> > these issues may combine into undefined behavior for resource sizes
> > with non-zero upper 32-bits.
> >
> > For example, a 128GB address space starting at physical CPU address of
> > 0x2000000000 with size of 0x2000000000 needs the following values
> > programmed into the lower and upper 32-bit limit registers:
> > 0x3fffffff in the upper 32-bit limit register
> > 0xffffffff in the lower 32-bit limit register
> >
> > Currently, only the lower 32-bit limit register is programmed with a
> > value of 0xffffffff but the upper 32-bit limit register is not being
> > programmed. As a result, the upper 32-bit limit register remains at its
> > default value after reset of 0x0.
> >
> > These issues may combine to produce undefined behavior since the ATU
> > limit address may be lower than the ATU base address. Programming the
> > upper ATU limit address register prevents such undefined behavior despite
> > the region size getting truncated due to the 32-bit size limit.
> >
> > Signed-off-by: Alan Mikhak <alan.mikhak@sifive.com>
> > ---
> > drivers/pci/controller/dwc/pcie-designware.c | 7 +++++--
> > drivers/pci/controller/dwc/pcie-designware.h | 3 ++-
> > 2 files changed, 7 insertions(+), 3 deletions(-)
>
> I would appreciate some feedback and possibly and ACK from DWC
> maintainers. Should this go to stable kernels ? It seems so,
> let me know if we want to add a stable tag.
>
> I will merge it, along with:
>
> https://urldefense.com/v3/__https://patchwork.kernel.org/patch/11468465/__;!!A4F2R9G_pg!NoymSJCWmOx51jB7LdQQAbXFin14nfuVIQNQxROnskLmmGkzFeNOrf8nFWX_-KgsgO87N9M$
>
> Lorenzo
Sorry for the delay. I just gave the ACK to that patch. For me, it makes
sense to me to send it along with the patch that you just referred to the
stable kernels.
-Gustavo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] PCI: dwc: Program outbound ATU upper limit register
2020-04-01 23:58 [PATCH] PCI: dwc: Program outbound ATU upper limit register Alan Mikhak
2020-05-05 10:29 ` Lorenzo Pieralisi
2020-05-05 11:16 ` Gustavo Pimentel
@ 2020-05-12 10:52 ` Lorenzo Pieralisi
2 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Pieralisi @ 2020-05-12 10:52 UTC (permalink / raw)
To: Alan Mikhak
Cc: linux-kernel, linux-pci, jingoohan1, gustavo.pimentel, amurray,
bhelgaas, kishon, paul.walmsley
On Wed, Apr 01, 2020 at 04:58:13PM -0700, Alan Mikhak wrote:
> From: Alan Mikhak <alan.mikhak@sifive.com>
>
> Function dw_pcie_prog_outbound_atu_unroll() does not program the upper
> 32-bit ATU limit register. Since ATU programming functions limit the
> size of the translated region to 4GB by using a u32 size parameter,
> these issues may combine into undefined behavior for resource sizes
> with non-zero upper 32-bits.
>
> For example, a 128GB address space starting at physical CPU address of
> 0x2000000000 with size of 0x2000000000 needs the following values
> programmed into the lower and upper 32-bit limit registers:
> 0x3fffffff in the upper 32-bit limit register
> 0xffffffff in the lower 32-bit limit register
>
> Currently, only the lower 32-bit limit register is programmed with a
> value of 0xffffffff but the upper 32-bit limit register is not being
> programmed. As a result, the upper 32-bit limit register remains at its
> default value after reset of 0x0.
>
> These issues may combine to produce undefined behavior since the ATU
> limit address may be lower than the ATU base address. Programming the
> upper ATU limit address register prevents such undefined behavior despite
> the region size getting truncated due to the 32-bit size limit.
>
> Signed-off-by: Alan Mikhak <alan.mikhak@sifive.com>
> ---
> drivers/pci/controller/dwc/pcie-designware.c | 7 +++++--
> drivers/pci/controller/dwc/pcie-designware.h | 3 ++-
> 2 files changed, 7 insertions(+), 3 deletions(-)
Applied to pci/dwc, thanks.
Lorenzo
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 681548c88282..c92496e36fd5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -244,13 +244,16 @@ static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> u64 pci_addr, u32 size)
> {
> u32 retries, val;
> + u64 limit_addr = cpu_addr + size - 1;
>
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_BASE,
> lower_32_bits(cpu_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_BASE,
> upper_32_bits(cpu_addr));
> - dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> - lower_32_bits(cpu_addr + size - 1));
> + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_LIMIT,
> + lower_32_bits(limit_addr));
> + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_LIMIT,
> + upper_32_bits(limit_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LOWER_TARGET,
> lower_32_bits(pci_addr));
> dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_UPPER_TARGET,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index a22ea5982817..5ce1aef706c5 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -112,9 +112,10 @@
> #define PCIE_ATU_UNR_REGION_CTRL2 0x04
> #define PCIE_ATU_UNR_LOWER_BASE 0x08
> #define PCIE_ATU_UNR_UPPER_BASE 0x0C
> -#define PCIE_ATU_UNR_LIMIT 0x10
> +#define PCIE_ATU_UNR_LOWER_LIMIT 0x10
> #define PCIE_ATU_UNR_LOWER_TARGET 0x14
> #define PCIE_ATU_UNR_UPPER_TARGET 0x18
> +#define PCIE_ATU_UNR_UPPER_LIMIT 0x20
>
> /*
> * The default address offset between dbi_base and atu_base. Root controller
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-12 10:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-01 23:58 [PATCH] PCI: dwc: Program outbound ATU upper limit register Alan Mikhak
2020-05-05 10:29 ` Lorenzo Pieralisi
2020-05-05 11:24 ` Gustavo Pimentel
2020-05-05 11:16 ` Gustavo Pimentel
2020-05-12 10:52 ` 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).