On 08/22/16 08:35, Ard Biesheuvel wrote:
> If the pci-host-ecam-generic DT node describes a 64-bit MMIO region,
> account for it in the PCI_ROOT_BRIDGE description that we return to
> the generic PciHostBridgeDxe implementation, which will be able to
> allocate BARs from it without any further changes.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 59
> +++++++++++++-------
> 1 file changed, 38 insertions(+), 21 deletions(-)
(1) So this patch will have to be rebased to the IoTranslation change
discussed under patch 2/5,
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 0aff149e8029..74fda4d23fa3 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -87,9 +87,10 @@ ProcessPciHost (
> OUT UINT64 *IoBase,
> OUT UINT64 *IoSize,
> OUT UINT64 *IoTranslation,
> - OUT UINT64 *MmioBase,
> - OUT UINT64 *MmioSize,
> - OUT UINT64 *MmioTranslation,
> + OUT UINT64 *Mmio32Base,
> + OUT UINT64 *Mmio32Size,
> + OUT UINT64 *Mmio64Base,
> + OUT UINT64 *Mmio64Size,
> OUT UINT32 *BusMin,
> OUT UINT32 *BusMax
> )
> @@ -101,6 +102,7 @@ ProcessPciHost (
> UINT32 Len;
> UINT32 RecordIdx;
> EFI_STATUS Status;
> + UINT64 MmioTranslation;
>
> //
> // The following output arguments are initialized only in
> @@ -109,8 +111,8 @@ ProcessPciHost (
> //
> *IoBase = 0;
> *IoTranslation = 0;
> - *MmioBase = 0;
> - *MmioTranslation = 0;
> + *Mmio32Base = 0;
> + *Mmio64Base = MAX_UINT64;
> *BusMin = 0;
> *BusMax = 0;
>
> @@ -120,7 +122,8 @@ ProcessPciHost (
> // above, they are initialized early.
> //
> *IoSize = 0;
> - *MmioSize = 0;
> + *Mmio32Size = 0;
> + *Mmio64Size = 0;
(2) and here the leading comment should be updated -- at the moment it
refers to IoSize and MmioSize.
>
> Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
> (VOID **)&FdtClient);
> @@ -207,26 +210,39 @@ ProcessPciHost (
> break;
>
> case DTB_PCI_HOST_RANGE_MMIO32:
> - *MmioBase = SwapBytes64 (Record->ChildBase);
> - *MmioSize = SwapBytes64 (Record->Size);
> - *MmioTranslation = SwapBytes64 (Record->CpuBase) - *MmioBase;
> + *Mmio32Base = SwapBytes64 (Record->ChildBase);
> + *Mmio32Size = SwapBytes64 (Record->Size);
> + MmioTranslation = SwapBytes64 (Record->CpuBase) - *Mmio32Base;
>
> - if (*MmioBase > MAX_UINT32 || *MmioSize > MAX_UINT32 ||
> - *MmioBase + *MmioSize > SIZE_4GB) {
> + if (*Mmio32Base > MAX_UINT32 || *Mmio32Size > MAX_UINT32 ||
> + *Mmio32Base + *Mmio32Size > SIZE_4GB) {
> DEBUG ((EFI_D_ERROR, "%a: MMIO32 space invalid\n", __FUNCTION__));
> return EFI_PROTOCOL_ERROR;
> }
>
> - if (*MmioTranslation != 0) {
> + if (MmioTranslation != 0) {
> DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO32 translation "
> - "0x%Lx\n", __FUNCTION__, *MmioTranslation));
> + "0x%Lx\n", __FUNCTION__, MmioTranslation));
> + return EFI_UNSUPPORTED;
> + }
> +
> + break;
> +
> + case DTB_PCI_HOST_RANGE_MMIO64:
> + *Mmio64Base = SwapBytes64 (Record->ChildBase);
> + *Mmio64Size = SwapBytes64 (Record->Size);
> + MmioTranslation = SwapBytes64 (Record->CpuBase) - *Mmio64Base;
> +
> + if (MmioTranslation != 0) {
> + DEBUG ((EFI_D_ERROR, "%a: unsupported nonzero MMIO64 translation "
> + "0x%Lx\n", __FUNCTION__, MmioTranslation));
> return EFI_UNSUPPORTED;
> }
>
> break;
> }
> }
> - if (*IoSize == 0 || *MmioSize == 0) {
> + if (*IoSize == 0 || *Mmio32Size == 0) {
> DEBUG ((EFI_D_ERROR, "%a: %a space empty\n", __FUNCTION__,
> (*IoSize == 0) ? "IO" : "MMIO32"));
> return EFI_PROTOCOL_ERROR;
> @@ -239,9 +255,9 @@ ProcessPciHost (
> ASSERT (PcdGet64 (PcdPciExpressBaseAddress) == ConfigBase);
>
> DEBUG ((EFI_D_INFO, "%a: Config[0x%Lx+0x%Lx) Bus[0x%x..0x%x] "
> - "Io[0x%Lx+0x%Lx)@0x%Lx Mem[0x%Lx+0x%Lx)@0x%Lx\n", __FUNCTION__,
> ConfigBase,
> - ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize, *IoTranslation,
> *MmioBase,
> - *MmioSize, *MmioTranslation));
> + "Io[0x%Lx+0x%Lx)@0x%Lx Mem32[0x%Lx+0x%Lx) Mem64[0x%Lx+0x%Lx)\n",
> + __FUNCTION__, ConfigBase, ConfigSize, *BusMin, *BusMax, *IoBase, *IoSize,
> + *IoTranslation, *Mmio32Base, *Mmio32Size, *Mmio64Base, *Mmio64Size));
> return EFI_SUCCESS;
> }
>
> @@ -263,7 +279,8 @@ PciHostBridgeGetRootBridges (
> {
> PCI_ROOT_BRIDGE *RootBridge;
> UINT64 IoBase, IoSize, IoTranslation;
> - UINT64 Mmio32Base, Mmio32Size, Mmio32Translation;
> + UINT64 Mmio32Base, Mmio32Size;
> + UINT64 Mmio64Base, Mmio64Size;
> UINT32 BusMin, BusMax;
> EFI_STATUS Status;
>
> @@ -275,7 +292,7 @@ PciHostBridgeGetRootBridges (
> }
>
> Status = ProcessPciHost (&IoBase, &IoSize, &IoTranslation, &Mmio32Base,
> - &Mmio32Size, &Mmio32Translation, &BusMin, &BusMax);
> + &Mmio32Size, &Mmio64Base, &Mmio64Size, &BusMin, &BusMax);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_INFO,
> "%a: failed to discover PCI host bridge: %s not present\n",
> @@ -304,8 +321,8 @@ PciHostBridgeGetRootBridges (
> RootBridge->Io.Limit = IoBase + IoSize - 1;
> RootBridge->Mem.Base = Mmio32Base;
> RootBridge->Mem.Limit = Mmio32Base + Mmio32Size - 1;
> - RootBridge->MemAbove4G.Base = MAX_UINT64;
> - RootBridge->MemAbove4G.Limit = 0;
> + RootBridge->MemAbove4G.Base = Mmio64Base;
> + RootBridge->MemAbove4G.Limit = Mmio64Base + Mmio64Size - 1;
>
> //
> // No separate ranges for prefetchable and non-prefetchable BARs
>
With those updates, the patch looks good to me:
Reviewed-by: Laszlo Ersek <[email protected]>
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
