Hi Ard, Please find my response inline marked [SAMI].
Regards, Sami Mujawar -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel via groups.io Sent: 07 July 2020 02:20 PM To: Sami Mujawar <sami.muja...@arm.com>; devel@edk2.groups.io Cc: l...@nuviainc.com; ler...@redhat.com; Alexandru Elisei <alexandru.eli...@arm.com>; Andre Przywara <andre.przyw...@arm.com>; Matteo Carlini <matteo.carl...@arm.com>; Laura Moretta <laura.more...@arm.com>; nd <n...@arm.com> Subject: Re: [edk2-devel] [PATCH v4 02/15] ArmVirtPkg: Add Kvmtool RTC Fdt Client Library On 7/7/20 3:47 PM, Sami Mujawar wrote: > Add library that parses the Kvmtool device tree and updates the > dynamic PCDs describing the RTC Memory map. > > It also maps the MMIO region used by the RTC as runtime memory so that > the RTC registers are accessible post ExitBootServices. > > Since UEFI takes ownership of the RTC hardware disable the RTC node in > the DT to prevent the OS from attaching its device driver as well. > > Signed-off-by: Sami Mujawar <sami.muja...@arm.com> > --- > > Notes: > v4: > - Updated based on review comments. [Sami] > - Cleanup include file list, make local functions static, [Ard] > drop EFIAPI, use EFI_PAGE_MASK and use ReadUnaligned64. > Ref: https://edk2.groups.io/g/devel/topic/75081472 > You haven't answered my question about adding and removing the GCD memory space. [SAMI] The call to gDS->AddMemorySpace () was failing on my experimental branch to enable PCI support for the kvmtool firmware. With the current kvmtool implementation, when PCI is enabled, some part of the PCI IO region is overlapping with the region used by the RTC controller. Since this patch series is not adding PCI support for kvmtool firmware, this issue cannot be seen. [/SAMI] > v3: > - Introduce library to read and map the MMIO base addresses [Sami] > for the RTC registers from the DT and configure the Index > and Target register PCDs. > > ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c | 230 > ++++++++++++++++++++ > ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf | 42 > ++++ > 2 files changed, 272 insertions(+) > > diff --git > a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c > b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..93715958952f933007ff143a22cbe81606cc539e > --- /dev/null > +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.c > @@ -0,0 +1,230 @@ > +/** @file > + FDT client library for motorola,mc146818 RTC driver > + > + Copyright (c) 2020, ARM Limited. All rights reserved.<BR> > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/DxeServicesTableLib.h> > +#include <Library/PcdLib.h> > +#include <Library/UefiBootServicesTableLib.h> > +#include <Protocol/FdtClient.h> > + > +/** RTC Index register is at offset 0x0 > +*/ > +#define RTC_INDEX_REG_OFFSET 0x0ULL > + > +/** RTC Target register is at offset 0x1 > +*/ > +#define RTC_TARGET_REG_OFFSET 0x1ULL > + > +/** Add the RTC controller address range to the memory map. > + > + @param [in] ImageHandle The handle to the image. > + @param [in] RtcPageBase Base address of the RTC controller. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER A parameter is invalid. > + @retval EFI_NOT_FOUND Flash device not found. > +**/ > +STATIC > +EFI_STATUS > +KvmtoolRtcMapMemory ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_PHYSICAL_ADDRESS RtcPageBase > + ) > +{ > + EFI_STATUS Status; > + > + Status = gDS->AddMemorySpace ( > + EfiGcdMemoryTypeMemoryMappedIo, > + RtcPageBase, > + EFI_PAGE_SIZE, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, "Failed to add memory space. Status = %r\n", > + Status > + )); > + return Status; > + } > + > + Status = gDS->AllocateMemorySpace ( > + EfiGcdAllocateAddress, > + EfiGcdMemoryTypeMemoryMappedIo, > + 0, > + EFI_PAGE_SIZE, > + &RtcPageBase, > + ImageHandle, > + NULL > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "Failed to allocate memory space. Status = %r\n", > + Status > + )); > + gDS->RemoveMemorySpace ( > + RtcPageBase, > + EFI_PAGE_SIZE > + ); > + return Status; > + } > + > + Status = gDS->SetMemorySpaceAttributes ( > + RtcPageBase, > + EFI_PAGE_SIZE, > + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "Failed to set memory attributes. Status = %r\n", > + Status > + )); > + gDS->FreeMemorySpace ( > + RtcPageBase, > + EFI_PAGE_SIZE > + ); > + gDS->RemoveMemorySpace ( > + RtcPageBase, > + EFI_PAGE_SIZE > + ); > + } > + > + return Status; > +} > + > +/** Entrypoint for KvmtoolRtcFdtClientLib. > + > + Locate the RTC node in the DT and update the Index and > + Target register base addresses in the respective PCDs. > + Add the RTC memory region to the memory map. > + Disable the RTC node as the RTC is owned by UEFI. > + > + @param [in] ImageHandle The handle to the image. > + @param [in] SystemTable Pointer to the System Table. > + > + @retval EFI_SUCCESS Success. > + @retval EFI_INVALID_PARAMETER A parameter is invalid. > + @retval EFI_NOT_FOUND Flash device not found. > +**/ > +EFI_STATUS > +EFIAPI > +KvmtoolRtcFdtClientLibConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + FDT_CLIENT_PROTOCOL *FdtClient; > + INT32 Node; > + CONST UINT32 *Reg; > + UINT32 RegSize; > + UINT64 RegBase; > + UINT64 Range; > + RETURN_STATUS PcdStatus; > + > + Status = gBS->LocateProtocol ( > + &gFdtClientProtocolGuid, > + NULL, > + (VOID **)&FdtClient > + ); > + ASSERT_EFI_ERROR (Status); > + > + Status = FdtClient->FindCompatibleNode ( > + FdtClient, > + "motorola,mc146818", > + &Node > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: No 'motorola,mc146818' compatible DT node found\n", > + __FUNCTION__ > + )); > + return Status; > + } > + > + Status = FdtClient->GetNodeProperty ( > + FdtClient, > + Node, > + "reg", > + (CONST VOID **)&Reg, > + &RegSize > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: No 'reg' property found in 'motorola,mc146818' compatible DT > node\n", > + __FUNCTION__ > + )); > + return Status; > + } > + > + ASSERT (RegSize == 16); > + > + RegBase = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[0])); > + Range = SwapBytes64 (ReadUnaligned64 ((VOID *)&Reg[2])); > + DEBUG (( > + DEBUG_INFO, > + "Found motorola,mc146818 RTC @ 0x%Lx Range = 0x%x\n", > + RegBase, > + Range > + )); > + > + // The address range must cover the RTC Index and the Target registers. > + ASSERT (Range >= 0x2); > + > + // RTC Index register is at offset 0x0 > + PcdStatus = PcdSet64S ( > + PcdRtcIndexRegister64, > + (RegBase + RTC_INDEX_REG_OFFSET) > + ); > + ASSERT_RETURN_ERROR (PcdStatus); > + > + // RTC Target register is at offset 0x1 > + PcdStatus = PcdSet64S ( > + PcdRtcTargetRegister64, > + (RegBase + RTC_TARGET_REG_OFFSET) > + ); > + ASSERT_RETURN_ERROR (PcdStatus); > + > + Status = KvmtoolRtcMapMemory (ImageHandle, (RegBase & ~EFI_PAGE_MASK)); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "Failed to map memory for motorola,mc146818. Status = %r\n", > + Status > + )); > + return Status; > + } > + > + // > + // 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 (( > + DEBUG_WARN, > + "Failed to set motorola,mc146818 status to 'disabled', Status = %r\n", > + Status > + )); > + } > + > + return EFI_SUCCESS; > +} > diff --git > a/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf > b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf > new file mode 100644 > index > 0000000000000000000000000000000000000000..0d1b27997ed00d7c1be3578cab4d1d2eba663a90 > --- /dev/null > +++ b/ArmVirtPkg/Library/KvmtoolRtcFdtClientLib/KvmtoolRtcFdtClientLib.inf > @@ -0,0 +1,42 @@ > +#/** @file > +# FDT client library for motorola,mc146818 RTC driver > +# > +# Copyright (c) 2020, ARM Limited. All rights reserved.<BR> > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +#**/ > + > +[Defines] > + INF_VERSION = 0x0001001B > + BASE_NAME = KvmtoolRtcFdtClientLib > + FILE_GUID = 3254B4F7-30B5-48C6-B06A-D8FF97F3EF95 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = KvmtoolRtcFdtClientLib|DXE_DRIVER > DXE_RUNTIME_DRIVER > + CONSTRUCTOR = KvmtoolRtcFdtClientLibConstructor > + > +[Sources] > + KvmtoolRtcFdtClientLib.c > + > +[Packages] > + ArmVirtPkg/ArmVirtPkg.dec > + MdePkg/MdePkg.dec > + PcAtChipsetPkg/PcAtChipsetPkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + PcdLib > + UefiBootServicesTableLib > + DxeServicesTableLib > + > +[Protocols] > + gFdtClientProtocolGuid ## CONSUMES > + > +[Pcd] > + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64 > + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64 > + > +[Depex] > + gFdtClientProtocolGuid > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62301): https://edk2.groups.io/g/devel/message/62301 Mute This Topic: https://groups.io/mt/75354087/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
