[edk2-devel] [PATCH v5 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver

Sami Mujawar posted 15 patches 5 years, 4 months ago
[edk2-devel] [PATCH v5 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
Posted by Sami Mujawar 5 years, 4 months ago
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.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
---

Notes:
    v5:
      - Updated based on review comments.                             [Sami]
      - Drop the unnecessary whitespace and header changes to         [Ard]
        PcRtc.h, and the whitespace changes to PcRtcEntry.c
        Ref: https://edk2.groups.io/g/devel/topic/75354083
    
    v4:
      - Updated based on review comments.                             [Sami]
      - Use static helper functions instead of function pointers.     [Ard]
        Ref: https://edk2.groups.io/g/devel/topic/75081468
    
    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            [Sami]
        MMIO access
      - 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                         | 120 ++++++++++++++++++--
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                    |  54 ++++++++-
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf |   8 ++
 4 files changed, 186 insertions(+), 12 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.<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|0x00001000
 
+  ## 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|0x00000021
+
 [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|0x0000000E
 
+  ## Specifies RTC Index Register address in MMIO space.
+  # @Prompt RTC Index Register address
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00000022
+
+  ## Specifies RTC Target Register address in MMIO space.
+  # @Prompt RTC Target Register address
+  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00000023
+
 [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..64f36f6fbbd1b03967bd1a1290d108d5b0f294fa 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.
 //
@@ -54,38 +58,132 @@ IsWithinOneDay (
   );
 
 /**
+  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.
+
+  @return The data of UINT8 type read from RTC.
+**/
+STATIC
+UINT8
+IoRtcRead (
+  IN  UINTN Address
+  )
+{
+  IoWrite8 (
+    PcdGet8 (PcdRtcIndexRegister),
+    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
+    );
+  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
+}
+
+/**
+  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.
+
+**/
+STATIC
+VOID
+IoRtcWrite (
+  IN  UINTN   Address,
+  IN  UINT8   Data
+  )
+{
+  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
+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
+MmioRtcWrite (
+  IN  UINTN   Address,
+  IN  UINT8   Data
+  )
+{
+  MmioWrite8 (
+    mRtcIndexRegister,
+    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
+    );
+  MmioWrite8 (mRtcTargetRegister, Data);
+}
+
+/**
   Read RTC content through its registers.
 
-  @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
+  IN  UINTN Address
   )
 {
-  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
-  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
+  if (FeaturePcdGet (PcdRtcUseMmio)) {
+    return MmioRtcRead (Address);
+  }
+
+  return IoRtcRead (Address);
 }
 
 /**
   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.
+  @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,
+  IN  UINTN   Address,
   IN  UINT8   Data
   )
 {
-  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
-  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
+  if (FeaturePcdGet (PcdRtcUseMmio)) {
+    MmioRtcWrite (Address, Data);
+  } else {
+    IoRtcWrite (Address, Data);
+  }
 }
 
 /**
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
index ccda6331373bfe4069b0a59495b5e5cc731c8fc8..606b88adafb7ef5d81e32e212794a5ccae9d0443 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
@@ -2,16 +2,23 @@
   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;
+
 /**
   Returns the current time and date information, and the time-keeping capabilities
   of the hardware platform.
@@ -106,6 +113,30 @@ 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);
+}
+
+/**
   The user Entry Point for PcRTC module.
 
   This is the entry point for PcRTC module. It installs the UEFI runtime service
@@ -131,6 +162,11 @@ InitializePcRtc (
   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 +201,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/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
index c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe5ad6247320e79d 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.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
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#65855): https://edk2.groups.io/g/devel/message/65855
Mute This Topic: https://groups.io/mt/77270941/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v5 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
Posted by Ard Biesheuvel 5 years, 4 months ago
On 10/2/20 11:13 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.mujawar@arm.com>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Thanks for the resend.

Ray, do you have any concerns regarding this patch? The series looks 
otherwise ok, and I would like to get this merged this week.

Guomin, you replied before that you were intending to review this patch 
after August. Do you still intend to do so?

If I don't hear from you by the end of this week, I am going to assume 
there are no objections from your side.

Thanks,
Ard.


> ---
> 
> Notes:
>      v5:
>        - Updated based on review comments.                             [Sami]
>        - Drop the unnecessary whitespace and header changes to         [Ard]
>          PcRtc.h, and the whitespace changes to PcRtcEntry.c
>          Ref: https://edk2.groups.io/g/devel/topic/75354083
>      
>      v4:
>        - Updated based on review comments.                             [Sami]
>        - Use static helper functions instead of function pointers.     [Ard]
>          Ref: https://edk2.groups.io/g/devel/topic/75081468
>      
>      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            [Sami]
>          MMIO access
>        - 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                         | 120 ++++++++++++++++++--
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                    |  54 ++++++++-
>   PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf |   8 ++
>   4 files changed, 186 insertions(+), 12 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.<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|0x00001000
>   
> +  ## 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|0x00000021
> +
>   [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|0x0000000E
>   
> +  ## Specifies RTC Index Register address in MMIO space.
> +  # @Prompt RTC Index Register address
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00000022
> +
> +  ## Specifies RTC Target Register address in MMIO space.
> +  # @Prompt RTC Target Register address
> +  gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00000023
> +
>   [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..64f36f6fbbd1b03967bd1a1290d108d5b0f294fa 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.
>   //
> @@ -54,38 +58,132 @@ IsWithinOneDay (
>     );
>   
>   /**
> +  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.
> +
> +  @return The data of UINT8 type read from RTC.
> +**/
> +STATIC
> +UINT8
> +IoRtcRead (
> +  IN  UINTN Address
> +  )
> +{
> +  IoWrite8 (
> +    PcdGet8 (PcdRtcIndexRegister),
> +    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> +    );
> +  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
> +}
> +
> +/**
> +  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.
> +
> +**/
> +STATIC
> +VOID
> +IoRtcWrite (
> +  IN  UINTN   Address,
> +  IN  UINT8   Data
> +  )
> +{
> +  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
> +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
> +MmioRtcWrite (
> +  IN  UINTN   Address,
> +  IN  UINT8   Data
> +  )
> +{
> +  MmioWrite8 (
> +    mRtcIndexRegister,
> +    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
> +    );
> +  MmioWrite8 (mRtcTargetRegister, Data);
> +}
> +
> +/**
>     Read RTC content through its registers.
>   
> -  @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
> +  IN  UINTN Address
>     )
>   {
> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> -  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    return MmioRtcRead (Address);
> +  }
> +
> +  return IoRtcRead (Address);
>   }
>   
>   /**
>     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.
> +  @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,
> +  IN  UINTN   Address,
>     IN  UINT8   Data
>     )
>   {
> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> -  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
> +    MmioRtcWrite (Address, Data);
> +  } else {
> +    IoRtcWrite (Address, Data);
> +  }
>   }
>   
>   /**
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index ccda6331373bfe4069b0a59495b5e5cc731c8fc8..606b88adafb7ef5d81e32e212794a5ccae9d0443 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -2,16 +2,23 @@
>     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;
> +
>   /**
>     Returns the current time and date information, and the time-keeping capabilities
>     of the hardware platform.
> @@ -106,6 +113,30 @@ 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);
> +}
> +
> +/**
>     The user Entry Point for PcRTC module.
>   
>     This is the entry point for PcRTC module. It installs the UEFI runtime service
> @@ -131,6 +162,11 @@ InitializePcRtc (
>     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 +201,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/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> index c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe5ad6247320e79d 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.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 (#65885): https://edk2.groups.io/g/devel/message/65885
Mute This Topic: https://groups.io/mt/77270941/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v5 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
Posted by Laszlo Ersek 5 years, 4 months ago
Hi Ard,

On 10/05/20 12:58, Ard Biesheuvel wrote:
> On 10/2/20 11:13 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.mujawar@arm.com>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Thanks for the resend.
> 
> Ray, do you have any concerns regarding this patch? The series looks
> otherwise ok, and I would like to get this merged this week.

Thank you very much for handling this. I've skimmed the series quickly
and it mostly adds new files. I think the risk of regressions is really
low. Please feel free to merge this from my side.

For wherever it applies in the series:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo



> 
> Guomin, you replied before that you were intending to review this patch
> after August. Do you still intend to do so?
> 
> If I don't hear from you by the end of this week, I am going to assume
> there are no objections from your side.
> 
> Thanks,
> Ard.
> 
> 
>> ---
>>
>> Notes:
>>      v5:
>>        - Updated based on review comments.                            
>> [Sami]
>>        - Drop the unnecessary whitespace and header changes to        
>> [Ard]
>>          PcRtc.h, and the whitespace changes to PcRtcEntry.c
>>          Ref: https://edk2.groups.io/g/devel/topic/75354083
>>           v4:
>>        - Updated based on review comments.                            
>> [Sami]
>>        - Use static helper functions instead of function pointers.    
>> [Ard]
>>          Ref: https://edk2.groups.io/g/devel/topic/75081468
>>           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           
>> [Sami]
>>          MMIO access
>>        - 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                        
>> | 120 ++++++++++++++++++--
>>  
>> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c                   
>> |  54 ++++++++-
>>  
>> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
>> |   8 ++
>>   4 files changed, 186 insertions(+), 12 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.<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|0x00001000
>>   +  ## 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|0x00000021
>> +
>>   [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|0x0000000E
>>   +  ## Specifies RTC Index Register address in MMIO space.
>> +  # @Prompt RTC Index Register address
>> + 
>> gPcAtChipsetPkgTokenSpaceGuid.PcdRtcIndexRegister64|0x0|UINT64|0x00000022
>> +
>> +  ## Specifies RTC Target Register address in MMIO space.
>> +  # @Prompt RTC Target Register address
>> + 
>> gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister64|0x0|UINT64|0x00000023
>>
>> +
>>   [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..64f36f6fbbd1b03967bd1a1290d108d5b0f294fa
>> 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.
>>   //
>> @@ -54,38 +58,132 @@ IsWithinOneDay (
>>     );
>>     /**
>> +  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.
>> +
>> +  @return The data of UINT8 type read from RTC.
>> +**/
>> +STATIC
>> +UINT8
>> +IoRtcRead (
>> +  IN  UINTN Address
>> +  )
>> +{
>> +  IoWrite8 (
>> +    PcdGet8 (PcdRtcIndexRegister),
>> +    (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister))
>> & 0x80))
>> +    );
>> +  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
>> +}
>> +
>> +/**
>> +  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.
>> +
>> +**/
>> +STATIC
>> +VOID
>> +IoRtcWrite (
>> +  IN  UINTN   Address,
>> +  IN  UINT8   Data
>> +  )
>> +{
>> +  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
>> +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
>> +MmioRtcWrite (
>> +  IN  UINTN   Address,
>> +  IN  UINT8   Data
>> +  )
>> +{
>> +  MmioWrite8 (
>> +    mRtcIndexRegister,
>> +    (UINT8)(Address | (UINT8)(MmioRead8 (mRtcIndexRegister) & 0x80))
>> +    );
>> +  MmioWrite8 (mRtcTargetRegister, Data);
>> +}
>> +
>> +/**
>>     Read RTC content through its registers.
>>   -  @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
>> +  IN  UINTN Address
>>     )
>>   {
>> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8)
>> (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
>> -  return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
>> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
>> +    return MmioRtcRead (Address);
>> +  }
>> +
>> +  return IoRtcRead (Address);
>>   }
>>     /**
>>     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.
>> +  @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,
>> +  IN  UINTN   Address,
>>     IN  UINT8   Data
>>     )
>>   {
>> -  IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8)
>> (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
>> -  IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
>> +  if (FeaturePcdGet (PcdRtcUseMmio)) {
>> +    MmioRtcWrite (Address, Data);
>> +  } else {
>> +    IoRtcWrite (Address, Data);
>> +  }
>>   }
>>     /**
>> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> index
>> ccda6331373bfe4069b0a59495b5e5cc731c8fc8..606b88adafb7ef5d81e32e212794a5ccae9d0443
>> 100644
>> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
>> @@ -2,16 +2,23 @@
>>     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;
>> +
>>   /**
>>     Returns the current time and date information, and the
>> time-keeping capabilities
>>     of the hardware platform.
>> @@ -106,6 +113,30 @@ 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);
>> +}
>> +
>> +/**
>>     The user Entry Point for PcRTC module.
>>       This is the entry point for PcRTC module. It installs the UEFI
>> runtime service
>> @@ -131,6 +162,11 @@ InitializePcRtc (
>>     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 +201,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/PcatRealTimeClockRuntimeDxe.inf
>> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
>>
>> index
>> c73ee98105e510f9e4e23c1a6c1e5c505325d2c9..0d8eca28b65954b073a72fc4fe5ad6247320e79d
>> 100644
>> ---
>> a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
>>
>> +++
>> b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.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 (#65917): https://edk2.groups.io/g/devel/message/65917
Mute This Topic: https://groups.io/mt/77270941/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v5 01/15] PcAtChipsetPkg: Add MMIO Support to RTC driver
Posted by Ard Biesheuvel 5 years, 3 months ago
On 10/6/20 12:16 PM, Laszlo Ersek wrote:
> Hi Ard,
> 
> On 10/05/20 12:58, Ard Biesheuvel wrote:
>> On 10/2/20 11:13 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.mujawar@arm.com>
>>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>>
>> Thanks for the resend.
>>
>> Ray, do you have any concerns regarding this patch? The series looks
>> otherwise ok, and I would like to get this merged this week.
> 
> Thank you very much for handling this. I've skimmed the series quickly
> and it mostly adds new files. I think the risk of regressions is really
> low. Please feel free to merge this from my side.
> 
> For wherever it applies in the series:
> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> 

Thank you Laszlo.

Note to all: I have given ample time to anyone involved to respond to 
this series, and to this patch in particular. Therefore, I am going to 
assume there are no objections, and I intend to merge this entire series 
tomorrow.

Thanks,
Ard.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66259): https://edk2.groups.io/g/devel/message/66259
Mute This Topic: https://groups.io/mt/77270941/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-