Re: [edk2-platforms: PATCH v2 00/14] Armada7k8k PCIE support

[kettenis@jive.eu]

> Date: Thu, 23 May 2019 15:14:27 +0100
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> On Thu, May 23, 2019 at 03:27:47PM +0200, Mark Kettenis wrote:
> > > From: Marcin Wojtas <mw@semihalf.com>
> > > Date: Mon, 20 May 2019 17:27:13 +0200
> > > 
> > > Hi,
> > > 
> > > Thank you for thorough review of v1. I submit second
> > > version of the Armada7k8k PCIE support. I addressed
> > > all comments. There is no functional change to initial
> > > patchset, but mostly clean-up and improvements - please
> > > refer to the changelog below.
> > >  
> > > The patches are available in the github:
> > > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/pcie-upstream-r20190520
> > > 
> > > I'm looking forward to your comments or remarks.
> > > 
> > > Best regards,
> > > Marcin
> > 
> > Tested this on my mcbin running OpenBSD.  It incovers a small issue in
> > our kernel which I'm fixing.  Otherwise this seems to work fine.
> > 
> > So tested-by: Mark Kettenis <kettenis@openbsd.org> if that matters.
> 
> Always helpful, thanks.
> 
> Out of interest, what was the issue?
> Could I still expect the 6.5 installer to run on this
> hardware/firmware combo?

That should still work fine.

The issue is with mapping PCI ROMS, which currently fails with the new
firmware due to an oversight in the OpenBSD code.  There are only a
few OpenBSD drivers that attempt to map the PCI ROM.  But one of those
is radeondrm(4) and I stuck an AMD graphics card into the PCIe slot on
my machine.

There is a bit of a firmware angle to this though.  The issue happens
because the PCI ROM address register has been set to 0xfffe0000.  All
the writable address bits in the register are set to 1.  While it is
possible that the hardware comes up in that state, I suspect this is
done by an attempt by the firmware to determine the size of the ROM
that doesn't properly restore the original contents of the register.
It may be related to the following messages that are printed by the
firmware:

Image type X64 can't be loaded on AARCH64 UEFI system.
Unloading driver at 0x00000000000
Connect: PcieRoot(0x0)/Pci(0x0,0x0): Not Found

Hmm, that's actually interesting.  Maybe I should play with the X86
emulator that Ard added recently to see if that gives me a framebuffer
console.

Cheers,

Mark

> > Cheers,
> > 
> > Mark
> > 
> > > Changelog:
> > > v1->v2:
> > > *All
> > >   - s/PcieBaseAddress/PcieDbiAdress/
> > > 
> > > *2/14
> > >   - fix alignment in comment
> > > 
> > > * 3/14
> > >   - add CONST** to library callback
> > > 
> > > * 4/14
> > >   - add missing reset GPIO to McBin description
> > > 
> > > * 5/15
> > >   - add CONST** to protocol callback
> > > 
> > > * 6/14
> > >   - cleanup all casting in file
> > >   - use MAX_UINTx macros
> > >   - add Linaro copyright
> > >   - use MmioWrite8 instead of volatile in PciExpressReadBuffer
> > >   - correct commient in IgnoreBusDeviceFunction ()
> > >   - fix typo in commit message
> > > 
> > > * 7/10
> > >   - correct line endings
> > >   - use temporary variable for memory description in  PciHostBridgeResourceConflict
> > >   - use MAX_UINTx macros
> > >   - add comments around stalls and MemoryFence in GPIO reset
> > >   - keep the reset active for 150ms
> > >   - assign translation values instead of asserting
> > > 
> > > *8/14
> > >   - assign gArmTokenSpaceGuid.PcdPciIoTranslation value in .dsc
> > > 
> > > * 9-11/14
> > >   - correct line endings
> > >   - remove unused methods
> > >   - extend commit messages with 32k shift description
> > > 
> > > Ard Biesheuvel (1):
> > >   Marvell/Armada7k8k: Add PciExpressLib implementation
> > > 
> > > Marcin Wojtas (13):
> > >   Marvell/Library: MvGpioLib: Extend GPIO pin description
> > >   Marvell/Library: ArmadaSoCDescLib: Add PCIE information
> > >   Marvell/Library: ArmadaBoardDescLib: Add PCIE information
> > >   Marvell/Armada7k8k: Extend board description libraries with PCIE
> > >   Marvell/Armada7k8k: MvBoardDesc: Extend protocol with PCIE support
> > >   Marvell/Armada7k8k: Implement PciHostBridgeLib
> > >   Marvell/Armada7k8k: Enable PCIE support
> > >   Marvell/Armada80x0McBin: Enable ACPI PCIE support
> > >   Marvell/Armada80x0Db: Enable ACPI PCIE support
> > >   Marvell/Armada70x0Db: Enable ACPI PCIE support
> > >   Marvell/Armada80x0McBin: DeviceTree: Use pci-host-generic driver
> > >   Marvell/Armada7k8k: Remove duplication in .dsc files
> > >   Marvell/Armada7k8: Add 'acpiview' shell command to build
> > > 
> > >  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                                               |   19 +-
> > >  Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc                                              |    4 +-
> > >  Platform/Marvell/Armada80x0Db/Armada80x0Db.dsc                                              |    4 +-
> > >  Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc                                       |    4 +-
> > >  Silicon/Marvell/Armada7k8k/Armada7k8k.fdf                                                   |    5 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db.inf                                      |    1 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db.inf                                      |    1 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin.inf                                   |    1 +
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.inf                |   42 +
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.inf          |   52 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Pcie.h                                   |   26 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Pcie.h                                   |   26 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Pcie.h                                |   26 +
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.h |   95 ++
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h              |    6 +
> > >  Silicon/Marvell/Include/Library/ArmadaBoardDescLib.h                                        |   46 +
> > >  Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h                                          |   20 +
> > >  Silicon/Marvell/Include/Library/MvGpioLib.h                                                 |    1 +
> > >  Silicon/Marvell/Include/Protocol/BoardDesc.h                                                |   22 +
> > >  Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c           |   48 +
> > >  Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c               |    4 +
> > >  Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c           |   48 +
> > >  Platform/Marvell/Armada80x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c               |    6 +
> > >  Platform/SolidRun/Armada80x0McBin/Armada80x0McBinBoardDescLib/Armada80x0McBinBoardDescLib.c |   54 +
> > >  Platform/SolidRun/Armada80x0McBin/NonDiscoverableInitLib/NonDiscoverableInitLib.c           |    1 +
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.c                  | 1531 ++++++++++++++++++++
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.c            |  265 ++++
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c |  345 +++++
> > >  Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c              |   44 +
> > >  Silicon/Marvell/Drivers/BoardDesc/MvBoardDescDxe.c                                          |   86 ++
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl                                 |  108 ++
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Mcfg.aslc                                |   47 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl                                 |  108 ++
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Mcfg.aslc                                |   47 +
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Dsdt.asl                              |  108 ++
> > >  Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Mcfg.aslc                             |   47 +
> > >  Silicon/Marvell/Armada7k8k/DeviceTree/armada-8040-mcbin.dts                                 |    3 +
> > >  37 files changed, 3290 insertions(+), 11 deletions(-)
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.inf
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.inf
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Pcie.h
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Pcie.h
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Pcie.h
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.h
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciExpressLib/PciExpressLib.c
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLib.c
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Mcfg.aslc
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Mcfg.aslc
> > >  create mode 100644 Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0McBin/Mcfg.aslc
> > > 
> > > -- 
> > > 2.7.4
> > > 
>