diff mbox

[edk2,4/5] ArmVirtPkg/FdtPciHostBridgeLib: add MMIO64 support

Message ID 1471847752-26574-5-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel Aug. 22, 2016, 6:35 a.m. UTC 
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 <ard.biesheuvel@linaro.org>

---
 ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c | 59 +++++++++++++-------
 1 file changed, 38 insertions(+), 21 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Aug. 31, 2016, 2:06 p.m. UTC | #1 
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 <ard.biesheuvel@linaro.org>

> ---

>  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 <lersek@redhat.com>


Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

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;
 
   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