[edk2-devel] [PATCH v3] UefiPayloadPkg: Fix issues when MULTIPLE_DEBUG_PORT_SUPPORT is true

paytonx.hsieh@intel.com posted 1 patch 12 months ago
Failed in applying to current master (apply log)
UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c      | 15 +++++-
UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c   | 56 ++++++++++++++++++++
UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf | 41 ++++++++++++++
UefiPayloadPkg/UefiPayloadPkg.dsc                                       | 11 +++-
4 files changed, 120 insertions(+), 3 deletions(-)
[edk2-devel] [PATCH v3] UefiPayloadPkg: Fix issues when MULTIPLE_DEBUG_PORT_SUPPORT is true
Posted by paytonx.hsieh@intel.com 12 months ago
From: PaytonX Hsieh <paytonx.hsieh@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4427

1. Since UART speed is slower than CPU, BIOS need to check the write
   buffer is empty, to avoid overwrite the buffer content.
2. LPSS UART might disable MMIO space for Windows debug usage during
   ExitBootServices event. BIOS need to avoid access the MMIO space
   after ExitBootServices.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Signed-off-by: PaytonX Hsieh <paytonx.hsieh@intel.com>
---
 UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c      | 15 +++++-
 UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c   | 56 ++++++++++++++++++++
 UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf | 41 ++++++++++++++
 UefiPayloadPkg/UefiPayloadPkg.dsc                                       | 11 +++-
 4 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c b/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c
index 8216195c62..82d0dd5855 100644
--- a/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c
+++ b/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c
@@ -52,7 +52,8 @@ typedef struct {
 } UART_INFO;
 
 UART_INFO  mUartInfo[MAX_SIZE];
-UINT8      mUartCount = 0;
+UINT8      mUartCount                     = 0;
+BOOLEAN    mBaseSerialPortLibHobAtRuntime = FALSE;
 
 /**
   Reads an 8-bit register. If UseMmio is TRUE, then the value is read from
@@ -285,6 +286,11 @@ SerialPortWrite (
     UseMmio     = mUartInfo[Count].UseMmio;
     Stride      = mUartInfo[Count].RegisterStride;
 
+    if (UseMmio && mBaseSerialPortLibHobAtRuntime) {
+      Count++;
+      continue;
+    }
+
     if (BaseAddress == 0) {
       Count++;
       continue;
@@ -294,6 +300,13 @@ SerialPortWrite (
     BytesLeft  = NumberOfBytes;
 
     while (BytesLeft != 0) {
+      //
+      // Wait for the serial port to be ready, to make sure both the transmit FIFO
+      // and shift register empty.
+      //
+      while ((SerialPortReadRegister (BaseAddress, R_UART_LSR, UseMmio, Stride) & B_UART_LSR_TXRDY) == 0) {
+      }
+
       //
       // Fill the entire Tx FIFO
       //
diff --git a/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c
new file mode 100644
index 0000000000..6106e9a933
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c
@@ -0,0 +1,56 @@
+/** @file
+  UART Serial Port library functions.
+
+  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#include <Uefi.h>
+
+extern BOOLEAN  mBaseSerialPortLibHobAtRuntime;
+
+/**
+  Set mSerialIoUartLibAtRuntime flag as TRUE after ExitBootServices.
+
+  @param[in]  Event   The Event that is being processed.
+  @param[in]  Context The Event Context.
+
+**/
+STATIC
+VOID
+EFIAPI
+BaseSerialPortLibHobExitBootServicesEvent (
+  IN EFI_EVENT  Event,
+  IN VOID       *Context
+  )
+{
+  mBaseSerialPortLibHobAtRuntime = TRUE;
+}
+
+/**
+  The constructor function registers a callback for the ExitBootServices event.
+
+  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
+  @param[in]  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The operation completed successfully.
+  @retval other         Either the serial port failed to initialize or the
+                        ExitBootServices event callback registration failed.
+**/
+EFI_STATUS
+EFIAPI
+DxeBaseSerialPortLibHobConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  EFI_EVENT  SerialPortLibHobExitBootServicesEvent;
+
+  return SystemTable->BootServices->CreateEvent (
+                                      EVT_SIGNAL_EXIT_BOOT_SERVICES,
+                                      TPL_NOTIFY,
+                                      BaseSerialPortLibHobExitBootServicesEvent,
+                                      NULL,
+                                      &SerialPortLibHobExitBootServicesEvent
+                                      );
+}
diff --git a/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf
new file mode 100644
index 0000000000..7bb3a6ae96
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf
@@ -0,0 +1,41 @@
+## @file
+#  SerialPortLib instance for UART information retrieved from bootloader.
+#
+#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeBaseSerialPortLibHob
+  FILE_GUID                      = c8def0c5-48e7-45b8-8299-485ea2e63b2c
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = SerialPortLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+  CONSTRUCTOR                    = DxeBaseSerialPortLibHobConstructor
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  PcdLib
+  IoLib
+  HobLib
+  TimerLib
+
+[Sources]
+  DxeBaseSerialPortLibHob.c
+  BaseSerialPortLibHob.c
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialLineControl
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl
+
+[Guids]
+  gUniversalPayloadSerialPortInfoGuid
diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 9847f189ff..998d222909 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -256,7 +256,11 @@
   SerialPortLib|UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf
   PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
 !else
-  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  !if $(MULTIPLE_DEBUG_PORT_SUPPORT) == TRUE
+    SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf
+  !else
+    SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  !endif
   PlatformHookLib|UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
 !endif
   PlatformBootManagerLib|UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -313,6 +317,9 @@
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   DxeHobListLib|UefiPayloadPkg/Library/DxeHobListLibNull/DxeHobListLibNull.inf
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+!if $(MULTIPLE_DEBUG_PORT_SUPPORT) == TRUE
+  SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf
+!endif
 
 [LibraryClasses.common.DXE_CORE]
   DxeHobListLib|UefiPayloadPkg/Library/DxeHobListLibNull/DxeHobListLibNull.inf
@@ -617,7 +624,7 @@
     <LibraryClasses>
       !if $(MULTIPLE_DEBUG_PORT_SUPPORT) == TRUE
         DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-        SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf
+        SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf
       !endif
       NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
   }
-- 
2.39.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103784): https://edk2.groups.io/g/devel/message/103784
Mute This Topic: https://groups.io/mt/98557876/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v3] UefiPayloadPkg: Fix issues when MULTIPLE_DEBUG_PORT_SUPPORT is true
Posted by Guo, Gua 12 months ago
Thanks for follow up that.

Reviewed-by: Gua Guo <gua.guo@intel.com>

-----Original Message-----
From: Hsieh, PaytonX <paytonx.hsieh@intel.com> 
Sent: Friday, April 28, 2023 7:41 PM
To: devel@edk2.groups.io
Cc: Hsieh, PaytonX <paytonx.hsieh@intel.com>; Dong, Guo <guo.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>
Subject: [PATCH v3] UefiPayloadPkg: Fix issues when MULTIPLE_DEBUG_PORT_SUPPORT is true

From: PaytonX Hsieh <paytonx.hsieh@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4427

1. Since UART speed is slower than CPU, BIOS need to check the write
   buffer is empty, to avoid overwrite the buffer content.
2. LPSS UART might disable MMIO space for Windows debug usage during
   ExitBootServices event. BIOS need to avoid access the MMIO space
   after ExitBootServices.

Cc: Guo Dong <guo.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Sean Rhodes <sean@starlabs.systems>
Cc: James Lu <james.lu@intel.com>
Cc: Gua Guo <gua.guo@intel.com>
Signed-off-by: PaytonX Hsieh <paytonx.hsieh@intel.com>
---
 UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c      | 15 +++++-
 UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c   | 56 ++++++++++++++++++++
 UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf | 41 ++++++++++++++
 UefiPayloadPkg/UefiPayloadPkg.dsc                                       | 11 +++-
 4 files changed, 120 insertions(+), 3 deletions(-)

diff --git a/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c b/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c
index 8216195c62..82d0dd5855 100644
--- a/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c
+++ b/UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.c
@@ -52,7 +52,8 @@ typedef struct {
 } UART_INFO;

 

 UART_INFO  mUartInfo[MAX_SIZE];

-UINT8      mUartCount = 0;

+UINT8      mUartCount                     = 0;

+BOOLEAN    mBaseSerialPortLibHobAtRuntime = FALSE;

 

 /**

   Reads an 8-bit register. If UseMmio is TRUE, then the value is read from

@@ -285,6 +286,11 @@ SerialPortWrite (
     UseMmio     = mUartInfo[Count].UseMmio;

     Stride      = mUartInfo[Count].RegisterStride;

 

+    if (UseMmio && mBaseSerialPortLibHobAtRuntime) {

+      Count++;

+      continue;

+    }

+

     if (BaseAddress == 0) {

       Count++;

       continue;

@@ -294,6 +300,13 @@ SerialPortWrite (
     BytesLeft  = NumberOfBytes;

 

     while (BytesLeft != 0) {

+      //

+      // Wait for the serial port to be ready, to make sure both the transmit FIFO

+      // and shift register empty.

+      //

+      while ((SerialPortReadRegister (BaseAddress, R_UART_LSR, UseMmio, Stride) & B_UART_LSR_TXRDY) == 0) {

+      }

+

       //

       // Fill the entire Tx FIFO

       //

diff --git a/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c
new file mode 100644
index 0000000000..6106e9a933
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.c
@@ -0,0 +1,56 @@
+/** @file

+  UART Serial Port library functions.

+

+  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>

+  SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+#include <Uefi.h>

+

+extern BOOLEAN  mBaseSerialPortLibHobAtRuntime;

+

+/**

+  Set mSerialIoUartLibAtRuntime flag as TRUE after ExitBootServices.

+

+  @param[in]  Event   The Event that is being processed.

+  @param[in]  Context The Event Context.

+

+**/

+STATIC

+VOID

+EFIAPI

+BaseSerialPortLibHobExitBootServicesEvent (

+  IN EFI_EVENT  Event,

+  IN VOID       *Context

+  )

+{

+  mBaseSerialPortLibHobAtRuntime = TRUE;

+}

+

+/**

+  The constructor function registers a callback for the ExitBootServices event.

+

+  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.

+  @param[in]  SystemTable   A pointer to the EFI System Table.

+

+  @retval EFI_SUCCESS   The operation completed successfully.

+  @retval other         Either the serial port failed to initialize or the

+                        ExitBootServices event callback registration failed.

+**/

+EFI_STATUS

+EFIAPI

+DxeBaseSerialPortLibHobConstructor (

+  IN EFI_HANDLE        ImageHandle,

+  IN EFI_SYSTEM_TABLE  *SystemTable

+  )

+{

+  EFI_EVENT  SerialPortLibHobExitBootServicesEvent;

+

+  return SystemTable->BootServices->CreateEvent (

+                                      EVT_SIGNAL_EXIT_BOOT_SERVICES,

+                                      TPL_NOTIFY,

+                                      BaseSerialPortLibHobExitBootServicesEvent,

+                                      NULL,

+                                      &SerialPortLibHobExitBootServicesEvent

+                                      );

+}

diff --git a/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf
new file mode 100644
index 0000000000..7bb3a6ae96
--- /dev/null
+++ b/UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf
@@ -0,0 +1,41 @@
+## @file

+#  SerialPortLib instance for UART information retrieved from bootloader.

+#

+#  Copyright (c) 2023, Intel Corporation. All rights reserved.<BR>

+#

+#  SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+##

+

+[Defines]

+  INF_VERSION                    = 0x00010005

+  BASE_NAME                      = DxeBaseSerialPortLibHob

+  FILE_GUID                      = c8def0c5-48e7-45b8-8299-485ea2e63b2c

+  MODULE_TYPE                    = DXE_DRIVER

+  VERSION_STRING                 = 1.0

+  LIBRARY_CLASS                  = SerialPortLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER

+  CONSTRUCTOR                    = DxeBaseSerialPortLibHobConstructor

+

+[Packages]

+  MdePkg/MdePkg.dec

+  MdeModulePkg/MdeModulePkg.dec

+

+[LibraryClasses]

+  PcdLib

+  IoLib

+  HobLib

+  TimerLib

+

+[Sources]

+  DxeBaseSerialPortLibHob.c

+  BaseSerialPortLibHob.c

+

+[Pcd]

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialLineControl

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl

+

+[Guids]

+  gUniversalPayloadSerialPortInfoGuid

diff --git a/UefiPayloadPkg/UefiPayloadPkg.dsc b/UefiPayloadPkg/UefiPayloadPkg.dsc
index 9847f189ff..998d222909 100644
--- a/UefiPayloadPkg/UefiPayloadPkg.dsc
+++ b/UefiPayloadPkg/UefiPayloadPkg.dsc
@@ -256,7 +256,11 @@
   SerialPortLib|UefiPayloadPkg/Library/CbSerialPortLib/CbSerialPortLib.inf

   PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf

 !else

-  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf

+  !if $(MULTIPLE_DEBUG_PORT_SUPPORT) == TRUE

+    SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf

+  !else

+    SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf

+  !endif

   PlatformHookLib|UefiPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf

 !endif

   PlatformBootManagerLib|UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf

@@ -313,6 +317,9 @@
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf

   DxeHobListLib|UefiPayloadPkg/Library/DxeHobListLibNull/DxeHobListLibNull.inf

   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf

+!if $(MULTIPLE_DEBUG_PORT_SUPPORT) == TRUE

+  SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf

+!endif

 

 [LibraryClasses.common.DXE_CORE]

   DxeHobListLib|UefiPayloadPkg/Library/DxeHobListLibNull/DxeHobListLibNull.inf

@@ -617,7 +624,7 @@
     <LibraryClasses>

       !if $(MULTIPLE_DEBUG_PORT_SUPPORT) == TRUE

         DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf

-        SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/BaseSerialPortLibHob.inf

+        SerialPortLib|UefiPayloadPkg/Library/BaseSerialPortLibHob/DxeBaseSerialPortLibHob.inf

       !endif

       NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf

   }

-- 
2.39.2.windows.1



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