lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 15 Dec 2023 06:08:11 +0000
From: Jianjun Wang (王建军) <Jianjun.Wang@...iatek.com>
To: "robh@...nel.org" <robh@...nel.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "kw@...ux.com" <kw@...ux.com>,
	"lpieralisi@...nel.org" <lpieralisi@...nel.org>, "bhelgaas@...gle.com"
	<bhelgaas@...gle.com>, "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	Jieyy Yang (杨洁) <Jieyy.Yang@...iatek.com>,
	Chuanjia Liu (柳传嘉) <Chuanjia.Liu@...iatek.com>,
	Jian Yang (杨戬) <Jian.Yang@...iatek.com>,
	Qizhong Cheng (程啟忠)
	<Qizhong.Cheng@...iatek.com>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, Ryder Lee <Ryder.Lee@...iatek.com>
Subject: Re: [PATCH v2] PCI: mediatek-gen3: Fix translation window

Hi Bjorn, Lorenzo,

Sorry to bother you, but would you mind taking a look at this patch?
Appreciate any suggestions.

Thanks.

On Thu, 2023-11-23 at 01:27 +0000, Jianjun Wang (王建军) wrote:
> Hi Maintainers,
> 
> Gentle ping for this patch, if there is anything I need to do, please
> let me know.
> 
> Thanks.
> 
> On Mon, 2023-10-23 at 16:14 +0800, Jianjun Wang wrote:
> > The size of translation table should be a power of 2, using fls()
> > cannot get the proper value when the size is not a power of 2. For
> > example, fls(0x3e00000) - 1 = 25, hence the PCIe translation window
> > size will be set to 0x2000000 instead of the expected size
> > 0x3e00000.
> > 
> > Fix translation window by splitting the MMIO space to multiple
> > tables
> > if
> > its size is not a power of 2.
> > 
> > Fixes: d3bf75b579b9 ("PCI: mediatek-gen3: Add MediaTek Gen3 driver
> > for MT8192")
> > Signed-off-by: Jianjun Wang <jianjun.wang@...iatek.com>
> > 
> > ---
> > Changes in v2:
> > Only print warning message when reach the maximum translation table
> > number, the MEM space still works but will be smaller than
> > expected.
> > 
> > Bootup logs on MT8195 Platform:
> > 
> > > Before this patch:
> > 
> > mtk-pcie-gen3 112f0000.pcie: Parsing ranges property...
> > mtk-pcie-gen3 112f0000.pcie:       IO 0x0020000000..0x00201fffff ->
> > 0x0020000000
> > mtk-pcie-gen3 112f0000.pcie:      MEM 0x0020200000..0x0023ffffff ->
> > 0x0020200000
> > mtk-pcie-gen3 112f0000.pcie: set IO trans window[0]: cpu_addr =
> > 0x20000000, pci_addr = 0x20000000, size = 0x200000
> > mtk-pcie-gen3 112f0000.pcie: set MEM trans window[1]: cpu_addr =
> > 0x20200000, pci_addr = 0x20200000, size = 0x3e00000
> > 
> > > We expect the MEM trans window size to be 0x3e00000, but its
> > > actual
> > > available size is 0x2000000.
> > > After applying this patch:
> > 
> > mtk-pcie-gen3 112f0000.pcie: Parsing ranges property...
> > mtk-pcie-gen3 112f0000.pcie:       IO 0x0020000000..0x00201fffff ->
> > 0x0020000000
> > mtk-pcie-gen3 112f0000.pcie:      MEM 0x0020200000..0x0023ffffff ->
> > 0x0020200000
> > mtk-pcie-gen3 112f0000.pcie: set IO trans window[0]: cpu_addr =
> > 0x20000000, pci_addr = 0x20000000, size = 0x200000
> > mtk-pcie-gen3 112f0000.pcie: set MEM trans window[1]: cpu_addr =
> > 0x20200000, pci_addr = 0x20200000, size = 0x200000
> > mtk-pcie-gen3 112f0000.pcie: set MEM trans window[2]: cpu_addr =
> > 0x20400000, pci_addr = 0x20400000, size = 0x400000
> > mtk-pcie-gen3 112f0000.pcie: set MEM trans window[3]: cpu_addr =
> > 0x20800000, pci_addr = 0x20800000, size = 0x800000
> > mtk-pcie-gen3 112f0000.pcie: set MEM trans window[4]: cpu_addr =
> > 0x21000000, pci_addr = 0x21000000, size = 0x1000000
> > mtk-pcie-gen3 112f0000.pcie: set MEM trans window[5]: cpu_addr =
> > 0x22000000, pci_addr = 0x22000000, size = 0x2000000
> > 
> > > Total available size for MEM trans window is 0x3e00000.
> > 
> > ---
> > ---
> >  drivers/pci/controller/pcie-mediatek-gen3.c | 85 ++++++++++++-----
> > --
> > --
> >  1 file changed, 50 insertions(+), 35 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-mediatek-gen3.c
> > b/drivers/pci/controller/pcie-mediatek-gen3.c
> > index e0e27645fdf4..975b3024fb08 100644
> > --- a/drivers/pci/controller/pcie-mediatek-gen3.c
> > +++ b/drivers/pci/controller/pcie-mediatek-gen3.c
> > @@ -245,35 +245,60 @@ static int mtk_pcie_set_trans_table(struct
> > mtk_gen3_pcie *pcie,
> >  				    resource_size_t cpu_addr,
> >  				    resource_size_t pci_addr,
> >  				    resource_size_t size,
> > -				    unsigned long type, int num)
> > +				    unsigned long type, int *num)
> >  {
> > +	resource_size_t remaining = size;
> > +	resource_size_t table_size;
> > +	resource_size_t addr_align;
> > +	const char *range_type;
> >  	void __iomem *table;
> >  	u32 val;
> >  
> > -	if (num >= PCIE_MAX_TRANS_TABLES) {
> > -		dev_err(pcie->dev, "not enough translate table for
> > addr: %#llx, limited to [%d]\n",
> > -			(unsigned long long)cpu_addr,
> > PCIE_MAX_TRANS_TABLES);
> > -		return -ENODEV;
> > -	}
> > +	while (remaining && (*num < PCIE_MAX_TRANS_TABLES)) {
> > +		/* Table size needs to be a power of 2 */
> > +		table_size = BIT(fls(remaining) - 1);
> > +
> > +		if (cpu_addr > 0) {
> > +			addr_align = BIT(ffs(cpu_addr) - 1);
> > +			table_size = min(table_size, addr_align);
> > +		}
> > +
> > +		/* Minimum size of translate table is 4KiB */
> > +		if (table_size < 0x1000) {
> > +			dev_err(pcie->dev, "illegal table size
> > %#llx\n",
> > +				(unsigned long long)table_size);
> > +			return -EINVAL;
> > +		}
> >  
> > -	table = pcie->base + PCIE_TRANS_TABLE_BASE_REG +
> > -		num * PCIE_ATR_TLB_SET_OFFSET;
> > +		table = pcie->base + PCIE_TRANS_TABLE_BASE_REG + *num *
> > PCIE_ATR_TLB_SET_OFFSET;
> > +		writel_relaxed(lower_32_bits(cpu_addr) |
> > PCIE_ATR_SIZE(fls(table_size) - 1), table);
> > +		writel_relaxed(upper_32_bits(cpu_addr), table +
> > PCIE_ATR_SRC_ADDR_MSB_OFFSET);
> > +		writel_relaxed(lower_32_bits(pci_addr), table +
> > PCIE_ATR_TRSL_ADDR_LSB_OFFSET);
> > +		writel_relaxed(upper_32_bits(pci_addr), table +
> > PCIE_ATR_TRSL_ADDR_MSB_OFFSET);
> >  
> > -	writel_relaxed(lower_32_bits(cpu_addr) |
> > PCIE_ATR_SIZE(fls(size) - 1),
> > -		       table);
> > -	writel_relaxed(upper_32_bits(cpu_addr),
> > -		       table + PCIE_ATR_SRC_ADDR_MSB_OFFSET);
> > -	writel_relaxed(lower_32_bits(pci_addr),
> > -		       table + PCIE_ATR_TRSL_ADDR_LSB_OFFSET);
> > -	writel_relaxed(upper_32_bits(pci_addr),
> > -		       table + PCIE_ATR_TRSL_ADDR_MSB_OFFSET);
> > +		if (type == IORESOURCE_IO) {
> > +			val = PCIE_ATR_TYPE_IO | PCIE_ATR_TLP_TYPE_IO;
> > +			range_type = "IO";
> > +		} else {
> > +			val = PCIE_ATR_TYPE_MEM |
> > PCIE_ATR_TLP_TYPE_MEM;
> > +			range_type = "MEM";
> > +		}
> >  
> > -	if (type == IORESOURCE_IO)
> > -		val = PCIE_ATR_TYPE_IO | PCIE_ATR_TLP_TYPE_IO;
> > -	else
> > -		val = PCIE_ATR_TYPE_MEM | PCIE_ATR_TLP_TYPE_MEM;
> > +		writel_relaxed(val, table +
> > PCIE_ATR_TRSL_PARAM_OFFSET);
> >  
> > -	writel_relaxed(val, table + PCIE_ATR_TRSL_PARAM_OFFSET);
> > +		dev_dbg(pcie->dev, "set %s trans window[%d]: cpu_addr =
> > %#llx, pci_addr = %#llx, size = %#llx\n",
> > +			range_type, *num, (unsigned long long)cpu_addr,
> > +			(unsigned long long)pci_addr, (unsigned long
> > long)table_size);
> > +
> > +		cpu_addr += table_size;
> > +		pci_addr += table_size;
> > +		remaining -= table_size;
> > +		(*num)++;
> > +	}
> > +
> > +	if (remaining)
> > +		dev_warn(pcie->dev, "not enough translate table for
> > addr: %#llx, limited to [%d]\n",
> > +			 (unsigned long long)cpu_addr,
> > PCIE_MAX_TRANS_TABLES);
> >  
> >  	return 0;
> >  }
> > @@ -380,30 +405,20 @@ static int mtk_pcie_startup_port(struct
> > mtk_gen3_pcie *pcie)
> >  		resource_size_t cpu_addr;
> >  		resource_size_t pci_addr;
> >  		resource_size_t size;
> > -		const char *range_type;
> >  
> > -		if (type == IORESOURCE_IO) {
> > +		if (type == IORESOURCE_IO)
> >  			cpu_addr = pci_pio_to_address(res->start);
> > -			range_type = "IO";
> > -		} else if (type == IORESOURCE_MEM) {
> > +		else if (type == IORESOURCE_MEM)
> >  			cpu_addr = res->start;
> > -			range_type = "MEM";
> > -		} else {
> > +		else
> >  			continue;
> > -		}
> >  
> >  		pci_addr = res->start - entry->offset;
> >  		size = resource_size(res);
> >  		err = mtk_pcie_set_trans_table(pcie, cpu_addr,
> > pci_addr, size,
> > -					       type, table_index);
> > +					       type, &table_index);
> >  		if (err)
> >  			return err;
> > -
> > -		dev_dbg(pcie->dev, "set %s trans window[%d]: cpu_addr =
> > %#llx, pci_addr = %#llx, size = %#llx\n",
> > -			range_type, table_index, (unsigned long
> > long)cpu_addr,
> > -			(unsigned long long)pci_addr, (unsigned long
> > long)size);
> > -
> > -		table_index++;
> >  	}
> >  
> >  	return 0;

Powered by blists - more mailing lists