linux-kernel - Re: [PATCH v2 1/2] pcie-designware: add iATU Unroll feature

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] [day] [month] [year] [list]

Date:	Tue, 2 Aug 2016 15:27:56 +0100
From:	Joao Pinto <Joao.Pinto@...opsys.com>
To:	Bjorn Helgaas <helgaas@...nel.org>,
	Joao Pinto <Joao.Pinto@...opsys.com>
CC:	<jingoohan1@...il.com>, <linux-pci@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <jszhang@...vell.com>,
	<CARLOS.PALMINHA@...opsys.com>, <MiguelFalcao.Sousa@...opsys.com>
Subject: Re: [PATCH v2 1/2] pcie-designware: add iATU Unroll feature

On 8/2/2016 1:53 PM, Bjorn Helgaas wrote:
> On Tue, Aug 02, 2016 at 11:27:45AM +0100, Joao Pinto wrote:
>> On 7/25/2016 9:44 PM, Bjorn Helgaas wrote:
>>> On Fri, Jul 22, 2016 at 02:29:38PM +0100, Joao Pinto wrote:
>>>> This patch adds the support to the new iATU mechanism that will be used
>>>> from Core version 4.80, which is called iATU Unroll.
>>>> The new Cores can support the iATU Unroll or support the "old" iATU 
>>>> method now called Legacy Mode. The driver is perfectly capable of
>>>> performing well for both.
>>>>
>>>> In order to make sure that the iATU is really enabled a for loop was
>>>> introduced in dw_pcie_prog_outbound_atu() to improve reliability.
>>>>

(snip...)

>> ...
>>>> +static inline void dw_pcie_readl_unroll(struct pcie_port *pp, u32 type,
>>>> +						u32 index, u32 reg, u32 *val)
>>>> +{
>>>> +	u32 reg_addr = 0;
>>>> +
>>>> +	if (type == PCIE_TRANSL_OUTB)
>>>> +		reg_addr = PCIE_GET_ATU_OUTB_UNR_REG_ADDR(index, reg);
>>>> +	else if  (type == PCIE_TRANSL_INB)
>>>> +		reg_addr = PCIE_GET_ATU_INB_UNR_REG_ADDR(index, reg);
>>>
>>> PCIE_TRANSL_INB is never used.  In fact, dw_pcie_readl_unroll() is
>>> *always* called with PCIE_TRANSL_OUTB, so it's not clear what the
>>> value of passing it as an argument is.
>>
>> I included The Inbound translation mechanism because you can have Inbound or
>> Outbound translation in the iATU. The old mechanism also had Inbound properties
>> that are not used in the code. I suggest we keep the Inbound mechanism to avoid
>> future rework. Agree?
> 
> Again, not a huge deal, but since PCIE_TRANSL_INB is not used, it
> makes life harder for the reader, and the related code is not tested.
> So personally, I would remove it and add it back if/when it is needed.
> 
> Bjorn
> 

Ok, I will do that and send a new version soon. Thanks!

Joao