Re: [PATCH 3/3] PCI: dwc: ep: Improve alignment check in dw_pcie_prog_ep_inbound_atu()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Nov 14, 2024 at 12:03:29PM +0100, Niklas Cassel wrote:

according to patch, subject look like

"Add 'address' alignment to 'size' check in dw_pcie_prog_ep_inbound_atu()."

> dw_pcie_prog_ep_inbound_atu() is used to program an inbound iATU in
> "BAR Match Mode".
>
> While a memory address returned by kmalloc() is guaranteed to be naturally
> aligned (aligned to the size of the allocation), it is not guaranteed that
> the address that is supplied to pci_epc_set_bar() (and thus the addres that
> is supplied to dw_pcie_prog_ep_inbound_atu()) is naturally aligned.

short sentence may be better

The memory address returned by kmalloc() is guaranteed to be naturally
aligned (aligned to the size of the allocation), but it may not align when
pass to pci_epc_set_bar().

>
> See the register description for IATU_LWR_TARGET_ADDR_OFF_INBOUND_i,
> specifically fields LWR_TARGET_RW and LWR_TARGET_HW in the DWC Databook.
>
> "Field size depends on log2(BAR_MASK+1) in BAR match mode."
>
> I.e. only the upper bits are writable, and the number of writable bits is
> dependent on the configured BAR_MASK.
>
> Add a check to ensure that the physical address programmed in the iATU is
> aligned to the size of the BAR (BAR_MASK+1), as without this, we can get
> hard to debug errors, as we could write to bits that are read-only (without
> getting a write error), which could cause the iATU to end up redirecting to
> a physical address that is different from the address that we intended.
>
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 8 +++++---
>  drivers/pci/controller/dwc/pcie-designware.c    | 5 +++--
>  drivers/pci/controller/dwc/pcie-designware.h    | 2 +-
>  3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 507e40bd18c8f..4ad6ebd2ea320 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -128,7 +128,8 @@ static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  }
>
>  static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> -				  dma_addr_t cpu_addr, enum pci_barno bar)
> +				  dma_addr_t cpu_addr, enum pci_barno bar,
> +				  size_t size)
>  {
>  	int ret;
>  	u32 free_win;
> @@ -145,7 +146,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
>  	}
>
>  	ret = dw_pcie_prog_ep_inbound_atu(pci, func_no, free_win, type,
> -					  cpu_addr, bar);
> +					  cpu_addr, bar, size);
>  	if (ret < 0) {
>  		dev_err(pci->dev, "Failed to program IB window\n");
>  		return ret;
> @@ -229,7 +230,8 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	else
>  		type = PCIE_ATU_TYPE_IO;
>
> -	ret = dw_pcie_ep_inbound_atu(ep, func_no, type, epf_bar->phys_addr, bar);
> +	ret = dw_pcie_ep_inbound_atu(ep, func_no, type, epf_bar->phys_addr, bar,
> +				     size);
>  	if (ret)
>  		return ret;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 6d6cbc8b5b2c6..3c683b6119c39 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -597,11 +597,12 @@ int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
>  }
>
>  int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -				int type, u64 cpu_addr, u8 bar)
> +				int type, u64 cpu_addr, u8 bar, size_t size)
>  {
>  	u32 retries, val;
>
> -	if (!IS_ALIGNED(cpu_addr, pci->region_align))
> +	if (!IS_ALIGNED(cpu_addr, pci->region_align) ||
> +	    !IS_ALIGNED(cpu_addr, size))
>  		return -EINVAL;
>
>  	dw_pcie_writel_atu_ib(pci, index, PCIE_ATU_LOWER_TARGET,
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 347ab74ac35aa..fc08727116725 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -491,7 +491,7 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
>  int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int type,
>  			     u64 cpu_addr, u64 pci_addr, u64 size);
>  int dw_pcie_prog_ep_inbound_atu(struct dw_pcie *pci, u8 func_no, int index,
> -				int type, u64 cpu_addr, u8 bar);
> +				int type, u64 cpu_addr, u8 bar, size_t size);
>  void dw_pcie_disable_atu(struct dw_pcie *pci, u32 dir, int index);
>  void dw_pcie_setup(struct dw_pcie *pci);
>  void dw_pcie_iatu_detect(struct dw_pcie *pci);
> --
> 2.47.0
>




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux