public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io
Cc: Chasel Chiu <chasel.chiu@intel.com>,
	Nate DeSimone <nathaniel.l.desimone@intel.com>,
	Benjamin Doron <benjamin.doron00@gmail.com>
Subject: [edk2-platforms][PATCH v1 1/1] KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Remove PPI from HOB
Date: Fri,  9 Jul 2021 12:08:28 -0400	[thread overview]
Message-ID: <20210709160828.17737-1-mikuback@linux.microsoft.com> (raw)

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

The PPI pointer should not be cached in a HOB as it could get out
of sync with the current pointers in the PPI list.

Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Benjamin Doron <benjamin.doron00@gmail.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c | 114 +++++++++-----------
 1 file changed, 48 insertions(+), 66 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c
index 0230149a38c4..fc48bdc6fccb 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c
@@ -2,6 +2,7 @@
   Serial I/O Port library implementation for output to SPI flash
 
 Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -17,40 +18,23 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/SpiLib.h>
 
 typedef struct {
-  PCH_SPI_PPI           *PchSpiPpi;
   UINT32                CurrentWriteOffset;
 } SPI_FLASH_DEBUG_CONTEXT;
 
 /**
-  Update reference to the most recent PCH SPI PPI installed
+  Returns a pointer to the PCH SPI PPI.
 
-  @param PeiServices       An indirect pointer to the EFI_PEI_SERVICES table published by the PEI Foundation
-  @param NotifyDescriptor  Address of the notification descriptor data structure.
-  @param Ppi               Address of the PPI that was installed.
-
-  @retval EFI_SUCCESS      Successfully update the PCH SPI PPI reference
-  @retval EFI_NOT_FOUND    An error occurred locating a required interface
-  @retval EFI_NOT_SUPPORTED
+  @return Pointer to PCH_SPI_PPI    If an instance of the PCH SPI PPI is found
+  @return NULL                      If an instance of the PCH SPI PPI is not found
 
 **/
-EFI_STATUS
-EFIAPI
-SpiPpiNotifyCallback (
-  IN EFI_PEI_SERVICES           **PeiServices,
-  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
-  IN VOID                       *Ppi
+PCH_SPI_PPI *
+GetSpiPpi (
+  VOID
   )
 {
-  EFI_STATUS                Status;
-  EFI_HOB_GUID_TYPE         *GuidHob;
-  PCH_SPI_PPI               *PchSpiPpi;
-  SPI_FLASH_DEBUG_CONTEXT   *Context;
-
-  GuidHob = GetFirstGuidHob (&gSpiFlashDebugHobGuid);
-  if (GuidHob == NULL) {
-    return EFI_NOT_FOUND;
-  }
-  Context = GET_GUID_HOB_DATA (GuidHob);
+  EFI_STATUS    Status;
+  PCH_SPI_PPI   *PchSpiPpi;
 
   Status =  PeiServicesLocatePpi (
               &gPchSpiPpiGuid,
@@ -59,27 +43,17 @@ SpiPpiNotifyCallback (
               (VOID **) &PchSpiPpi
               );
   if (EFI_ERROR (Status)) {
-    return EFI_NOT_FOUND;
+    return NULL;
   }
 
-  Context->PchSpiPpi = PchSpiPpi;
-
-  return EFI_SUCCESS;
+  return PchSpiPpi;
 }
 
-EFI_PEI_NOTIFY_DESCRIPTOR mSpiPpiNotifyList[] = {
-  {
-    (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
-    &gPchSpiPpiGuid,
-    SpiPpiNotifyCallback
-  }
-};
-
 /**
   Common function to write trace data to a chosen debug interface like
   UART Serial device, USB Serial device or Trace Hub device
 
-  @param  Buffer           Point of data buffer which need to be writed.
+  @param  Buffer           Point of data buffer which needs to be written.
   @param  NumberOfBytes    Number of output bytes which are cached in Buffer.
 
 **/
@@ -93,6 +67,7 @@ SerialPortWrite (
   EFI_STATUS                Status;
   EFI_HOB_GUID_TYPE         *GuidHob;
   SPI_FLASH_DEBUG_CONTEXT   *Context;
+  PCH_SPI_PPI               *PchSpiPpi;
   UINT32                    BytesWritten;
   UINT32                    SourceBufferOffset;
   UINT32                    NvMessageAreaSize;
@@ -111,18 +86,22 @@ SerialPortWrite (
     return 0;
   }
   Context = GET_GUID_HOB_DATA (GuidHob);
-  if (Context == NULL || Context->PchSpiPpi == NULL || Context->CurrentWriteOffset >= NvMessageAreaSize) {
+  if (Context == NULL || Context->CurrentWriteOffset >= NvMessageAreaSize) {
+    return 0;
+  }
+  PchSpiPpi = GetSpiPpi ();
+  if (PchSpiPpi == NULL) {
     return 0;
   }
 
   if ((Context->CurrentWriteOffset + NumberOfBytes) / NvMessageAreaSize > 0) {
     LinearOffset = (UINT32) (FixedPcdGet32 (PcdFlashNvDebugMessageBase) - FixedPcdGet32 (PcdFlashAreaBaseAddress));
-    Status =  Context->PchSpiPpi->FlashErase (
-                                    Context->PchSpiPpi,
-                                    FlashRegionBios,
-                                    LinearOffset,
-                                    NvMessageAreaSize
-                                    );
+    Status = PchSpiPpi->FlashErase (
+                          PchSpiPpi,
+                          FlashRegionBios,
+                          LinearOffset,
+                          NvMessageAreaSize
+                          );
     if (!EFI_ERROR (Status)) {
       Context->CurrentWriteOffset = 0;
     } else {
@@ -137,13 +116,13 @@ SerialPortWrite (
 
   LinearOffset = (FixedPcdGet32 (PcdFlashNvDebugMessageBase) + Context->CurrentWriteOffset) - FixedPcdGet32 (PcdFlashAreaBaseAddress);
 
-  Status =  Context->PchSpiPpi->FlashWrite (
-                                  Context->PchSpiPpi,
-                                  FlashRegionBios,
-                                  LinearOffset,
-                                  BytesWritten,
-                                  (UINT8 *) &Buffer[SourceBufferOffset]
-                                  );
+  Status = PchSpiPpi->FlashWrite (
+                        PchSpiPpi,
+                        FlashRegionBios,
+                        LinearOffset,
+                        BytesWritten,
+                        (UINT8 *) &Buffer[SourceBufferOffset]
+                        );
   if (!EFI_ERROR (Status)) {
     Context->CurrentWriteOffset += BytesWritten;
     return BytesWritten;
@@ -295,26 +274,29 @@ SerialPortInitialize (
   )
 {
   EFI_STATUS                Status;
+  EFI_HOB_GUID_TYPE         *GuidHob;
   SPI_FLASH_DEBUG_CONTEXT   *Context;
 
+  GuidHob = GetFirstGuidHob (&gSpiFlashDebugHobGuid);
+  if (GuidHob != NULL) {
+    // Initialization only needs to occur once
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Perform silicon specific initialization required to enable write to SPI flash.
+  //
+  Status = SpiServiceInit ();
+  if (EFI_ERROR (Status)) {
+    return EFI_DEVICE_ERROR;
+  }
+
   Context = (SPI_FLASH_DEBUG_CONTEXT *) BuildGuidHob (&gSpiFlashDebugHobGuid, sizeof (SPI_FLASH_DEBUG_CONTEXT));
   if (Context == NULL) {
     return EFI_DEVICE_ERROR;
   }
+
   ZeroMem ((VOID *) Context, sizeof (SPI_FLASH_DEBUG_CONTEXT));
 
-  Status = PeiServicesNotifyPpi (&mSpiPpiNotifyList[0]);
-  if (EFI_ERROR (Status)) {
-    return EFI_DEVICE_ERROR;
-  }
-
-  //
-  // Perform silicon specific initialization required to enable write to SPI flash.
-  //
-  Status = SpiServiceInit ();
-  if (EFI_ERROR (Status)) {
-    Status = EFI_DEVICE_ERROR;
-  }
-
-  return Status;
+  return EFI_SUCCESS;
 }
-- 
2.28.0.windows.1


             reply	other threads:[~2021-07-09 16:08 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-09 16:08 Michael Kubacki [this message]
2021-07-29  1:41 ` [edk2-platforms][PATCH v1 1/1] KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash: Remove PPI from HOB Nate DeSimone
2021-07-29  1:47 ` Nate DeSimone

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210709160828.17737-1-mikuback@linux.microsoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox