Hi Ard, When build MinPlatform, it will show Edk2Platforms/Platform/Intel/MinPlatformPkg/Pci/Library/PciSegmentInfoLibSimple/PciSegmentInfoLibSimple.c:34:1: error: conflicting types for ‘GetPciSegmentInfo’
Could you confirm it? Thanks Guomin > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard > Biesheuvel > Sent: Thursday, June 25, 2020 7:25 PM > To: Sami Mujawar <sami.muja...@arm.com>; devel@edk2.groups.io > Cc: l...@nuviainc.com; Ni, Ray <ray...@intel.com>; > alexandru.eli...@arm.com; andre.przyw...@arm.com; > matteo.carl...@arm.com; laura.more...@arm.com; n...@arm.com > Subject: Re: [edk2-devel] [PATCH v3 01/15] PcAtChipsetPkg: Add MMIO > Support to RTC driver > > On 6/24/20 3:34 PM, Sami Mujawar wrote: > > Some virtual machine managers like Kvmtool emulate the MC146818 RTC > > controller in the MMIO space so that architectures that do not support > > I/O Mapped I/O can use the RTC. This patch adds MMIO support to the > > RTC controller driver. > > > > The PCD PcdRtcUseMmio has been added to select I/O or MMIO support. > > If PcdRtcUseMmio is: > > TRUE - Indicates the RTC port registers are in MMIO space. > > FALSE - Indicates the RTC port registers are in I/O space. > > Default is I/O space. > > > > Additionally two new PCDs PcdRtcIndexRegister64 and > > PcdRtcTargetRegister64 have been introduced to provide the base > > address for the RTC registers in the MMIO space. > > > > When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver > > converts the pointers to the RTC MMIO registers so that the RTC > > registers are accessible post ExitBootServices. > > > > Signed-off-by: Sami Mujawar <sami.muja...@arm.com> > > Can't we implement this without function pointers? What Leif suggested was > using STATIC helper functions for read and write, where each one has a test > for the feature PCD to decide whether to use the IO or the MMIO version. > > Using function pointers is unneeded complexity, especially given the fact > that we need to fix up their values when the address space is virtually > remapped by the OS. > > > --- > > > > Notes: > > v3: > > - Make PcdRtcUseMmio a feature PCD. > > [Sami] > > - Read the RTC MMIO base address from the DT. > > [Andre] > > - Introduce PCDs for RTC Index and Target register base > > [Sami] > > address in the MMIO space. > > - Move RTC MMIO region mapping code to a separate platform > [Sami] > > specific library. This library also reads the base addresses > > for the RTC from DT and configures the RTC Index and Target > > register PCDs. > > Ref: https://edk2.groups.io/g/devel/topic/74200905#60307 > > > > v2: > > - Code review comments incorporated. > > [Sami] > > > > v1: > > - Add support to read/write from RTC registers using MMIO access > [Sami] > > - Use wrapper functions for RtcRead/Write accessors > > [Leif] > > Ref: https://edk2.groups.io/g/devel/topic/30915281#30695 > > > > PcAtChipsetPkg/PcAtChipsetPkg.dec > > | 16 +++ > > PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > | 119 > +++++++++++++++++--- > > PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > | 31 > +++++ > > PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c > > | > 72 +++++++++++- > > > PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntime > Dxe.inf | 8 ++ > > 5 files changed, 230 insertions(+), 16 deletions(-) > > > > diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec > > b/PcAtChipsetPkg/PcAtChipsetPkg.dec > > index > > > 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc > 290c > > f3bb905e211b 100644 > > --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec > > +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec > > @@ -6,6 +6,7 @@ > > # > > # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR> > > # Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -41,6 +42,13 @@ [PcdsFeatureFlag] > > # @Prompt Configure HPET to use MSI. > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x00 > 001000 > > > > + ## Indicates the RTC port registers are in MMIO space, or in I/O space. > > + # Default is I/O space.<BR><BR> > > + # TRUE - RTC port registers are in MMIO space.<BR> > > + # FALSE - RTC port registers are in I/O space.<BR> > > + # @Prompt RTC port registers use MMIO. > > + > > + > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x0000 > 0021 > > + > > [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, > PcdsPatchableInModule] > > ## This PCD specifies the base address of the HPET timer. > > # @Prompt HPET base address. > > @@ -68,6 +76,14 @@ [PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx, > PcdsPatchableInModule] > > # @Expression 0x80000001 | > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < > gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100 > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000 > 000 > > 0E > > > > + ## Specifies RTC Index Register address in MMIO space. > > + # @Prompt RTC Index Register address > > + > > + > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00 > 0 > > + 00022 > > + > > + ## Specifies RTC Target Register address in MMIO space. > > + # @Prompt RTC Target Register address > > + > > + > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00 > > + 000023 > > + > > [PcdsFixedAtBuild, PcdsPatchableInModule] > > ## Defines the ACPI register set base address. > > # The invalid 0xFFFF is as its default value. It must be configured to > > the > real value. > > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > index > > > 52af17941786ef81c3911512ee64551724e67209..5ddc06549103ce9944054d3a0 > 5b7 > > 3803f7037ec2 100644 > > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c > > @@ -3,6 +3,7 @@ > > > > Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> > > Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR> > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -10,6 +11,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > #include "PcRtc.h" > > > > +extern UINTN mRtcIndexRegister; > > +extern UINTN mRtcTargetRegister; > > + > > // > > // Days of month. > > // > > @@ -21,6 +25,28 @@ UINTN mDayOfMonth[] = { 31, 29, 31, 30, 31, 30, 31, > 31, 30, 31, 30, 31 }; > > CHAR16 mTimeZoneVariableName[] = L"RTC"; > > > > /** > > + A function pointer that evaluates to a function that reads the RTC > > + content through its registers either using IO or MMIO access. > > + > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + > > + @return The data of UINT8 type read from RTC. > > +**/ > > +RTC_READ RtcRead; > > + > > +/** > > + A function pointer that evaluates to a function that reads the RTC > > +content > > + through its registers either using IO or MMIO access. > > + > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + > > + @return The data of UINT8 type read from RTC. > > +**/ > > +RTC_WRITE RtcWrite; > > + > > +/** > > Compare the Hour, Minute and Second of the From time and the To > time. > > > > Only compare H/M/S in EFI_TIME and ignore other fields here. > > @@ -54,41 +80,96 @@ IsWithinOneDay ( > > ); > > > > /** > > - Read RTC content through its registers. > > + Read RTC content through its registers using IO access. > > > > - @param Address Address offset of RTC. It is recommended to use > macros such as > > - RTC_ADDRESS_SECONDS. > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > > > @return The data of UINT8 type read from RTC. > > **/ > > +STATIC > > UINT8 > > -RtcRead ( > > - IN UINT8 Address > > +EFIAPI > > +IoRtcRead ( > > + IN UINTN Address > > ) > > { > > - IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) > > (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))); > > + IoWrite8 ( > > + PcdGet8 (PcdRtcIndexRegister), > > + (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & > 0x80)) > > + ); > > return IoRead8 (PcdGet8 (PcdRtcTargetRegister)); > > } > > > > /** > > - Write RTC through its registers. > > + Write RTC through its registers using IO access. > > > > - @param Address Address offset of RTC. It is recommended to use > macros such as > > - RTC_ADDRESS_SECONDS. > > - @param Data The content you want to write into RTC. > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + @param Data The content you want to write into RTC. > > > > **/ > > +STATIC > > VOID > > -RtcWrite ( > > - IN UINT8 Address, > > +EFIAPI > > +IoRtcWrite ( > > + IN UINTN Address, > > IN UINT8 Data > > ) > > { > > - IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) > > (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))); > > + IoWrite8 ( > > + PcdGet8 (PcdRtcIndexRegister), > > + (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & > 0x80)) > > + ); > > IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data); > > } > > > > /** > > + Read RTC content through its registers using MMIO access. > > + > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + > > + @return The data of UINT8 type read from RTC. > > +**/ > > +STATIC > > +UINT8 > > +EFIAPI > > +MmioRtcRead ( > > + IN UINTN Address > > + ) > > +{ > > + MmioWrite8 ( > > + mRtcIndexRegister, > > + (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80)) > > + ); > > + return MmioRead8 (mRtcTargetRegister); } > > + > > +/** > > + Write RTC through its registers using MMIO access. > > + > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + @param Data The content you want to write into RTC. > > + > > +**/ > > +STATIC > > +VOID > > +EFIAPI > > +MmioRtcWrite ( > > + IN UINTN Address, > > + IN UINT8 Data > > + ) > > +{ > > + MmioWrite8 ( > > + mRtcIndexRegister, > > + (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80)) > > + ); > > + MmioWrite8 (mRtcTargetRegister, Data); } > > + > > +/** > > Initialize RTC. > > > > @param Global For global use inside this module. > > @@ -113,6 +194,18 @@ PcRtcInit ( > > BOOLEAN Pending; > > > > // > > + // Initialize the RtcRead and RtcWrite functions // based on the > > + chosen IO/MMIO access. > > + // > > + if (FeaturePcdGet (PcdRtcUseMmio)) { > > + RtcRead = MmioRtcRead; > > + RtcWrite = MmioRtcWrite; > > + } else { > > + RtcRead = IoRtcRead; > > + RtcWrite = IoRtcWrite; > > + } > > + > > + // > > // Acquire RTC Lock to make access to RTC atomic > > // > > if (!EfiAtRuntime ()) { > > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > index > > > 47293ce44c5a1f4792892892f7da40d7f0a5a001..4074261c2ca5dbc7b0727ea796 > 11 > > 54eaf41c0e38 100644 > > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.h > > @@ -3,6 +3,7 @@ > > > > Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> > > Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > +Copyright (c) 2019 - 2020, ARM Limited. All rights reserved.<BR> > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > @@ -371,4 +372,34 @@ PcRtcAcpiTableChangeCallback ( > > IN EFI_EVENT Event, > > IN VOID *Context > > ); > > + > > +/** > > + Function pointer to Read RTC content through its registers. > > + > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + > > + @return The data of UINT8 type read from RTC. > > +**/ > > +typedef > > +UINT8 > > +(EFIAPI *RTC_READ) ( > > + IN UINTN Address > > + ); > > + > > +/** > > + Function pointer to Write RTC through its registers. > > + > > + @param Address Address offset of RTC. It is recommended to use > > + macros such as RTC_ADDRESS_SECONDS. > > + @param Data The content you want to write into RTC. > > + > > +**/ > > +typedef > > +VOID > > +(EFIAPI *RTC_WRITE) ( > > + IN UINTN Address, > > + IN UINT8 Data > > + ); > > + > > #endif > > diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c > > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c > > index > > > ccda6331373bfe4069b0a59495b5e5cc731c8fc8..e30883e53333aae655bbf891f8 > 6d > > 1a6ed739715f 100644 > > --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c > > +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c > > @@ -2,16 +2,33 @@ > > Provides Set/Get time operations. > > > > Copyright (c) 2006 - 2018, Intel Corporation. All rights > > reserved.<BR> > > +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR> > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > **/ > > > > +#include <Library/DxeServicesTableLib.h> > > #include "PcRtc.h" > > > > PC_RTC_MODULE_GLOBALS mModuleGlobal; > > > > EFI_HANDLE mHandle = NULL; > > > > +STATIC EFI_EVENT mVirtualAddrChangeEvent; > > + > > +UINTN mRtcIndexRegister; > > +UINTN mRtcTargetRegister; > > + > > +// > > +// Function pointer for the Rtc Read interface function // > > +extern RTC_READ RtcRead; > > + > > +// > > +// Function pointer for the Rtc Write interface function // extern > > +RTC_WRITE RtcWrite; > > + > > /** > > Returns the current time and date information, and the time-keeping > capabilities > > of the hardware platform. > > @@ -106,6 +123,34 @@ PcRtcEfiSetWakeupTime ( > > } > > > > /** > > + Fixup internal data so that EFI can be called in virtual mode. > > + Call the passed in Child Notify event and convert any pointers in > > + lib to virtual mode. > > + > > + @param[in] Event The Event that is being processed > > + @param[in] Context Event Context > > +**/ > > +VOID > > +EFIAPI > > +LibRtcVirtualNotifyEvent ( > > + IN EFI_EVENT Event, > > + IN VOID *Context > > + ) > > +{ > > + // Only needed if you are going to support the OS calling RTC > > +functions in > > + // virtual mode. You will need to call EfiConvertPointer (). To > > +convert any > > + // stored physical addresses to virtual address. After the OS > > +transitions to > > + // calling in virtual mode, all future runtime calls will be made > > +in virtual > > + // mode. > > + EfiConvertPointer (0x0, (VOID**)&mRtcIndexRegister); > > + EfiConvertPointer (0x0, (VOID**)&mRtcTargetRegister); > > + > > + // Convert the RtcRead and RtcWrite pointers for runtime use. > > + EfiConvertPointer (0x0, (VOID**)&RtcRead); > > + EfiConvertPointer (0x0, (VOID**)&RtcWrite); } > > + > > +/** > > The user Entry Point for PcRTC module. > > > > This is the entry point for PcRTC module. It installs the UEFI > > runtime service @@ -125,12 +170,17 @@ InitializePcRtc ( > > IN EFI_SYSTEM_TABLE *SystemTable > > ) > > { > > - EFI_STATUS Status; > > - EFI_EVENT Event; > > + EFI_STATUS Status; > > + EFI_EVENT Event; > > > > EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK); > > mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress (); > > > > + if (FeaturePcdGet (PcdRtcUseMmio)) { > > + mRtcIndexRegister = (UINTN)PcdGet64 (PcdRtcIndexRegister64); > > + mRtcTargetRegister = (UINTN)PcdGet64 (PcdRtcTargetRegister64); } > > + > > Status = PcRtcInit (&mModuleGlobal); > > ASSERT_EFI_ERROR (Status); > > > > @@ -165,7 +215,23 @@ InitializePcRtc ( > > NULL, > > NULL > > ); > > - ASSERT_EFI_ERROR (Status); > > + if (EFI_ERROR (Status)) { > > + ASSERT_EFI_ERROR (Status); > > + return Status; > > + } > > + > > + if (FeaturePcdGet (PcdRtcUseMmio)) { > > + // Register for the virtual address change event > > + Status = gBS->CreateEventEx ( > > + EVT_NOTIFY_SIGNAL, > > + TPL_NOTIFY, > > + LibRtcVirtualNotifyEvent, > > + NULL, > > + &gEfiEventVirtualAddressChangeGuid, > > + &mVirtualAddrChangeEvent > > + ); > > + ASSERT_EFI_ERROR (Status); > > + } > > > > return Status; > > } > > diff --git > > > a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim > eD > > xe.inf > > > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim > eD > > xe.inf index > > > c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe > 5a > > d6247320e79d 100644 > > --- > > > a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntim > eD > > xe.inf > > +++ > b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRunt > > +++ imeDxe.inf > > @@ -6,6 +6,7 @@ > > # > > # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR> > > # Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved.<BR> > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -61,6 +62,11 @@ [Guids] > > ## SOMETIMES_CONSUMES ## SystemTable > > gEfiAcpiTableGuid > > > > + gEfiEventVirtualAddressChangeGuid > > + > > +[FeaturePcd] > > + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio ## > CONSUMES > > + > > [FixedPcd] > > gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA ## > CONSUMES > > gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB ## > CONSUMES > > @@ -72,6 +78,8 @@ [Pcd] > > gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear ## > CONSUMES > > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister ## > CONSUMES > > gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister ## > CONSUMES > > + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64 ## > CONSUMES > > + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64 ## > CONSUMES > > > > [Depex] > > gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61779): https://edk2.groups.io/g/devel/message/61779 Mute This Topic: https://groups.io/mt/75081468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
