Re: [edk2-devel] [PATCH v3 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
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 On Behalf Of Ard > Biesheuvel > Sent: Thursday, June 25, 2020 7:25 PM > To: Sami Mujawar ; devel@edk2.groups.io > Cc: l...@nuviainc.com; Ni, Ray ; > 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 > > 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. > > # Copyright (c) 2017, AMD Inc. All rights reserved. > > +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. > > # > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > # > > @@ -41,6 +42,13 @@ [PcdsFeatureFlag] > > # @Prompt Configure HPET to use MSI. > > > > > gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x00 > 001000 > > > > + #
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 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/PcatRealTimeClockRuntimeDxe.inf | 8 ++ 5 files changed, 230 insertions(+), 16 deletions(-) diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec b/PcAtChipsetPkg/PcAtChipsetPkg.dec index 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc290cf3bb905e211b 100644 --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec @@ -6,6 +6,7 @@ # # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved. # Copyright (c) 2017, AMD Inc. All rights reserved. +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -41,6 +42,13 @@ [PcdsFeatureFlag] # @Prompt Configure HPET to use MSI. gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x1000 + ## Indicates the RTC port registers are in MMIO space, or in I/O space. + # Default is I/O space. + # TRUE - RTC port registers are in MMIO space. + # FALSE - RTC port registers are in I/O space. + # @Prompt RTC port registers use MMIO. + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x0021 + [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 0x8001 | gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100 gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000E + ## Specifies RTC Index Register address in MMIO space. + # @Prompt RTC Index Register address + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x0022 + + ## Specifies RTC Target Register address in MMIO space. + # @Prompt RTC Target Register address + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x0023 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Defines the ACPI register set base address. # The invalid 0x 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..5ddc06549103ce9944054d3a05b73803f7037ec2 100644 ---
[edk2-devel] [PATCH v3 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
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 --- 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/PcatRealTimeClockRuntimeDxe.inf | 8 ++ 5 files changed, 230 insertions(+), 16 deletions(-) diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec b/PcAtChipsetPkg/PcAtChipsetPkg.dec index 88de5cceea593176c3a2425a5963b66b789f2b9e..ed2d95550b8d153995b30cdc290cf3bb905e211b 100644 --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec @@ -6,6 +6,7 @@ # # Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved. # Copyright (c) 2017, AMD Inc. All rights reserved. +# Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. # # SPDX-License-Identifier: BSD-2-Clause-Patent # @@ -41,6 +42,13 @@ [PcdsFeatureFlag] # @Prompt Configure HPET to use MSI. gPcAtChipsetPkgTokenSpaceGuid.PcdHpetMsiEnable|TRUE|BOOLEAN|0x1000 + ## Indicates the RTC port registers are in MMIO space, or in I/O space. + # Default is I/O space. + # TRUE - RTC port registers are in MMIO space. + # FALSE - RTC port registers are in I/O space. + # @Prompt RTC port registers use MMIO. + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x0021 + [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 0x8001 | gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear < gPcAtChipsetPkgTokenSpaceGuid.PcdMinimalValidYear + 100 gPcAtChipsetPkgTokenSpaceGuid.PcdMaximalValidYear|2097|UINT16|0x000E + ## Specifies RTC Index Register address in MMIO space. + # @Prompt RTC Index Register address + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x0022 + + ## Specifies RTC Target Register address in MMIO space. + # @Prompt RTC Target Register address + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x0023 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Defines the ACPI register set base address. # The invalid 0x 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..5ddc06549103ce9944054d3a05b73803f7037ec2 100644 --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c @@ -3,6 +3,7 @@ Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. Copyright (c) 2017, AMD Inc. All rights reserved. +Copyright (c) 2018 - 2020, ARM Limited. All rights reserved. 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
