Messages in this thread | | | Date | Thu, 12 Oct 2023 15:30:12 +0200 | Subject | Re: [PATCH] PCI: mediatek-gen3: Fix translation window | From | Alexandre Mergnat <> |
| |
On 12/10/2023 14:52, AngeloGioacchino Del Regno wrote: > Il 12/10/23 12:27, Alexandre Mergnat ha scritto: >> >> >> 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. >> > > From what I understand, using only one table with a size that is a > power of two > won't let us use the entire MMIO space, hence the only solution to allow > using > the entire range is to split to more than one table.
You can take the power of 2 above, which is directly returned by fls(). That let us use the entire MMIO space. In this example, if your size is 0x3e00000, the you will allow 0x4000000.
-- Regards, Alexandre
| |