Re: [edk2-platforms][PATCH 1/1] Silicon/Broadcom/Bcm27xx: Allow more than one device on pcie busses >1

[treffer@measite.de]

On 13.03.21 06:08, Jeremy Linton wrote:
> Hi,
>
>
> First, thanks for the patches, this really helps!
>
> On 3/12/21 12:32 PM, René Treffer wrote:
>> There is only a single pcie port on the bcm2711 so limiting the
>> number of
>> devices to 1 worked as long as there is no way to add a pcie switch.
>>
>> On the compute module 4 it is possible to add a pcie switch (tested with
>> asm1184e) which adds 5 new pcie busses.
>>
>> In the current state the pci enumeration fails for the pcie switch
>> internal bus (bus 2, device 1,3,5,7). The root port gets configured with
>> subordniate=0x2, blocking e.g. a booting linux from discovering devices
>> behind the switch.
>>
>> Devices behind the switch work after lifting the device limit on busses
>> other than 0 and 1.
>>
>> Signed-off-by: René Treffer <treffer@measite.de>
>> ---
>>   .../Library/Bcm2711PciSegmentLib/PciSegmentLib.c   | 14 +++++++-------
>>   1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git
>> a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
>> b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
>> index 44ce3b4b99..4af9374d23 100644
>> ---
>> a/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
>> +++
>> b/Silicon/Broadcom/Bcm27xx/Library/Bcm2711PciSegmentLib/PciSegmentLib.c
>> @@ -78,6 +78,7 @@ PciSegmentLibGetConfigBase (
>>     UINT64        Base;
>>     UINT64        Offset;
>>     UINT32        Dev;
>> +  UINT32        Bus;
>>       Base = PCIE_REG_BASE;
>>     Offset = Address & 0xFFF;         /* Pick off the 4k register
>> offset */
>> @@ -89,17 +90,16 @@ PciSegmentLibGetConfigBase (
>>       Base += PCIE_EXT_CFG_DATA;
>>       if (mPciSegmentLastAccess != Address) {
>>         Dev = EFI_PCI_ADDR_DEV (Address);
>> +      Bus = EFI_PCI_ADDR_BUS (Address);
>>         /*
>> -       * Scan things out directly rather than translating the "bus" to
>> a device, etc..
>> -       * only we need to limit each bus to a single device.
>> +       * There can only be a single device on bus 1 (downstream of
>> root).
>> +       * Subsequent busses (behind a PCIe switch) could have more.
>>          */
>> -      if (Dev < 1) {
>> -          MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address);
>> -          mPciSegmentLastAccess = Address;
>> -      } else {
>> -          mPciSegmentLastAccess = 0;
>> +      if (Dev > 0 && (Bus == 1 || Bus == 0)) {
>
> Did you try this with another switch plugged in downstream the first one?

I don't have a second switch yet.

The topology I tried is:

# lspci -vnnt
-[0000:00]---00.0-[01-06]----00.0-[02-06]--+-01.0-[03]----00.0  Toshiba
Corporation XG4 NVMe SSD Controller [1179:0115]
                                           +-03.0-[04]----00.0  Renesas
Technology Corp. uPD720201 USB 3.0 Host Controller [1912:0014]
                                           +-05.0-[05]--
                                           \-07.0-[06]--

# lspci -nn
00:00.0 PCI bridge [0604]: Broadcom Inc. and subsidiaries BCM2711 PCIe
Bridge [14e4:2711] (rev 20)
01:00.0 PCI bridge [0604]: ASMedia Technology Inc. ASM1184e PCIe Switch
Port [1b21:1184]
02:01.0 PCI bridge [0604]: ASMedia Technology Inc. ASM1184e PCIe Switch
Port [1b21:1184]
02:03.0 PCI bridge [0604]: ASMedia Technology Inc. ASM1184e PCIe Switch
Port [1b21:1184]
02:05.0 PCI bridge [0604]: ASMedia Technology Inc. ASM1184e PCIe Switch
Port [1b21:1184]
02:07.0 PCI bridge [0604]: ASMedia Technology Inc. ASM1184e PCIe Switch
Port [1b21:1184]
03:00.0 Non-Volatile memory controller [0108]: Toshiba Corporation XG4
NVMe SSD Controller [1179:0115] (rev 01)
04:00.0 USB controller [0c03]: Renesas Technology Corp. uPD720201 USB
3.0 Host Controller [1912:0014] (rev 03)

So far I have tried (behind the switch):
- NVMe card. Works, even as a boot device after enabling the NVMe
package for the RPI4
- USB card. Works. upd72020x-load works for uploading firmware (I have
misflashed the ROM though)
- SATA controller ASM1062 + 2xJMB575 based (10 ports) - got detected,
haven't tried more
- LSI HBA, IT mode. Didn't work due to "Resource conflict", breaks NVMe
booting / PCIe bus

The last setup looks like a bug. The LSI HBA resource conflict shouldn't
stop other cards from functioning, right?
I can give that another try and upload the debug logs if anyone is
interested.

The 64MB BAR space default seems to hurt in this setup, but I haven't
been able to get more working.

Devices ordered for future testing:
- An old RADEON HD 3450 256MB (I don't expect that one to work)
- An old Intel I350 dual port network card (hoping to test option ROM
emulation)
- Another PCIe switch (will take 2+ weeks)

I hope to test most of that next week and would probably send another
patch if these things work as intended.

Regards,
  René

> This looks right, because presumably the first new switch is sending
> the UR's properly for its subordinate buses.
>
> Dumping the segment last access reset is fine too.
>
> Although, this set has unicode whitespace characters, which presumably
> Ard fixed up in the previous patch since I see it there too.
>
>
>>             return 0xFFFFFFFF;
>>         }
>> +      MmioWrite32 (PCIE_REG_BASE + PCIE_EXT_CFG_INDEX, Address);
>> +      mPciSegmentLastAccess = Address;
>>       }
>>     }
>>     return Base + Offset;
>>
>
> So,
>
> Reviewed-by: Jeremy Linton <jeremy.linton@arm.com>
>
> Thanks again!