| Message ID | 1460534539-2169-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On 04/13/16 10:02, 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. > > 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 | 80 ++++++++++++++++++++ > 2 files changed, 127 insertions(+) Heh, did you adopt a git-diff-order file that prioritizes *.inf over *.c? :) If so, thank you! > > diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf > new file mode 100644 > index 000000000000..65ba8f356d1d > --- /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. > +# > +# > +#**/ (1) I think the empty lines could be distributed better. (Same comment as for an earlier patch). > + > +[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] > + ArmPkg/ArmPkg.dec (2) What needs "ArmPkg/ArmPkg.dec"? > + ArmPlatformPkg/ArmPlatformPkg.dec > + ArmVirtPkg/ArmVirtPkg.dec > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + PcdLib > + UefiBootServicesTableLib > + > +[Protocols] > + gFdtClientProtocolGuid ## CONSUMES > + > +[Pcd] > + gArmPlatformTokenSpaceGuid.PcdPL031RtcBase > + > +[Depex] > + gFdtClientProtocolGuid > diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c > new file mode 100644 > index 000000000000..02ab404d5763 > --- /dev/null > +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c > @@ -0,0 +1,80 @@ > +/** @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)); > + > + // > + // 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")); > + } (3) I believe you forgot that this last section should depend on !FeaturePcdGet (PcdPureAcpiBoot) There are three such actions; the chosen node thing, the config table install, and the RTC node disablement. The first is handled well in commit cd2178bb73b5, the second is handled well in patch v3 6/9, and the third should be fixed here, I believe. > + > + return EFI_SUCCESS; > +} > (4) Let me see if the NULL library class resolution, predicted in the commit message of this patch, and implemented in patch v3 2/9, is safe... The direct user of PcdPL031RtcBase is the library instance ArmPlatformPkg/Library/PL031RealTimeClockLib Now, if that library instance had a constructor, and that constructor consumed the PCD, then this solution would not be safe, because it would not enforce any ordering between the constructors. In that case, PL031RealTimeClockLib would have to be made dependent on this library class. However, I'm noticing that the call chain is actually the following: (i) The entry point function of "EmbeddedPkg/RealTimeClockRuntimeDxe", namely InitializeRealTimeClock(), calls LibRtcInitialize() explicitly. (ii) In "ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c", LibRtcInitialize() consumes PcdPL031RtcBase. (iii) Given that our constructor here will be executed before the entry point of RealTimeClockRuntimeDxe is called, this solution is safe after all. I can't easily decide if I should call this "brittle" or not. After all, the fact that LibRtcInitialize() needs to be called explicitly by its consumers is part of that library class. I guess we can rely on that. A problem would arise if there was another (independent) library instance that called LibRtcInitialize() in its constructor. However, that use case is broken in general (BaseTools cannot track such dependencies); it is not specific to this patch. Summary: - please fix (1) - please investigate (2) - please fix (3) - please update the commit message according to (4); i.e., please explain why this solution is safe. I think adding (i) through (iii) -- as a single paragraph -- should suffice. ... Actually, I don't see any other users of ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf in the edk2 tree. So, how about relocating the library instance to ArmVirtPkg, removing PcdPL031RtcBase for good (from "ArmPlatformPkg/ArmPlatformPkg.dec" and everywhere else), and consuming the FDT directly in LibRtcInitialize()? (Xen would not be disturbed by this, because it links RealTimeClockRuntimeDxe against XenRealTimeClockLib.) Or does something in Leif's platform tree use this library? Thanks Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 13 April 2016 at 14:16, Laszlo Ersek <lersek@redhat.com> wrote: > On 04/13/16 10:02, 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. >> >> 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 | 80 ++++++++++++++++++++ >> 2 files changed, 127 insertions(+) > > Heh, did you adopt a git-diff-order file that prioritizes *.inf over > *.c? :) If so, thank you! > Only for ArmVirtPkg and OvmfPkg patches :-) >> >> diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.inf >> new file mode 100644 >> index 000000000000..65ba8f356d1d >> --- /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. >> +# >> +# >> +#**/ > > (1) I think the empty lines could be distributed better. (Same comment > as for an earlier patch). > >> + >> +[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] >> + ArmPkg/ArmPkg.dec > > (2) What needs "ArmPkg/ArmPkg.dec"? > Probably nothing, will remove it I can ... >> + ArmPlatformPkg/ArmPlatformPkg.dec >> + ArmVirtPkg/ArmVirtPkg.dec >> + MdePkg/MdePkg.dec >> + >> +[LibraryClasses] >> + BaseLib >> + DebugLib >> + PcdLib >> + UefiBootServicesTableLib >> + >> +[Protocols] >> + gFdtClientProtocolGuid ## CONSUMES >> + >> +[Pcd] >> + gArmPlatformTokenSpaceGuid.PcdPL031RtcBase >> + >> +[Depex] >> + gFdtClientProtocolGuid > >> diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c >> new file mode 100644 >> index 000000000000..02ab404d5763 >> --- /dev/null >> +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c >> @@ -0,0 +1,80 @@ >> +/** @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)); >> + >> + // >> + // 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")); >> + } > > (3) I believe you forgot that this last section should depend on > > !FeaturePcdGet (PcdPureAcpiBoot) > > There are three such actions; the chosen node thing, the config table > install, and the RTC node disablement. The first is handled well in > commit cd2178bb73b5, the second is handled well in patch v3 6/9, and the > third should be fixed here, I believe. > It does not depend on it, strictly speaking, there is just little point in manipulating the FDT if it is never going to be passed to the OS. >> + >> + return EFI_SUCCESS; >> +} >> > > (4) Let me see if the NULL library class resolution, predicted in the > commit message of this patch, and implemented in patch v3 2/9, is > safe... The direct user of PcdPL031RtcBase is the library instance > > ArmPlatformPkg/Library/PL031RealTimeClockLib > > Now, if that library instance had a constructor, and that constructor > consumed the PCD, then this solution would not be safe, because it would > not enforce any ordering between the constructors. In that case, > PL031RealTimeClockLib would have to be made dependent on this library class. > > However, I'm noticing that the call chain is actually the following: > > (i) The entry point function of "EmbeddedPkg/RealTimeClockRuntimeDxe", > namely InitializeRealTimeClock(), calls LibRtcInitialize() explicitly. > > (ii) In > "ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c", > LibRtcInitialize() consumes PcdPL031RtcBase. > > (iii) Given that our constructor here will be executed before the entry > point of RealTimeClockRuntimeDxe is called, this solution is safe after all. > > I can't easily decide if I should call this "brittle" or not. After all, > the fact that LibRtcInitialize() needs to be called explicitly by its > consumers is part of that library class. I guess we can rely on that. > > A problem would arise if there was another (independent) library > instance that called LibRtcInitialize() in its constructor. However, > that use case is broken in general (BaseTools cannot track such > dependencies); it is not specific to this patch. > > Summary: > - please fix (1) > - please investigate (2) > - please fix (3) > - please update the commit message according to (4); i.e., please > explain why this solution is safe. I think adding (i) through (iii) -- > as a single paragraph -- should suffice. > > ... Actually, I don't see any other users of > > ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf > > in the edk2 tree. > > So, how about relocating the library instance to ArmVirtPkg, removing > PcdPL031RtcBase for good (from "ArmPlatformPkg/ArmPlatformPkg.dec" and > everywhere else), and consuming the FDT directly in LibRtcInitialize()? > > (Xen would not be disturbed by this, because it links > RealTimeClockRuntimeDxe against XenRealTimeClockLib.) > > Or does something in Leif's platform tree use this library? > PL031 is a standard ARM RTC IP block, and QEMU happens to emulate it. So it really does not belong in ArmVirtPkg _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
On 04/13/16 15:18, Ard Biesheuvel wrote: > On 13 April 2016 at 14:16, Laszlo Ersek <lersek@redhat.com> wrote: >> On 04/13/16 10:02, 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. >>> >>> 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 | 80 ++++++++++++++++++++ >>> 2 files changed, 127 insertions(+) >> >> Heh, did you adopt a git-diff-order file that prioritizes *.inf over >> *.c? :) If so, thank you! >> > > Only for ArmVirtPkg and OvmfPkg patches :-) I'm sure your kernel patch reviewers would also appreciate if you put *.h first! ;) >>> + // >>> + // 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")); >>> + } >> >> (3) I believe you forgot that this last section should depend on >> >> !FeaturePcdGet (PcdPureAcpiBoot) >> >> There are three such actions; the chosen node thing, the config table >> install, and the RTC node disablement. The first is handled well in >> commit cd2178bb73b5, the second is handled well in patch v3 6/9, and the >> third should be fixed here, I believe. >> > > It does not depend on it, strictly speaking, there is just little > point in manipulating the FDT if it is never going to be passed to the > OS. I agree about the dependency not being strict, but since this is a conversion / refactoring, let's stick with the original logic (plus let's remain consistent with the other manipulation sites.) (I believe you didn't disagree with my request, just wanted to make it more precise -- and now I wanted to make it even more precise. :)) >> Summary: >> - please fix (1) >> - please investigate (2) >> - please fix (3) >> - please update the commit message according to (4); i.e., please >> explain why this solution is safe. I think adding (i) through (iii) -- >> as a single paragraph -- should suffice. >> >> ... Actually, I don't see any other users of >> >> ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf >> >> in the edk2 tree. >> >> So, how about relocating the library instance to ArmVirtPkg, removing >> PcdPL031RtcBase for good (from "ArmPlatformPkg/ArmPlatformPkg.dec" and >> everywhere else), and consuming the FDT directly in LibRtcInitialize()? >> >> (Xen would not be disturbed by this, because it links >> RealTimeClockRuntimeDxe against XenRealTimeClockLib.) >> >> Or does something in Leif's platform tree use this library? >> > > PL031 is a standard ARM RTC IP block, and QEMU happens to emulate it. > So it really does not belong in ArmVirtPkg Makes perfect sense, thanks. Laszlo _______________________________________________ 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..65ba8f356d1d --- /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] + ArmPkg/ArmPkg.dec + ArmPlatformPkg/ArmPlatformPkg.dec + ArmVirtPkg/ArmVirtPkg.dec + MdePkg/MdePkg.dec + +[LibraryClasses] + BaseLib + DebugLib + PcdLib + UefiBootServicesTableLib + +[Protocols] + gFdtClientProtocolGuid ## CONSUMES + +[Pcd] + gArmPlatformTokenSpaceGuid.PcdPL031RtcBase + +[Depex] + gFdtClientProtocolGuid diff --git a/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c new file mode 100644 index 000000000000..02ab404d5763 --- /dev/null +++ b/ArmVirtPkg/Library/ArmVirtPL031FdtClientLib/ArmVirtPL031FdtClientLib.c @@ -0,0 +1,80 @@ +/** @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)); + + // + // 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. 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 | 80 ++++++++++++++++++++ 2 files changed, 127 insertions(+) -- 2.5.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel