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@xxxxxxxxxxxxxxxxxxxx>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
Thanks,
Mani
> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
>
> ---
>
> 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@xxxxxxxxxxx>
> */
>
> +#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
>
--
மணிவண்ணன் சதாசிவம்
