| Message ID | 1460557933-23346-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
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
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
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; +}
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