| Message ID | 20220624143947.8991-14-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8a5e57e5c936faebfeab350c6b6804e3142c44d6 |
| Headers | show |
| Series | PCI: dwc: Add hw version and dma-ranges support | expand |
On Fri, Jun 24, 2022 at 05:39:45PM +0300, Serge Semin wrote: > Since the DWC PCIe driver private data now contains the iATU inbound and > outbound regions constraints info like alignment, minimum and maximum > limits, we can use them to make the in- and outbound iATU regions setup > methods more strict to the ranges a callee tries to specify. That will > give us the safer dw_pcie_prog_outbound_atu(), > dw_pcie_prog_ep_outbound_atu() and dw_pcie_prog_inbound_atu() functions. > > First of all let's update the outbound ATU entries setup methods to > returning the operation status. The methods will fail either in case if > the range is failed to be activated or the passed region doesn't fulfill > iATU constraints. Secondly the passed to the > dw_pcie_prog_{ep_}outbound_atu() methods region-related parameters are > verified against the detected iATU regions constraints. In particular the > region limit address must not overflow the lower/upper limit CSR RW-fields > otherwise the specified range will be just silently clamped. That > verification will also protect the code from having u64 type overflow. > Secondly let's make sure base address (CPU-address), target address > (PCI-address) and size are properly aligned. Unaligned ranges will be > silently aligned down (addresses) and up (limit) on writing the values to > the corresponding registers, which in it turn may lead to unpredictable > results like ranges virtual overlap. Finally the CPU-address alignment > needs to be verified in the dw_pcie_prog_inbound_atu() method too as the > DWC PCIe RC/EP registers manual demands seeing the lower bits of the in- > and outbound iATU base address are always zeros. > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks, Mani > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > Changelog v3: > - Drop outbound iATU window size alignment constraint. (@Manivannan) > --- > drivers/pci/controller/dwc/pcie-designware.c | 38 +++++++++++++------- > drivers/pci/controller/dwc/pcie-designware.h | 10 +++--- > 2 files changed, 29 insertions(+), 19 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c > index 776752891d11..9c622b635fdd 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -8,6 +8,7 @@ > * Author: Jingoo Han <jg1.han@samsung.com> > */ > > +#include <linux/align.h> > #include <linux/bitops.h> > #include <linux/delay.h> > #include <linux/of.h> > @@ -308,9 +309,9 @@ static inline u32 dw_pcie_enable_ecrc(u32 val) > return val | PCIE_ATU_TD; > } > > -static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > - int index, int type, u64 cpu_addr, > - u64 pci_addr, u64 size) > +static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > + int index, int type, u64 cpu_addr, > + u64 pci_addr, u64 size) > { > u32 retries, val; > u64 limit_addr; > @@ -320,6 +321,12 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > > limit_addr = cpu_addr + size - 1; > > + if ((limit_addr & ~pci->region_limit) != (cpu_addr & ~pci->region_limit) || > + !IS_ALIGNED(cpu_addr, pci->region_align) || > + !IS_ALIGNED(pci_addr, pci->region_align) || !size) { > + return -EINVAL; > + } > + > dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_BASE, > lower_32_bits(cpu_addr)); > dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_BASE, > @@ -353,27 +360,29 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, > for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) { > val = dw_pcie_readl_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2); > if (val & PCIE_ATU_ENABLE) > - return; > + return 0; > > mdelay(LINK_WAIT_IATU); > } > > dev_err(pci->dev, "Outbound iATU is not being enabled\n"); > + > + return -ETIMEDOUT; > } > > -void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, > - u64 cpu_addr, u64 pci_addr, u64 size) > +int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, > + u64 cpu_addr, u64 pci_addr, u64 size) > { > - __dw_pcie_prog_outbound_atu(pci, 0, index, type, > - cpu_addr, pci_addr, size); > + return __dw_pcie_prog_outbound_atu(pci, 0, index, type, > + cpu_addr, pci_addr, size); > } > > -void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, > - int type, u64 cpu_addr, u64 pci_addr, > - u64 size) > +int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, > + int type, u64 cpu_addr, u64 pci_addr, > + u64 size) > { > - __dw_pcie_prog_outbound_atu(pci, func_no, index, type, > - cpu_addr, pci_addr, size); > + return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, > + cpu_addr, pci_addr, size); > } > > static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg) > @@ -392,6 +401,9 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, > { > u32 retries, val; > > + if (!IS_ALIGNED(cpu_addr, pci->region_align)) > + return -EINVAL; > + > dw_pcie_writel_atu_ib(pci, index, PCIE_ATU_LOWER_TARGET, > lower_32_bits(cpu_addr)); > dw_pcie_writel_atu_ib(pci, index, PCIE_ATU_UPPER_TARGET, > diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h > index 25c86771c810..60f1ddc54933 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -304,12 +304,10 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val); > int dw_pcie_link_up(struct dw_pcie *pci); > void dw_pcie_upconfig_setup(struct dw_pcie *pci); > int dw_pcie_wait_for_link(struct dw_pcie *pci); > -void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, > - int type, u64 cpu_addr, u64 pci_addr, > - u64 size); > -void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, > - int type, u64 cpu_addr, u64 pci_addr, > - u64 size); > +int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, > + u64 cpu_addr, u64 pci_addr, u64 size); > +int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, > + int type, u64 cpu_addr, u64 pci_addr, u64 size); > int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, > int type, u64 cpu_addr, u8 bar); > void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index); > -- > 2.35.1 >
diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c index 776752891d11..9c622b635fdd 100644 --- a/drivers/pci/controller/dwc/pcie-designware.c +++ b/drivers/pci/controller/dwc/pcie-designware.c @@ -8,6 +8,7 @@ * Author: Jingoo Han <jg1.han@samsung.com> */ +#include <linux/align.h> #include <linux/bitops.h> #include <linux/delay.h> #include <linux/of.h> @@ -308,9 +309,9 @@ static inline u32 dw_pcie_enable_ecrc(u32 val) return val | PCIE_ATU_TD; } -static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, - int index, int type, u64 cpu_addr, - u64 pci_addr, u64 size) +static int __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, + int index, int type, u64 cpu_addr, + u64 pci_addr, u64 size) { u32 retries, val; u64 limit_addr; @@ -320,6 +321,12 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, limit_addr = cpu_addr + size - 1; + if ((limit_addr & ~pci->region_limit) != (cpu_addr & ~pci->region_limit) || + !IS_ALIGNED(cpu_addr, pci->region_align) || + !IS_ALIGNED(pci_addr, pci->region_align) || !size) { + return -EINVAL; + } + dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_LOWER_BASE, lower_32_bits(cpu_addr)); dw_pcie_writel_atu_ob(pci, index, PCIE_ATU_UPPER_BASE, @@ -353,27 +360,29 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES; retries++) { val = dw_pcie_readl_atu_ob(pci, index, PCIE_ATU_REGION_CTRL2); if (val & PCIE_ATU_ENABLE) - return; + return 0; mdelay(LINK_WAIT_IATU); } dev_err(pci->dev, "Outbound iATU is not being enabled\n"); + + return -ETIMEDOUT; } -void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, - u64 cpu_addr, u64 pci_addr, u64 size) +int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, + u64 cpu_addr, u64 pci_addr, u64 size) { - __dw_pcie_prog_outbound_atu(pci, 0, index, type, - cpu_addr, pci_addr, size); + return __dw_pcie_prog_outbound_atu(pci, 0, index, type, + cpu_addr, pci_addr, size); } -void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, - int type, u64 cpu_addr, u64 pci_addr, - u64 size) +int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, + int type, u64 cpu_addr, u64 pci_addr, + u64 size) { - __dw_pcie_prog_outbound_atu(pci, func_no, index, type, - cpu_addr, pci_addr, size); + return __dw_pcie_prog_outbound_atu(pci, func_no, index, type, + cpu_addr, pci_addr, size); } static inline u32 dw_pcie_readl_atu_ib(struct dw_pcie *pci, u32 index, u32 reg) @@ -392,6 +401,9 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, { u32 retries, val; + if (!IS_ALIGNED(cpu_addr, pci->region_align)) + return -EINVAL; + dw_pcie_writel_atu_ib(pci, index, PCIE_ATU_LOWER_TARGET, lower_32_bits(cpu_addr)); dw_pcie_writel_atu_ib(pci, index, PCIE_ATU_UPPER_TARGET, diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h index 25c86771c810..60f1ddc54933 100644 --- a/drivers/pci/controller/dwc/pcie-designware.h +++ b/drivers/pci/controller/dwc/pcie-designware.h @@ -304,12 +304,10 @@ void dw_pcie_write_dbi2(struct dw_pcie *pci, u32 reg, size_t size, u32 val); int dw_pcie_link_up(struct dw_pcie *pci); void dw_pcie_upconfig_setup(struct dw_pcie *pci); int dw_pcie_wait_for_link(struct dw_pcie *pci); -void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, - int type, u64 cpu_addr, u64 pci_addr, - u64 size); -void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, - int type, u64 cpu_addr, u64 pci_addr, - u64 size); +int dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int index, int type, + u64 cpu_addr, u64 pci_addr, u64 size); +int dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int index, + int type, u64 cpu_addr, u64 pci_addr, u64 size); int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no, int index, int type, u64 cpu_addr, u8 bar); void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index);
Since the DWC PCIe driver private data now contains the iATU inbound and outbound regions constraints info like alignment, minimum and maximum limits, we can use them to make the in- and outbound iATU regions setup methods more strict to the ranges a callee tries to specify. That will give us the safer dw_pcie_prog_outbound_atu(), dw_pcie_prog_ep_outbound_atu() and dw_pcie_prog_inbound_atu() functions. First of all let's update the outbound ATU entries setup methods to returning the operation status. The methods will fail either in case if the range is failed to be activated or the passed region doesn't fulfill iATU constraints. Secondly the passed to the dw_pcie_prog_{ep_}outbound_atu() methods region-related parameters are verified against the detected iATU regions constraints. In particular the region limit address must not overflow the lower/upper limit CSR RW-fields otherwise the specified range will be just silently clamped. That verification will also protect the code from having u64 type overflow. Secondly let's make sure base address (CPU-address), target address (PCI-address) and size are properly aligned. Unaligned ranges will be silently aligned down (addresses) and up (limit) on writing the values to the corresponding registers, which in it turn may lead to unpredictable results like ranges virtual overlap. Finally the CPU-address alignment needs to be verified in the dw_pcie_prog_inbound_atu() method too as the DWC PCIe RC/EP registers manual demands seeing the lower bits of the in- and outbound iATU base address are always zeros. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Reviewed-by: Rob Herring <robh@kernel.org> --- Changelog v3: - Drop outbound iATU window size alignment constraint. (@Manivannan) --- drivers/pci/controller/dwc/pcie-designware.c | 38 +++++++++++++------- drivers/pci/controller/dwc/pcie-designware.h | 10 +++--- 2 files changed, 29 insertions(+), 19 deletions(-)