Re: [PATCH v2 5/6] ArmVirtPkg/FdtPciHostBridgeLib: add MMIO64 support

[Laszlo Ersek <lersek@redhat.com>]

On 08/31/16 19:59, Ard Biesheuvel wrote:

> @@ -308,8 +329,21 @@ PciHostBridgeGetRootBridges (
>    mRootBridge.Io.Limit              = IoBase + IoSize - 1;
>    mRootBridge.Mem.Base              = Mmio32Base;
>    mRootBridge.Mem.Limit             = Mmio32Base + Mmio32Size - 1;
> -  mRootBridge.MemAbove4G.Base       = MAX_UINT64;
> -  mRootBridge.MemAbove4G.Limit      = 0;
> +
> +  if (sizeof (UINTN) == sizeof (UINT64)) {
> +    mRootBridge.MemAbove4G.Base       = Mmio64Base;
> +    mRootBridge.MemAbove4G.Limit      = Mmio64Base + Mmio64Size - 1;
> +    mRootBridge.AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
> +  } else {
> +    //
> +    // UEFI mandates a 1:1 virtual-to-physical mapping, so on a 32-bit
> +    // architecture such as ARM, we will not be able to access 64-bit MMIO
> +    // BARs unless they are allocated below 4 GB. So ignore the range above
> +    // 4 GB in this case.
> +    //
> +    mRootBridge.MemAbove4G.Base       = MAX_UINT64;
> +    mRootBridge.MemAbove4G.Limit      = 0;
> +  }
>  
>    //
>    // No separate ranges for prefetchable and non-prefetchable BARs

Please further restrict the one assignment

  mRootBridge.AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;

(while keeping it in its current location) with the following condition:

  if (Mmio64Size > 0) {
    mRootBridge.AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
  }

We should only set this bit if there's actually a 64-bit MMIO aperture.

With that,

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(No need to repost the series just because of this; you can implement the above on commit.)

Thanks!
Laszlo