[edk2-devel][edk2-platforms][PATCH v1 2/3] [WIP] KabylakeOpenBoardPkg/I2cHdmiDebugSerialPortLib: Commit local
Nate DeSimone
nathaniel.l.desimone at intel.com
Fri Sep 9 23:09:19 UTC 2022
> -----Original Message-----
> From: Benjamin Doron <benjamin.doron00 at gmail.com>
> Sent: Tuesday, September 6, 2022 10:27 AM
> To: devel at edk2.groups.io
> Cc: Chaganty, Rangasai V <rangasai.v.chaganty at intel.com>; Oram, Isaac W
> <isaac.w.oram at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Sinha, Ankit <ankit.sinha at intel.com>;
> Chiu, Chasel <chasel.chiu at intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH v1 2/3] [WIP]
> KabylakeOpenBoardPkg/I2cHdmiDebugSerialPortLib: Commit local
>
> While the key patches here should probably be merged into the initial
> patches for this library, it appears it isn't being merged soon.
> Therefore, commit the patches that improve the library for S3 use.
>
> Other than edits to INF LibraryClasses and header `#include`s, the primary
> patch here assists runtime by creating events to toggle a boolean such that
> gBS is not used after end-of-BS. This is necessary for DebugLibSerialPort, but
> not RSC, which uninstalls the serial port handler at end-of-BS.
>
> For S3 resume, a key finding was that this is still insufficient. The image is
> copied into the lockbox for its own security at DxeSmmReadyToLock, so
> toggling booleans in the data section is ineffective. Early DXE is fairly single-
> threaded and testing indicates that simply consuming ..TplNull.c is workable.
>
> Also, GCC 12 requires a patch to a `switch` block that improved flow
> coherency.
>
> Cc: Sai Chaganty <rangasai.v.chaganty at intel.com>
> Cc: Isaac Oram <isaac.w.oram at intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Cc: Ankit Sinha <ankit.sinha at intel.com>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Signed-off-by: Benjamin Doron <benjamin.doron00 at gmail.com>
> ---
> ...ptExecutorDxeI2cHdmiDebugSerialPortLib.inf | 48 ++++++++++
> .../DxeI2cHdmiDebugSerialPortLib.inf | 8 +-
> .../DxeSmmI2cHdmiDebugSerialPortLib.c | 2 -
> .../Library/I2cHdmiDebugSerialPortLib/Gmbus.c | 39 ++++----
> .../I2cDebugPortProtocol.c | 2 -
> .../I2cDebugPortTplDxe.c | 9 ++
> .../I2cDebugPortTplRuntimeDxe.c | 93 +++++++++++++++++++
> .../I2cHdmiDebugSerialPortLib.c | 3 -
> .../I2cHdmiDebugSerialPortLib/IgfxI2c.c | 9 +-
> .../PeiI2cHdmiDebugSerialPortLib.c | 1 -
> .../PeiI2cHdmiDebugSerialPortLib.inf | 5 +-
> .../RuntimeDxeI2cHdmiDebugSerialPortLib.inf | 51 ++++++++++
> .../SecI2cHdmiDebugSerialPortLib.c | 1 -
> .../SecI2cHdmiDebugSerialPortLib.inf | 7 +-
> .../SmmI2cHdmiDebugSerialPortLib.inf | 6 +-
> 15 files changed, 231 insertions(+), 53 deletions(-)
> create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/BootScriptExecutorDxeI2cHdmiDebugSerialPortLib.inf
> create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplRuntimeDxe.c
> create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/RuntimeDxeI2cHdmiDebugSerialPortLib.inf
>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/BootScriptExecutorDxeI2cHdmiDebugSerialPortLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/BootScriptExecutorDxeI2cHdmiDebugSerialPortLib.inf
> new file mode 100644
> index 000000000000..995e67bde7d4
> --- /dev/null
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/BootScriptExecutorDxeI2cHdmiDebugSerialPortLib.inf
> @@ -0,0 +1,48 @@
> +### @file
> +# Component description file for Serial I/O Port library for the HDMI I2C Debug Port
> +#
> +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = RuntimeDxeI2cHdmiDebugSerialPortLib
> + FILE_GUID = 7E514680-470B-409C-8FC4-2FE62BF010BC
> + VERSION_STRING = 1.0
> + MODULE_TYPE = DXE_DRIVER
> + LIBRARY_CLASS = SerialPortLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER SMM_CORE
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64
> +#
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + PcdLib
> + TimerLib
> + IoLib
> + PciLib
> + UefiLib
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + KabylakeOpenBoardPkg/OpenBoardPkg.dec
> +
> +[Sources]
> + DxeSmmI2cHdmiDebugSerialPortLib.c
> + Gmbus.c
> + Gmbus.h
> + I2cDebugPortProtocol.c
> + I2cDebugPortProtocol.h
> + I2cDebugPortTplNull.c
> + I2cHdmiDebugSerialPortLib.c
> + IgfxI2c.c
> + IgfxI2c.h
> +
> +[Pcd]
> + gKabylakeOpenBoardPkgTokenSpaceGuid.PcdI2cHdmiDebugPortDdcI2cChannel ## CONSUMES
> + gKabylakeOpenBoardPkgTokenSpaceGuid.PcdGttMmAddress ## CONSUMES
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeI2cHdmiDebugSerialPortLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeI2cHdmiDebugSerialPortLib.inf
> index 5403d8ae0fd7..5eeee504c7ec 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeI2cHdmiDebugSerialPortLib.inf
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeI2cHdmiDebugSerialPortLib.inf
> @@ -21,11 +21,13 @@
> #
>
> [LibraryClasses]
> - BaseLib
> BaseMemoryLib
> PcdLib
> TimerLib
> + IoLib
> PciLib
> + UefiBootServicesTableLib
> + UefiLib
>
> [Packages]
> MdePkg/MdePkg.dec
> @@ -42,10 +44,6 @@
> IgfxI2c.c
> IgfxI2c.h
>
> -[Ppis]
> -
> -[Guids]
> -
> [Pcd]
> gKabylakeOpenBoardPkgTokenSpaceGuid.PcdI2cHdmiDebugPortDdcI2cChannel ## CONSUMES
> gKabylakeOpenBoardPkgTokenSpaceGuid.PcdGttMmAddress ## CONSUMES
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeSmmI2cHdmiDebugSerialPortLib.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeSmmI2cHdmiDebugSerialPortLib.c
> index 5556e09a7419..46827c6cefae 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeSmmI2cHdmiDebugSerialPortLib.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/DxeSmmI2cHdmiDebugSerialPortLib.c
> @@ -8,10 +8,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> #include <Base.h>
> -#include <Library/BaseLib.h>
> #include <Library/SerialPortLib.h>
> #include <Library/PcdLib.h>
> -#include <Library/TimerLib.h>
>
> #include <IgfxI2c.h>
> #include <I2cDebugPortProtocol.h>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/Gmbus.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/Gmbus.c
> index c04bcd285060..df5dfd70a5f2 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/Gmbus.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/Gmbus.c
> @@ -7,8 +7,7 @@
> **/
>
> #include <Uefi.h>
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> +//#include <Library/DebugLib.h>
> #include <Library/PcdLib.h>
> #include <Library/IoLib.h>
> #include <Library/PciLib.h>
> @@ -33,6 +32,7 @@ GmbusGetGttMmAdr (
> // Check if GTT Memory Mapped BAR has been already assigned, initialize if not
> //
> GttMmPciAddress = PCI_LIB_ADDRESS (SA_IGD_BUS, SA_IGD_DEV, SA_IGD_FUN_0, R_SA_IGD_GTTMMADR);
> + // TODO(benjamindoron): TigerLake has 64-bit BAR
Please delete this comment. It lacks relevance to the scope of the current patch series. If you would like to port this to Tiger Lake at some point in the future, I would recommend filing a Bugzilla, assigning it to yourself, and making a note of the 64-bit BAR being one of the issues to be addressed when porting to newer silicon.
> GttMmAdr = PciRead32 (GttMmPciAddress) & 0xFFFFFFF0;
> //DEBUG ((DEBUG_INFO, "GttMmPciAddress = %x\n", (UINTN) GttMmPciAddress)); //@TODO
> //DEBUG ((DEBUG_INFO, "GttMmAdr = %x\n", (UINTN) GttMmAdr)); //@TODO
> @@ -361,6 +361,7 @@ GmbusPrepare (
> }
> //
> // Wait for GMBUS to complete any pending commands
> + // - TODO(benjamindoron): GmbusRecoverError()
Please remove the TODO and either put the call to GmbusRecoverError() in... or not. I think it is a good idea to add a call to GmbusRecoverError() in the case of GmbusWaitForReady() returning EFI_TIMEOUT.
> //
> Status = GmbusWaitForReady (B_SA_GTTMMADR_GMBUS2_INUSE, FALSE);
> if (EFI_ERROR (Status)) {
> @@ -488,28 +489,28 @@ GmbusRead (
> // Input Validation
> //
> if ((*ByteCount) <= 0) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - ByteCount is 0, no bytes to read.\n", __FUNCTION__));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - ByteCount is 0, no bytes to read.\n", __FUNCTION__));
> return EFI_INVALID_PARAMETER;
> }
> if ((*ByteCount) > GMBUS_MAX_BYTES) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - ByteCount is greater than GMBUS_MAX_BYTES[%d].\n", __FUNCTION__, GMBUS_MAX_BYTES));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - ByteCount is greater than GMBUS_MAX_BYTES[%d].\n", __FUNCTION__, GMBUS_MAX_BYTES));
> return EFI_INVALID_PARAMETER;
> }
> if (ReadBuffer == NULL) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - ReadBuffer is NULL.\n", __FUNCTION__));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - ReadBuffer is NULL.\n", __FUNCTION__));
> return EFI_INVALID_PARAMETER;
> }
> if ((SlaveAddress & BIT0) != BIT0) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - BIT0 of SlaveAddress should be set for an I2C read.\n", __FUNCTION__));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - BIT0 of SlaveAddress should be set for an I2C read.\n", __FUNCTION__));
> return EFI_INVALID_PARAMETER;
> }
>
> //
> // Configure Gmbus port and clock speed
> - //
> + //GMBUS_CLOCK_RATE_100K @todo
> Status = GmbusPrepare (GMBUS_CLOCK_RATE_50K, (DdcBusPinPair & B_SA_GTTMMADR_GMBUS0_PIN_PAIR_MASK));
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - GmbusPrepare() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GmbusPrepare() failed - %r\n", __FUNCTION__, Status));
> goto Done;
> }
>
> @@ -534,7 +535,7 @@ GmbusRead (
> //
> Status = SetGmbus1Command (GmbusCmdSts);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - SetGmbus1Command() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - SetGmbus1Command() failed - %r\n", __FUNCTION__, Status));
> goto Done;
> }
>
> @@ -556,7 +557,7 @@ GmbusRead (
> //
> Status = GmbusWaitForReady (B_SA_GTTMMADR_GMBUS2_HW_RDY, TRUE);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - GmbusWaitForReady() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GmbusWaitForReady() failed - %r\n", __FUNCTION__, Status));
> }
> //
> // Check the GMBUS2 register for error conditions (NACK or Slave Stall Timeout)
> @@ -564,13 +565,13 @@ GmbusRead (
> Status2 = GetGmbus2Status (&GmbusStatus);
> if (EFI_ERROR (Status2)) {
> Status = Status2;
> - DEBUG ((DEBUG_INFO, "Error: %a() - GetGmbus2Status() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GetGmbus2Status() failed - %r\n", __FUNCTION__, Status));
> goto Done;
> }
> if (EFI_ERROR (Status) && ((GmbusStatus & B_SA_GTTMMADR_GMBUS2_NACK_INDICATOR) == 0)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - Unexpected behavior detected!\n", __FUNCTION__));
> - DEBUG ((DEBUG_INFO, "The GMBUS controller did not encounter a NACK and it did not set the HW_RDY bit.\n"));
> - DEBUG ((DEBUG_INFO, "Status = %r\n", Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - Unexpected behavior detected!\n", __FUNCTION__));
> + //DEBUG ((DEBUG_INFO, "The GMBUS controller did not encounter a NACK and it did not set the HW_RDY bit.\n"));
> + //DEBUG ((DEBUG_INFO, "Status = %r\n", Status));
> Status = EFI_DEVICE_ERROR;
> goto Done;
> }
> @@ -580,10 +581,10 @@ GmbusRead (
> // If a NACK or Slave Stall Timeout occurs, then a bus error has occurred.
> // In the event of a bus error, one must reset the GMBUS controller to resume normal operation.
> //
> - DEBUG ((DEBUG_INFO, "Error: %a() - NACK occurred during read operation.\n", __FUNCTION__));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - NACK occurred during read operation.\n", __FUNCTION__));
> Status = GmbusRecoverError ();
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - GmbusRecoverError() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GmbusRecoverError() failed - %r\n", __FUNCTION__, Status));
> goto Done;
> }
> Status = EFI_DEVICE_ERROR;
> @@ -594,7 +595,7 @@ GmbusRead (
> //
> Status = GetGmbus3Data (&GmbusData);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - GetGmbus3Data() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GetGmbus3Data() failed - %r\n", __FUNCTION__, Status));
> goto Done;
> }
> for (Index = 0; (Index < sizeof (UINT32)) && (BytesRead < (*ByteCount)); Index++) {
> @@ -608,7 +609,7 @@ GmbusRead (
> //
> Status = GmbusWaitForReady (B_SA_GTTMMADR_GMBUS2_BUS_ACTIVE, FALSE);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "Error: %a() - GmbusWaitForReady() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GmbusWaitForReady() failed - %r\n", __FUNCTION__, Status));
> return Status;
> }
>
> @@ -616,7 +617,7 @@ Done:
> Status2 = GmbusRelease ();
> if (EFI_ERROR (Status2)) {
> Status = Status2;
> - DEBUG ((DEBUG_INFO, "Error: %a() - GmbusRelease() failed - %r\n", __FUNCTION__, Status));
> + //DEBUG ((DEBUG_INFO, "Error: %a() - GmbusRelease() failed - %r\n", __FUNCTION__, Status));
> }
> GmbusResetBusMaster ();
>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortProtocol.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortProtocol.c
> index 1a31c98347db..51eeadd75af9 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortProtocol.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortProtocol.c
> @@ -7,9 +7,7 @@
> **/
>
> #include <Base.h>
> -#include <Library/BaseLib.h>
> #include <Library/BaseMemoryLib.h>
> -#include <Library/PcdLib.h>
> #include <Library/TimerLib.h>
>
> #include <IgfxI2c.h>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplDxe.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplDxe.c
> index 9d69c0365795..d92b8d262793 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplDxe.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplDxe.c
> @@ -22,6 +22,11 @@ RaiseTplForI2cDebugPortAccess (
> VOID
> )
> {
> + // DebugLibSerialPort exposes potential DEBUG bugs, such as early assertions
> + if (gBS == NULL) {
> + return;
> + }
> +
> if (EfiGetCurrentTpl () < TPL_NOTIFY) {
> mPreviousTpl = gBS->RaiseTPL (TPL_NOTIFY);
> }
> @@ -37,6 +42,10 @@ RestoreTplAfterI2cDebugPortAccess (
> VOID
> )
> {
> + if (gBS == NULL) {
> + return;
> + }
> +
> if (mPreviousTpl > 0) {
> gBS->RestoreTPL (mPreviousTpl);
> mPreviousTpl = 0;
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplRuntimeDxe.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplRuntimeDxe.c
> new file mode 100644
> index 000000000000..7aa95157d734
> --- /dev/null
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cDebugPortTplRuntimeDxe.c
> @@ -0,0 +1,93 @@
> +/** @file
> + Serial I/O Port library implementation for the HDMI I2C Debug Port
> + DXE Library implementation
> +
> +Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +STATIC EFI_TPL mPreviousTpl = 0;
> +STATIC EFI_EVENT mEvent = NULL;
> +STATIC UINT8 mEndOfBootServices = 0;
> +
> +/**
> + Exit Boot Services Event notification handler.
> +
> + @param[in] Event Event whose notification function is being invoked
> + @param[in] Context Pointer to the notification function's context
> +
> +**/
> +VOID
> +EFIAPI
> +OnExitBootServices (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + mEndOfBootServices = 1;
> +
> + gBS->CloseEvent (Event);
> +}
> +
> +/**
> + For boot phases that utilize task priority levels (TPLs), this function raises
> + the TPL to the appriopriate level needed to execute I/O to the I2C Debug Port
> +**/
> +VOID
> +RaiseTplForI2cDebugPortAccess (
> + VOID
> + )
> +{
> + // DebugLibSerialPort exposes potential DEBUG bugs, such as early assertions
> + if (gBS == NULL || mEndOfBootServices == 1) {
> + return;
> + }
> +
> + // An event is required for a boolean to bypass TPL modification after
> + // exit-BS. RSC obviates this, requiring it for runtime DebugLibSerialPort.
> + // - Consider creating a TplRuntimeDxe, although UefiRuntimeLib uses gST?
> + // - BootScriptExecutorDxe is a special-case, where booleans are ineffective
> + //
> + // A constructor would cycle, SerialPortInitialize() takes no arguments,
> + // and no BootServicesTableLib can be called by AutoGen early enough.
> + // Therefore, we generate the event here.
> + if (mEvent == NULL) {
> + gBS->CreateEventEx (
> + EVT_NOTIFY_SIGNAL,
> + TPL_NOTIFY,
> + OnExitBootServices,
> + NULL,
> + &gEfiEventExitBootServicesGuid,
> + &mEvent
> + );
> + }
> +
> + if (EfiGetCurrentTpl () < TPL_NOTIFY) {
> + mPreviousTpl = gBS->RaiseTPL (TPL_NOTIFY);
> + }
> +}
> +
> +/**
> + For boot phases that utilize task priority levels (TPLs), this function
> + restores the TPL to the previous level after I/O to the I2C Debug Port is
> + complete
> +**/
> +VOID
> +RestoreTplAfterI2cDebugPortAccess (
> + VOID
> + )
> +{
> + if (gBS == NULL || mEndOfBootServices == 1) {
> + return;
> + }
> +
> + if (mPreviousTpl > 0) {
> + gBS->RestoreTPL (mPreviousTpl);
> + mPreviousTpl = 0;
> + }
> +}
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cHdmiDebugSerialPortLib.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cHdmiDebugSerialPortLib.c
> index 89a01b868da3..402b5a3033a9 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cHdmiDebugSerialPortLib.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/I2cHdmiDebugSerialPortLib.c
> @@ -7,10 +7,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> #include <Base.h>
> -#include <Library/BaseLib.h>
> #include <Library/SerialPortLib.h>
> -#include <Library/PcdLib.h>
> -#include <Library/TimerLib.h>
>
> #include <IgfxI2c.h>
> #include <I2cDebugPortProtocol.h>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/IgfxI2c.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/IgfxI2c.c
> index b1273c7a5d10..886351ad4297 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/IgfxI2c.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/IgfxI2c.c
> @@ -7,8 +7,6 @@
> **/
>
> #include <Uefi.h>
> -#include <Library/BaseLib.h>
> -#include <Library/IoLib.h>
> #include <Library/PciLib.h>
> #include <IgfxI2c.h>
>
> @@ -84,7 +82,6 @@ GetGmbusBusPinPair (
> default:
> return EFI_INVALID_PARAMETER;
> }
> - break;
I realize that the break is redundant because all possible paths above here return. Still, it is best practice to keep it just in case someone adds a new case to the nested switch statement in the future that does not contain a return.
> // The PCH design lineage from newer CoffeeLake & WhiskeyLake
> case PchTypeCnlLp:
> case PchTypeCnlH:
> @@ -105,8 +102,8 @@ GetGmbusBusPinPair (
> default:
> return EFI_INVALID_PARAMETER;
> }
> - break;
I realize that the break is redundant because all possible paths above here return. Still, it is best practice to keep it just in case someone adds a new case to the nested switch statement in the future that does not contain a return.
> - }
>
> - return EFI_UNSUPPORTED;
> + default:
> + return EFI_UNSUPPORTED;
> + }
> }
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.c
> index c99821367354..05b2d31bbfc2 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.c
> @@ -10,7 +10,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <Base.h>
> #include <PiPei.h>
>
> -#include <Library/BaseLib.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/SerialPortLib.h>
> #include <Library/HobLib.h>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.inf
> index 62b3cd3e1e49..64d8f682b786 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.inf
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.inf
> @@ -21,10 +21,11 @@
> #
>
> [LibraryClasses]
> - BaseLib
> BaseMemoryLib
> PcdLib
> + HobLib
> TimerLib
> + IoLib
> PciLib
>
> [Packages]
> @@ -42,8 +43,6 @@
> IgfxI2c.c
> IgfxI2c.h
>
> -[Ppis]
> -
> [Guids]
> gI2cHdmiDebugHobGuid
>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/RuntimeDxeI2cHdmiDebugSerialPortLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/RuntimeDxeI2cHdmiDebugSerialPortLib.inf
> new file mode 100644
> index 000000000000..eefe85f2814c
> --- /dev/null
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/RuntimeDxeI2cHdmiDebugSerialPortLib.inf
> @@ -0,0 +1,51 @@
> +### @file
> +# Component description file for Serial I/O Port library for the HDMI I2C Debug Port
> +#
> +# Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = RuntimeDxeI2cHdmiDebugSerialPortLib
> + FILE_GUID = 08891D97-994C-48E9-9983-E99D622D32C8
> + VERSION_STRING = 1.0
> + MODULE_TYPE = DXE_DRIVER
> + LIBRARY_CLASS = SerialPortLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER SMM_CORE
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +# VALID_ARCHITECTURES = IA32 X64
> +#
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + PcdLib
> + TimerLib
> + IoLib
> + PciLib
> + UefiLib
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + KabylakeOpenBoardPkg/OpenBoardPkg.dec
> +
> +[Sources]
> + DxeSmmI2cHdmiDebugSerialPortLib.c
> + Gmbus.c
> + Gmbus.h
> + I2cDebugPortProtocol.c
> + I2cDebugPortProtocol.h
> + I2cDebugPortTplRuntimeDxe.c
> + I2cHdmiDebugSerialPortLib.c
> + IgfxI2c.c
> + IgfxI2c.h
> +
> +[Guids]
> + gEfiEventExitBootServicesGuid ## CONSUMES ## Event
> +
> +[Pcd]
> + gKabylakeOpenBoardPkgTokenSpaceGuid.PcdI2cHdmiDebugPortDdcI2cChannel ## CONSUMES
> + gKabylakeOpenBoardPkgTokenSpaceGuid.PcdGttMmAddress ## CONSUMES
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.c
> index 416114d4363c..e4a65a66d9c1 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.c
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.c
> @@ -8,7 +8,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> #include <Base.h>
> -#include <Library/BaseLib.h>
> #include <Library/SerialPortLib.h>
> #include <Library/PcdLib.h>
>
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.inf
> index 3ae724926fdd..a9780decd1a7 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.inf
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SecI2cHdmiDebugSerialPortLib.inf
> @@ -21,10 +21,9 @@
> #
>
> [LibraryClasses]
> - BaseLib
> BaseMemoryLib
> - PcdLib
> TimerLib
> + IoLib
> PciLib
>
> [Packages]
> @@ -42,10 +41,6 @@
> IgfxI2c.h
> SecI2cHdmiDebugSerialPortLib.c
>
> -[Ppis]
> -
> -[Guids]
> -
> [Pcd]
> gKabylakeOpenBoardPkgTokenSpaceGuid.PcdI2cHdmiDebugPortDdcI2cChannel ## CONSUMES
> gKabylakeOpenBoardPkgTokenSpaceGuid.PcdGttMmAddress ## CONSUMES
> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SmmI2cHdmiDebugSerialPortLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SmmI2cHdmiDebugSerialPortLib.inf
> index dcbf43b886c1..65f2b4f5f731 100644
> --- a/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SmmI2cHdmiDebugSerialPortLib.inf
> +++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/I2cHdmiDebugSerialPortLib/SmmI2cHdmiDebugSerialPortLib.inf
> @@ -21,10 +21,10 @@
> #
>
> [LibraryClasses]
> - BaseLib
> BaseMemoryLib
> PcdLib
> TimerLib
> + IoLib
> PciLib
>
> [Packages]
> @@ -42,10 +42,6 @@
> IgfxI2c.c
> IgfxI2c.h
>
> -[Ppis]
> -
> -[Guids]
> -
> [Pcd]
> gKabylakeOpenBoardPkgTokenSpaceGuid.PcdI2cHdmiDebugPortDdcI2cChannel ## CONSUMES
> gKabylakeOpenBoardPkgTokenSpaceGuid.PcdGttMmAddress ## CONSUMES
> --
> 2.37.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#93620): https://edk2.groups.io/g/devel/message/93620
Mute This Topic: https://groups.io/mt/93506703/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list