[edk2,v2,04/24] ArmVirtPkg/ArmGicArchLib: move to FdtClient protocol

Message ID 1460108711-12122-5-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 22486318830291a497e12324ab0e631212907c13
Headers show

Commit Message

Ard Biesheuvel April 8, 2016, 9:44 a.m.
Instead of relying on VirtFdtDxe to populate the GIC related PCDs, move
this handling to our implementation of ArmGicArchLib, and retrieve the
required DT info using the new FDT client protocol.

This removes one of the reasons we need to load VirtFdtDxe first using
an 'A PRIORI' declaration in the platform FDF.

As Laszlo kindly confirms:

  So, ultimately, the only user of this library instance is
  "ArmPkg/Drivers/ArmGic/ArmGicDxe.inf". ... Indeed, checking the build
  report file for ArmVirtQemu (AARCH64), I find ArmVirtGicArchLib (and
  ArmGicLib too) only under "ArmPkg/Drivers/ArmGic/ArmGicDxe.inf".

which means that the constructor is only invoked once, and so the dynamic
PCDs are set in time for ArmGicDxe to consume them, and never afterwards.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c   | 75 ++++++++++++++++++--
 ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf | 18 +++--
 2 files changed, 85 insertions(+), 8 deletions(-)

-- 
2.5.0

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

Comments

Laszlo Ersek April 8, 2016, 12:46 p.m. | #1
On 04/08/16 11:44, Ard Biesheuvel wrote:
> Instead of relying on VirtFdtDxe to populate the GIC related PCDs, move

> this handling to our implementation of ArmGicArchLib, and retrieve the

> required DT info using the new FDT client protocol.

> 

> This removes one of the reasons we need to load VirtFdtDxe first using

> an 'A PRIORI' declaration in the platform FDF.

> 

> As Laszlo kindly confirms:

> 

>   So, ultimately, the only user of this library instance is

>   "ArmPkg/Drivers/ArmGic/ArmGicDxe.inf". ... Indeed, checking the build

>   report file for ArmVirtQemu (AARCH64), I find ArmVirtGicArchLib (and

>   ArmGicLib too) only under "ArmPkg/Drivers/ArmGic/ArmGicDxe.inf".

> 

> which means that the constructor is only invoked once, and so the dynamic

> PCDs are set in time for ArmGicDxe to consume them, and never afterwards.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c   | 75 ++++++++++++++++++--

>  ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf | 18 +++--

>  2 files changed, 85 insertions(+), 8 deletions(-)


Nice!

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


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

Patch

diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
index 732860cadfe6..7ef829ff2d26 100644
--- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
+++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.c
@@ -1,7 +1,7 @@ 
 /** @file
   ArmGicArchLib library class implementation for DT based virt platforms
 
-  Copyright (c) 2015, Linaro Ltd. All rights reserved.<BR>
+  Copyright (c) 2015 - 2016, Linaro Ltd. All rights reserved.<BR>
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD License
@@ -14,11 +14,16 @@ 
 **/
 
 #include <Base.h>
+#include <Uefi.h>
 
 #include <Library/ArmGicLib.h>
 #include <Library/ArmGicArchLib.h>
-#include <Library/PcdLib.h>
+#include <Library/BaseLib.h>
 #include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+#include <Protocol/FdtClient.h>
 
 STATIC ARM_GIC_ARCH_REVISION        mGicArchRevision;
 
@@ -28,12 +33,62 @@  ArmVirtGicArchLibConstructor (
   VOID
   )
 {
-  UINT32  IccSre;
+  UINT32                IccSre;
+  FDT_CLIENT_PROTOCOL   *FdtClient;
+  CONST UINT64          *Reg;
+  UINT32                RegElemSize, RegSize;
+  UINTN                 GicRevision;
+  EFI_STATUS            Status;
+  UINT64                DistBase, CpuBase, RedistBase;
 
-  switch (PcdGet32 (PcdArmGicRevision)) {
+  Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
+                  (VOID **)&FdtClient);
+  ASSERT_EFI_ERROR (Status);
+
+  GicRevision = 2;
+  Status = FdtClient->FindCompatibleNodeReg (FdtClient, "arm,cortex-a15-gic",
+                        (CONST VOID **)&Reg, &RegElemSize, &RegSize);
+  if (Status == EFI_NOT_FOUND) {
+    GicRevision = 3;
+    Status = FdtClient->FindCompatibleNodeReg (FdtClient, "arm,gic-v3",
+                          (CONST VOID **)&Reg, &RegElemSize, &RegSize);
+  }
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  switch (GicRevision) {
 
   case 3:
     //
+    // The GIC v3 DT binding describes a series of at least 3 physical (base
+    // addresses, size) pairs: the distributor interface (GICD), at least one
+    // redistributor region (GICR) containing dedicated redistributor
+    // interfaces for all individual CPUs, and the CPU interface (GICC).
+    // Under virtualization, we assume that the first redistributor region
+    // listed covers the boot CPU. Also, our GICv3 driver only supports the
+    // system register CPU interface, so we can safely ignore the MMIO version
+    // which is listed after the sequence of redistributor interfaces.
+    // This means we are only interested in the first two memory regions
+    // supplied, and ignore everything else.
+    //
+    ASSERT (RegSize >= 32);
+
+    // RegProp[0..1] == { GICD base, GICD size }
+    DistBase = SwapBytes64 (Reg[0]);
+    ASSERT (DistBase < MAX_UINT32);
+
+    // RegProp[2..3] == { GICR base, GICR size }
+    RedistBase = SwapBytes64 (Reg[2]);
+    ASSERT (RedistBase < MAX_UINT32);
+
+    PcdSet32 (PcdGicDistributorBase, (UINT32)DistBase);
+    PcdSet32 (PcdGicRedistributorsBase, (UINT32)RedistBase);
+
+    DEBUG ((EFI_D_INFO, "Found GIC v3 (re)distributor @ 0x%Lx (0x%Lx)\n",
+      DistBase, RedistBase));
+
+    //
     // The default implementation of ArmGicArchLib is responsible for enabling
     // the system register interface on the GICv3 if one is found. So let's do
     // the same here.
@@ -55,6 +110,18 @@  ArmVirtGicArchLibConstructor (
     break;
 
   case 2:
+    ASSERT (RegSize == 32);
+
+    DistBase = SwapBytes64 (Reg[0]);
+    CpuBase  = SwapBytes64 (Reg[2]);
+    ASSERT (DistBase < MAX_UINT32);
+    ASSERT (CpuBase < MAX_UINT32);
+
+    PcdSet32 (PcdGicDistributorBase, (UINT32)DistBase);
+    PcdSet32 (PcdGicInterruptInterfaceBase, (UINT32)CpuBase);
+
+    DEBUG ((EFI_D_INFO, "Found GIC @ 0x%Lx/0x%Lx\n", DistBase, CpuBase));
+
     mGicArchRevision = ARM_GIC_ARCH_REVISION_2;
     break;
 
diff --git a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf
index c85b2d44d856..a25c191bc97d 100644
--- a/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf
+++ b/ArmVirtPkg/Library/ArmVirtGicArchLib/ArmVirtGicArchLib.inf
@@ -27,14 +27,24 @@  [Sources]
   ArmVirtGicArchLib.c
 
 [LibraryClasses]
-  PcdLib
-  DebugLib
   ArmGicLib
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
 
 [Packages]
-  MdePkg/MdePkg.dec
   ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
+  MdePkg/MdePkg.dec
+
+[Protocols]
+  gFdtClientProtocolGuid                                ## CONSUMES
 
 [Pcd]
-  gArmVirtTokenSpaceGuid.PcdArmGicRevision
+  gArmTokenSpaceGuid.PcdGicDistributorBase
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase
+  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
+
+[Depex]
+  gFdtClientProtocolGuid