[edk2,v4,1/9] ArmVirtPkg: implement ArmVirtPL031FdtClientLib

Message ID 1460557933-23346-2-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show

Commit Message

Ard Biesheuvel April 13, 2016, 2:32 p.m.
This implements a library ArmVirtPL031FdtClientLib which is intended to
be incorporated into RealTimeClockRuntimeDxe via NULL library class
resolution. This allows us to make RealTimeClockRuntimeDxe depend on the
FDT client protocol, and discover the PL031 base address from the device tree
directly rather than relying on VirtFdtDxe to set the dynamic PCDs.

The NULL library class resolution approach to strictly order production and
consumption of dynamic PCDs is not generally safe in cases such as this one,
where the producer and the consumer of the PCD are both libraries. However,
since the PCD is produced in this library's constructor, and the consumer
library's constructor 'LibRtcInitialize' is not a 'true' constructor (it is
invoked explicitly by RealTimeClockRuntimeDxe), this case is guaranteed to
be safe after all.

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

---
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf | 47 +++++++++++
 ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 82 ++++++++++++++++++++
 2 files changed, 129 insertions(+)

-- 
2.5.0

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

Comments

Laszlo Ersek April 13, 2016, 2:51 p.m. | #1
On 04/13/16 16:32, Ard Biesheuvel wrote:
> This implements a library ArmVirtPL031FdtClientLib which is intended to

> be incorporated into RealTimeClockRuntimeDxe via NULL library class

> resolution. This allows us to make RealTimeClockRuntimeDxe depend on the

> FDT client protocol, and discover the PL031 base address from the device tree

> directly rather than relying on VirtFdtDxe to set the dynamic PCDs.

> 

> The NULL library class resolution approach to strictly order production and

> consumption of dynamic PCDs is not generally safe in cases such as this one,

> where the producer and the consumer of the PCD are both libraries. However,

> since the PCD is produced in this library's constructor, and the consumer

> library's constructor 'LibRtcInitialize' is not a 'true' constructor (it is

> invoked explicitly by RealTimeClockRuntimeDxe), this case is guaranteed to

> be safe after all.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

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

> ---

>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf | 47 +++++++++++

>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 82 ++++++++++++++++++++

>  2 files changed, 129 insertions(+)

> 

> diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf

> new file mode 100644

> index 000000000000..21ee9fc96459

> --- /dev/null

> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf


> +[Pcd]

> +  gArmPlatformTokenSpaceGuid.PcdPL031RtcBase

> +  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot


You usually put PcdPureAcpiBoot in a [FeaturePcd] section separately
(see again commit cd2178bb73b5, and patch 6/9).

Not too important; you can fix it up before pushing the series, or maybe
not at all (as you like). The code itself (that I snipped) correctly
uses FeaturePcdGet().

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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel April 13, 2016, 2:55 p.m. | #2
On 13 April 2016 at 16:51, Laszlo Ersek <lersek@redhat.com> wrote:
> On 04/13/16 16:32, Ard Biesheuvel wrote:

>> This implements a library ArmVirtPL031FdtClientLib which is intended to

>> be incorporated into RealTimeClockRuntimeDxe via NULL library class

>> resolution. This allows us to make RealTimeClockRuntimeDxe depend on the

>> FDT client protocol, and discover the PL031 base address from the device tree

>> directly rather than relying on VirtFdtDxe to set the dynamic PCDs.

>>

>> The NULL library class resolution approach to strictly order production and

>> consumption of dynamic PCDs is not generally safe in cases such as this one,

>> where the producer and the consumer of the PCD are both libraries. However,

>> since the PCD is produced in this library's constructor, and the consumer

>> library's constructor 'LibRtcInitialize' is not a 'true' constructor (it is

>> invoked explicitly by RealTimeClockRuntimeDxe), this case is guaranteed to

>> be safe after all.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

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

>> ---

>>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf | 47 +++++++++++

>>  ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c   | 82 ++++++++++++++++++++

>>  2 files changed, 129 insertions(+)

>>

>> diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf

>> new file mode 100644

>> index 000000000000..21ee9fc96459

>> --- /dev/null

>> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf

>

>> +[Pcd]

>> +  gArmPlatformTokenSpaceGuid.PcdPL031RtcBase

>> +  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot

>

> You usually put PcdPureAcpiBoot in a [FeaturePcd] section separately

> (see again commit cd2178bb73b5, and patch 6/9).

>

> Not too important; you can fix it up before pushing the series, or maybe

> not at all (as you like). The code itself (that I snipped) correctly

> uses FeaturePcdGet().

>


I tend to prefer using FixedPcd/FeaturePcd when possible and
appropriate, and I agree that is the case here

> 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/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
new file mode 100644
index 000000000000..21ee9fc96459
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf
@@ -0,0 +1,47 @@ 
+#/** @file
+#  FDT client library for ARM's PL031 RTC driver
+#
+#  Copyright (c) 2016, Linaro Ltd. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution. The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = ArmVirtPL031FdtClientLib
+  FILE_GUID                      = 13173319-B270-4669-8592-3BB2B31E9E29
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ArmVirtPL031FdtClientLib|DXE_DRIVER DXE_RUNTIME_DRIVER
+  CONSTRUCTOR                    = ArmVirtPL031FdtClientLibConstructor
+
+[Sources]
+  ArmVirtPL031FdtClientLib.c
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid                                ## CONSUMES
+
+[Pcd]
+  gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
+  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
+
+[Depex]
+  gFdtClientProtocolGuid
diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
new file mode 100644
index 000000000000..3c4e44caa2f4
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c
@@ -0,0 +1,82 @@ 
+/** @file
+  FDT client library for ARM's PL031 RTC driver
+
+  Copyright (c) 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
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Uefi.h>
+
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+#include <Protocol/FdtClient.h>
+
+RETURN_STATUS
+EFIAPI
+ArmVirtPL031FdtClientLibConstructor (
+  VOID
+  )
+{
+  EFI_STATUS                    Status;
+  FDT_CLIENT_PROTOCOL           *FdtClient;
+  INT32                         Node;
+  CONST UINT64                  *Reg;
+  UINT32                        RegSize;
+  UINT64                        RegBase;
+
+  Status = gBS->LocateProtocol (&gFdtClientProtocolGuid, NULL,
+                  (VOID **)&FdtClient);
+  ASSERT_EFI_ERROR (Status);
+
+  Status = FdtClient->FindCompatibleNode (FdtClient, "arm,pl031", &Node);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_WARN, "%a: No 'arm,pl031' compatible DT node found\n",
+      __FUNCTION__));
+    return EFI_SUCCESS;
+  }
+
+  Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg",
+                        (CONST VOID **)&Reg, &RegSize);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_WARN,
+      "%a: No 'reg' property found in 'arm,pl031' compatible DT node\n",
+      __FUNCTION__));
+    return EFI_SUCCESS;
+  }
+
+  ASSERT (RegSize == 16);
+
+  RegBase = SwapBytes64 (Reg[0]);
+  ASSERT (RegBase < MAX_UINT32);
+
+  PcdSet32 (PcdPL031RtcBase, (UINT32)RegBase);
+
+  DEBUG ((EFI_D_INFO, "Found PL031 RTC @ 0x%Lx\n", RegBase));
+
+  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
+    //
+    // UEFI takes ownership of the RTC hardware, and exposes its functionality
+    // through the UEFI Runtime Services GetTime, SetTime, etc. This means we
+    // need to disable it in the device tree to prevent the OS from attaching
+    // its device driver as well.
+    //
+    Status = FdtClient->SetNodeProperty (FdtClient, Node, "status",
+                          "disabled", sizeof ("disabled"));
+    if (EFI_ERROR (Status)) {
+        DEBUG ((EFI_D_WARN, "Failed to set PL031 status to 'disabled'\n"));
+    }
+  }
+
+  return EFI_SUCCESS;
+}