Re: [PATCH v5 01/10] arm64: dts: lx2160a: extend 32-bit, and add 64-bit pci regions

Am 11.05.26 um 23:20 schrieb [email protected]:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Incorrect PCI Address Space Code for 64-bit prefetchable memory windows.
> - [High] Fragmenting the 16GB region into 4GB chunks breaks allocation of large PCIe BARs.
> - [Medium] Incorrect size for the `pcie4` 64-bit memory region in `fsl-lx2160a-rev2.dtsi`.
> - [Low] Conflation of the Open Firmware `relocatable` bit with the Linux-internal `IORESOURCE_BUSY` macro.
> --
>
> commit bfb49a7c3467d68eca95268c94cbd76b1401fb15
> Author: Josua Mayer <[email protected]>
>
> arm64: dts: lx2160a: extend 32-bit, and add 64-bit pci regions
>
> This commit extends the 32-bit PCIe memory regions to 3GB and adds new
> 64-bit memory regions up to 16GB for the LX2160A SoC. It also drops the
> relocatable bit from the existing ranges, and splits the 16GB area into
> 4GB chunks for silicon revision 1 to avoid an ATU programming limitation.
>
> The commit message states:
>
>> - drop IORESOURCE_BUSY flag
>>
>> IORESOURCE_BUSY is dropped since it has no effect when specified in dts.
> Is this description accurate? The bit being dropped (0x80000000, changing
> 0x82000000 to 0x02000000) is the Open Firmware 'n' bit, which designates
> whether the region is relocatable or fixed. 
>
> It doesn't appear to be related to the Linux-internal IORESOURCE_BUSY macro.
> Could the commit message be more precise about the flag being modified?
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a-rev2.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a-rev2.dtsi
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a-rev2.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a-rev2.dtsi
> @@ -14,8 +14,9 @@ &pcie1 {
>  	      0x80 0x00000000 0x0 0x00002000>; /* configuration space */
>  	reg-names = "regs", "config";
>  
> -	ranges = <0x81000000 0x0 0x00000000 0x80 0x00010000 0x0 0x00010000
> -		  0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>;
> +	ranges = <0x42000000 0x84 0x00000000 0x84 0x00000000 0x04 0x00000000>, /* 64-Bit - prefetchable - 16GB */
>
> Does the 0x42000000 value correctly specify a 64-bit memory space?
>
> In the device tree PCI bindings, bits 24-25 of the phys.hi cell denote
> the address space. The value 0b10 (0x02000000 or 0x42000000) designates
> a 32-bit memory space, while 0b11 (0x43000000) is required for a 64-bit
> memory space.
>
> If it is flagged as 32-bit, the PCI core's of_pci_range_to_resource()
> will omit the IORESOURCE_MEM_64 flag.

Actually this is being fixed automatically, as can be seen on boot log:

[    0.978699] layerscape-pcie 3800000.pcie: host bridge /soc/pcie@3800000 ranges:
[    0.978715] layerscape-pcie 3800000.pcie:      MEM 0xa400000000..0xa7ffffffff -> 0xa400000000
[    0.978726] layerscape-pcie 3800000.pcie:      MEM 0xa040000000..0xa0ffffffff -> 0x0040000000
[    0.978734] layerscape-pcie 3800000.pcie:       IO 0xa000010000..0xa00001ffff -> 0x0000000000
[    0.979460] layerscape-pcie 3800000.pcie: iATU: unroll F, 256 ob, 24 ib, align 4K, limit 4G
[    1.184102] layerscape-pcie 3800000.pcie: PCIe Gen.3 x8 link up
[    1.184219] layerscape-pcie 3800000.pcie: PCI host bridge to bus 0000:00
[    1.184223] pci_bus 0000:00: root bus resource [bus 00-ff]
[    1.184227] pci_bus 0000:00: root bus resource [mem 0xa400000000-0xa7ffffffff pref]
[    1.184231] pci_bus 0000:00: root bus resource [mem 0xa040000000-0xa0ffffffff] (bus address [0x40000000-0xffffffff])
[    1.184234] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    1.184260] pci 0000:00:00.0: [1957:8d90] type 01 class 0x060400 PCIe Root Port
[    1.184283] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    1.184291] pci 0000:00:00.0:   bridge window [mem 0xa040000000-0xa0470fffff]
[    1.184350] pci 0000:00:00.0: supports D1 D2
[    1.184353] pci 0000:00:00.0: PME# supported from D0 D1 D2 D3hot
[    1.185707] pci 0000:01:00.0: [8086:1572] type 00 class 0x020000 PCIe Endpoint
[    1.186168] pci 0000:01:00.0: BAR 0 [mem 0xa040000000-0xa040ffffff 64bit pref]
[    1.186206] pci 0000:01:00.0: BAR 3 [mem 0xa041000000-0xa041007fff 64bit pref]
[    1.186231] pci 0000:01:00.0: ROM [mem 0xa041080000-0xa0410fffff pref]
[    1.187225] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    1.187779] pci 0000:01:00.0: VF BAR 0 [mem 0x00000000-0x0000ffff 64bit pref]
[    1.187782] pci 0000:01:00.0: VF BAR 0 [mem 0x00000000-0x001fffff 64bit pref]: contains BAR 0 for 32 VFs
[    1.187820] pci 0000:01:00.0: VF BAR 3 [mem 0x00000000-0x00003fff 64bit pref]
[    1.187823] pci 0000:01:00.0: VF BAR 3 [mem 0x00000000-0x0007ffff 64bit pref]: contains BAR 3 for 32 VFs
[    1.190514] pci 0000:01:00.1: [8086:1572] type 00 class 0x020000 PCIe Endpoint
...
[    1.206543] pci_bus 0000:00: max bus depth: 1 pci_try_num: 2
[    1.206556] pci 0000:00:00.0: bridge window [mem 0xa400000000-0xa404afffff 64bit pref]: assigned
[    1.206560] pci 0000:00:00.0: bridge window [mem 0xa040000000-0xa0401fffff]: assigned
[    1.206568] pci 0000:01:00.0: BAR 0 [mem 0xa400000000-0xa400ffffff 64bit pref]: assigned
[    1.206971] pci 0000:01:00.0: ROM [mem 0xa040000000-0xa04007ffff pref]: assigned
[    1.206982] pci 0000:01:00.0: VF BAR 0 [mem 0xa404000000-0xa4041fffff 64bit pref]: assigned
[    1.207161] pci 0000:01:00.0: BAR 3 [mem 0xa404800000-0xa404807fff 64bit pref]: assigned
[    1.207562] pci 0000:01:00.0: VF BAR 3 [mem 0xa404820000-0xa40489ffff 64bit pref]: assigned

Setting flags 0x43000000 changes bar assignment messages, and adds new errors:

[    0.967809] layerscape-pcie 3800000.pcie: host bridge /soc/pcie@3800000 ranges:
[    0.967830] layerscape-pcie 3800000.pcie:      MEM 0xa400000000..0xa7ffffffff -> 0xa400000000
[    0.967842] layerscape-pcie 3800000.pcie:      MEM 0xa040000000..0xa0ffffffff -> 0x0040000000
[    0.967849] layerscape-pcie 3800000.pcie:       IO 0xa000010000..0xa00001ffff -> 0x0000000000
[    0.968574] layerscape-pcie 3800000.pcie: iATU: unroll F, 256 ob, 24 ib, align 4K, limit 4G
[    1.167697] layerscape-pcie 3800000.pcie: PCIe Gen.3 x8 link up
[    1.167826] layerscape-pcie 3800000.pcie: PCI host bridge to bus 0000:00
[    1.167830] pci_bus 0000:00: root bus resource [bus 00-ff]
[    1.167834] pci_bus 0000:00: root bus resource [mem 0xa400000000-0xa7ffffffff pref]
[    1.167838] pci_bus 0000:00: root bus resource [mem 0xa040000000-0xa0ffffffff] (bus address [0x40000000-0xffffffff])
[    1.167841] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
[    1.167868] pci 0000:00:00.0: [1957:8d90] type 01 class 0x060400 PCIe Root Port
[    1.167891] pci 0000:00:00.0: PCI bridge to [bus 01-ff]
[    1.167955] pci 0000:00:00.0: supports D1 D2
[    1.167958] pci 0000:00:00.0: PME# supported from D0 D1 D2 D3hot
[    1.169315] pci 0000:01:00.0: [8086:1572] type 00 class 0x020000 PCIe Endpoint
[    1.169733] pci 0000:01:00.0: BAR 0 [mem 0x00000000-0x00ffffff 64bit pref]
[    1.169771] pci 0000:01:00.0: BAR 3 [mem 0x00000000-0x00007fff 64bit pref]
[    1.169796] pci 0000:01:00.0: ROM [mem 0x00000000-0x0007ffff pref]
[    1.170801] pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
[    1.171356] pci 0000:01:00.0: VF BAR 0 [mem 0x00000000-0x0000ffff 64bit pref]
[    1.171359] pci 0000:01:00.0: VF BAR 0 [mem 0x00000000-0x001fffff 64bit pref]: contains BAR 0 for 32 VFs
[    1.171396] pci 0000:01:00.0: VF BAR 3 [mem 0x00000000-0x00003fff 64bit pref]
[    1.171399] pci 0000:01:00.0: VF BAR 3 [mem 0x00000000-0x0007ffff 64bit pref]: contains BAR 3 for 32 VFs
[    1.173389] OF: /soc/pcie@3800000: no msi-map translation for id 0x100 on (null)
[    1.173515] OF: /soc/pcie@3800000: no iommu-map translation for id 0x100 on (null)
[    1.173937] pci 0000:01:00.1: [8086:1572] type 00 class 0x020000 PCIe Endpoint
...
[    1.186848] pci_bus 0000:00: max bus depth: 1 pci_try_num: 2
[    1.186861] pci 0000:00:00.0: bridge window [mem 0xa400000000-0xa404afffff 64bit pref]: assigned
[    1.186864] pci 0000:00:00.0: bridge window [mem 0xa040000000-0xa0401fffff]: assigned
[    1.186871] pci 0000:01:00.0: BAR 0 [mem 0xa400000000-0xa400ffffff 64bit pref]: assigned
[    1.187197] pci 0000:01:00.0: ROM [mem 0xa040000000-0xa04007ffff pref]: assigned
[    1.187208] pci 0000:01:00.0: VF BAR 0 [mem 0xa404000000-0xa4041fffff 64bit pref]: assigned
[    1.187355] pci 0000:01:00.0: BAR 3 [mem 0xa404800000-0xa404807fff 64bit pref]: assigned
[    1.187680] pci 0000:01:00.0: VF BAR 3 [mem 0xa404820000-0xa40489ffff 64bit pref]: assigned

It appears that bootloader fails initializing the non-VF bars ("mem 0x00000000-0x00ffffff 64bit pref"),
once the 64-bit mem flag is added.

So I shall leave the flags as they are now, without 64-bit.

> Will this cause the PCI allocator
> to fail when assigning 64-bit BARs, as shown in the commit log ("failed
> to assign [mem size 0x200000000 64bit pref]")?
>
> [ ... ]
>
> @@ -63,8 +66,9 @@ &pcie4 {
>  	       0x98 0x00000000 0x0 0x00002000>; /* configuration space */
>  	reg-names = "regs", "config";
>  
> -	ranges = <0x81000000 0x0 0x00000000 0x98 0x00010000 0x0 0x00010000
> -		  0x82000000 0x0 0x40000000 0x98 0x40000000 0x0 0x40000000>;
> +	ranges = <0x42000000 0x9c 0x00000000 0x9c 0x00000000 0x01 0x00000000>, /* 64-Bit - prefetchable - 16GB */
>
> This isn't a bug, but there might be a typo in the size here. The comment
> says 16GB, but the size cells are 0x01 0x00000000 (4GB). The other PCIe
> controllers in this file use 0x04 0x00000000.

This is a bug, it was intended 16GB.

Queued for v6.

>
> [ ... ]
>
> diff --git a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> --- a/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> +++ b/arch/arm64/boot/dts/freescale/fsl-lx2160a.dtsi
> @@ -1193,7 +1193,12 @@ pcie1: pcie@3400000 {
>  			apio-wins = <8>;
>  			ppio-wins = <8>;
>  			bus-range = <0x0 0xff>;
> -			ranges = <0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
> +			ranges = <0x42000000 0x87 0x00000000 0x87 0x00000000 0x01 0x00000000>, /* 64-Bit - prefetchable - 4GB chunk */
> +				 <0x42000000 0x86 0x00000000 0x86 0x00000000 0x01 0x00000000>, /* 64-Bit - prefetchable - 4GB chunk */
> +				 <0x42000000 0x85 0x00000000 0x85 0x00000000 0x01 0x00000000>, /* 64-Bit - prefetchable - 4GB chunk */
> +				 <0x42000000 0x84 0x00000000 0x84 0x00000000 0x01 0x00000000>, /* 64-Bit - prefetchable - 4GB chunk */
>
> Does fragmenting the 16GB region into multiple 4GB chunks prevent the
> allocation of larger PCIe BARs?
>
> The Linux PCI allocator requires a single contiguous parent window to
> satisfy a BAR allocation request. By splitting the address space here,
> the root bus will have four separate 4GB parent windows.
>
> Will this make it impossible for the kernel to allocate any contiguous
> BAR larger than 4GB, preventing the successful mapping of the 8GB AI
> accelerator BAR mentioned in the commit message?
>