linux-kernel - Re: [PATCH] PCI: mediatek-gen3: Fix translation window

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]

Message-ID: <54ed1269-8699-4531-abc6-09b602adece9@baylibre.com>
Date:   Thu, 12 Oct 2023 12:27:28 +0200
From:   Alexandre Mergnat <amergnat@...libre.com>
To:     Jianjun Wang (王建军) 
        <Jianjun.Wang@...iatek.com>, "robh@...nel.org" <robh@...nel.org>,
        "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>,
        "matthias.bgg@...il.com" <matthias.bgg@...il.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] PCI: mediatek-gen3: Fix translation window



On 12/10/2023 08:17, Jianjun Wang (王建军) wrote:
> On Wed, 2023-10-11 at 17:38 +0200, Alexandre Mergnat wrote:
>>   
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  
>> 
>> On 11/10/2023 14:26, 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.
>> 
>> Hi Jianjun,
>> 
>> I've no knowledge in PCIE, so maybe what my suggestion is stupid:
>> 
>> Is it mandatory to fit the translation table size with 0x3e00000 (in 
>> this example) ?
>> I'm asking because you can have an issue by reaching the maximum 
>> translation table number.
>> 
>> Is it possible to just use only one table with the power of 2 size
>> above 
>> 0x3e00000 => 0x4000000 ( fls(0x3e00000) = 26 = 0x4000000). The
>> downside 
>> of this method is wasting allocation space. AFAIK I already see this 
>> kind of method for memory protection/allocation in embedded systems,
>> so 
>> I'm wondering if this method is safer than using multiple table for
>> only 
>> one size which isn't a power of 2.
> 
> Hi Alexandre,
> 
> It's not mandatory to fit the translation table size with 0x3e00000,
> and yes we can use only one table with the power of 2 size to prevent
> this.
> 
> For MediaTek's SoCs, the MMIO space range for each PCIe port is fixed,
> and it will always be a power of 2, most of them will be 64MB. The
> reason we have the size which isn't a power of 2 is that we reserve an
> IO space for compatible purpose, some older devices may still use IO
> space.
> 
> Take MT8195 as an example, its MMIO size is 64MB, and the declaration
> in the DT is like:
> ranges = <0x81000000 0 0x20000000 0x0 0x20000000 0 0x200000>,
>           <0x82000000 0 0x20200000 0x0 0x20200000 0 0x3e00000>;
> 
> The MMIO space is splited to 2MB IO space and 62MB MEM space, that's
> cause the current risk of the MEM space range, its actual available MEM
> space is 32MB. But it still works for now because most of the devices
> only require a very small amount of MEM space and will not reach ranges
> higher than 32MB.
> 
> So for the concern of reaching the maximum translation table number, I
> think maybe we can just print the warning message instead of return
> error code, since it still works but have some limitations(MEM space
> not set as DT expected).
> 

Ok understood, thanks for your explanation.
Then, IMHO, you should use only one table with the power of 2 size above 
to make the code simpler, efficient, robust, more readable and avoid 
confusion about the warning.

This is what is done for pci-mvebu.c AFAII.

If you prefer waiting another reviewer with a better PCIE expertise than 
me, it's ok for me. With the information I have currently, I prefer to 
not approve the current implementation because, from my PoV, it 
introduce unnecessary complexity.

Thanks


-- 
Regards,
Alexandre