public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2
@ 2022-07-27 12:27 Maciej Czajkowski
  2022-07-27 12:27 ` [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition Maciej Czajkowski
  2022-07-27 12:27 ` [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Maciej Czajkowski
  0 siblings, 2 replies; 6+ messages in thread
From: Maciej Czajkowski @ 2022-07-27 12:27 UTC (permalink / raw)
  To: devel

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

The purpose of those changes is to introduce the way to enumerate and assign resources in PEI for the systems with more than one PCI root. 
Here is a need to have an interface that will support such a mechanizm.
For now, the part that performs the enumeration will be implemented in the silicon code.
Sample code can be seen here: https://github.com/mczaj/edk2-platforms/commit/d443062e58f9fba228869b54f2546d9735b3b506

v1: https://edk2.groups.io/g/devel/topic/91575907

v2 changes:
- collected Acked-by tag for no.1 commit
- followed-up with change suggestions in no.2 commit

Maciej Czajkowski (2):
  MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition
  MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device

 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c    | 585 ++++++++++++++------
 MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c |  44 --
 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h    |  17 +-
 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf  |   5 +-
 MdeModulePkg/Include/Ppi/PciDevice.h      |  32 ++
 MdeModulePkg/MdeModulePkg.dec             |   3 +
 MdeModulePkg/MdeModulePkg.dsc             |   1 +
 7 files changed, 465 insertions(+), 222 deletions(-)
 create mode 100644 MdeModulePkg/Include/Ppi/PciDevice.h

-- 
2.27.0.windows.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition
  2022-07-27 12:27 [PATCH v2 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2 Maciej Czajkowski
@ 2022-07-27 12:27 ` Maciej Czajkowski
  2022-07-28  3:30   ` Wu, Hao A
  2022-07-27 12:27 ` [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Maciej Czajkowski
  1 sibling, 1 reply; 6+ messages in thread
From: Maciej Czajkowski @ 2022-07-27 12:27 UTC (permalink / raw)
  To: devel; +Cc: Hao A Wu, Ray Ni, Liming Gao

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

This commit introduces EDKII_PCI_DEVICE_PPI. The purpose of this PPI is
to provide a way of accessing PCI devices to drvice drivers such as
NvmExpressPei or AhciPei.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
Acked-by: Hao A Wu <hao.a.wu@intel.com>
---

Notes:
    v2 changes:
    - added acked-by tag
    - minor change - modified header guard to follow coding style

 MdeModulePkg/Include/Ppi/PciDevice.h | 32 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec        |  3 ++
 2 files changed, 35 insertions(+)

diff --git a/MdeModulePkg/Include/Ppi/PciDevice.h b/MdeModulePkg/Include/Ppi/PciDevice.h
new file mode 100644
index 000000000000..6750ae6ce394
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/PciDevice.h
@@ -0,0 +1,32 @@
+/** @file
+
+  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef EDKII_PCI_DEVICE_PPI_H_
+#define EDKII_PCI_DEVICE_PPI_H_
+
+#include <Protocol/PciIo.h>
+#include <Protocol/DevicePath.h>
+
+///
+/// Global ID for the EDKII_PCI_DEVICE_PPI_GUID.
+///
+#define EDKII_PCI_DEVICE_PPI_GUID \
+  { \
+    0x1597ab4f, 0xd542, 0x4efe, { 0x9a, 0xf7, 0xb2, 0x44, 0xec, 0x54, 0x4c, 0x0b } \
+  }
+
+///
+/// PCI Device PPI structure.
+///
+typedef struct {
+  EFI_PCI_IO_PROTOCOL         PciIo;
+  EFI_DEVICE_PATH_PROTOCOL    *DevicePath;
+} EDKII_PCI_DEVICE_PPI;
+
+extern EFI_GUID  gEdkiiPeiPciDevicePpiGuid;
+
+#endif // EDKII_PCI_DEVICE_PPI_H_
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 2bcb9f9453af..7d989108324a 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -509,6 +509,9 @@ [Ppis]
   ## Include/Ppi/NvmExpressPassThru.h
   gEdkiiPeiNvmExpressPassThruPpiGuid    = { 0x6af31b2c, 0x3be, 0x46c1, { 0xb1, 0x2d, 0xea, 0x4a, 0x36, 0xdf, 0xa7, 0x4c } }
 
+  ## Include/Ppi/PciDevice.h
+  gEdkiiPeiPciDevicePpiGuid                 = { 0x1597ab4f, 0xd542, 0x4efe, { 0x9a, 0xf7, 0xb2, 0x44, 0xec, 0x54, 0x4c, 0x0b } }
+
   ## Include/Ppi/CapsuleOnDisk.h
   gEdkiiPeiCapsuleOnDiskPpiGuid             = { 0x71a9ea61, 0x5a35, 0x4a5d, { 0xac, 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
   gEdkiiPeiBootInCapsuleOnDiskModePpiGuid   = { 0xb08a11e4, 0xe2b7, 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1  } }
-- 
2.27.0.windows.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device
  2022-07-27 12:27 [PATCH v2 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2 Maciej Czajkowski
  2022-07-27 12:27 ` [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition Maciej Czajkowski
@ 2022-07-27 12:27 ` Maciej Czajkowski
  2022-07-28  3:29   ` Wu, Hao A
  1 sibling, 1 reply; 6+ messages in thread
From: Maciej Czajkowski @ 2022-07-27 12:27 UTC (permalink / raw)
  To: devel; +Cc: Hao A Wu, Ray Ni, Liming Gao

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

This change modifies AhciPei library to allow usage both EDKII_PCI_DEVICE_PPI
and EDKII_PEI_ATA_AHCI_HOST_CONTROLLER_PPI to manage ATA HDD working under
AHCI mode.

Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
---

Notes:
    v2 changes:
    - added missing function descriptions
    - moved controller initialization from PCI_DEVICE_PPI to seperate funciton
    in order to reduce code duplication
    - added DevicePathLib BASE instance to the MdeModulePkg.dec to allow PEIMs to consume it

 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c    | 585 ++++++++++++++------
 MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c |  44 --
 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h    |  17 +-
 MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf  |   5 +-
 MdeModulePkg/MdeModulePkg.dsc             |   1 +
 5 files changed, 430 insertions(+), 222 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
index 208b7e9a3606..8ad98dc76bc1 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
@@ -9,6 +9,47 @@
 **/
 
 #include "AhciPei.h"
+#include <Ppi/PciDevice.h>
+#include <Library/DevicePathLib.h>
+#include <IndustryStandard/Pci.h>
+
+/**
+  Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation.
+
+  @param[in] PeiServices         Pointer to PEI Services Table.
+  @param[in] NotifyDescriptor    Pointer to the descriptor for the Notification
+                                 event that caused this function to execute.
+  @param[in] Ppi                 Pointer to the PPI data associated with this function.
+
+  @retval EFI_SUCCESS    The function completes successfully
+
+**/
+EFI_STATUS
+EFIAPI
+AtaAhciHostControllerPpiInstallationCallback (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID                       *Ppi
+  );
+
+/**
+  Callback for EDKII_PCI_DEVICE_PPI installation.
+
+  @param[in] PeiServices         Pointer to PEI Services Table.
+  @param[in] NotifyDescriptor    Pointer to the descriptor for the Notification
+                                 event that caused this function to execute.
+  @param[in] Ppi                 Pointer to the PPI data associated with this function.
+
+  @retval EFI_SUCCESS    The function completes successfully
+
+**/
+EFI_STATUS
+EFIAPI
+AtaAhciPciDevicePpiInstallationCallback (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID                       *Ppi
+  );
 
 EFI_PEI_PPI_DESCRIPTOR  mAhciAtaPassThruPpiListTemplate = {
   (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
@@ -40,6 +81,18 @@ EFI_PEI_NOTIFY_DESCRIPTOR  mAhciEndOfPeiNotifyListTemplate = {
   AhciPeimEndOfPei
 };
 
+EFI_PEI_NOTIFY_DESCRIPTOR  mAtaAhciHostControllerNotify = {
+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEdkiiPeiAtaAhciHostControllerPpiGuid,
+  AtaAhciHostControllerPpiInstallationCallback
+};
+
+EFI_PEI_NOTIFY_DESCRIPTOR  mPciDevicePpiNotify = {
+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+  &gEdkiiPeiPciDevicePpiGuid,
+  AtaAhciPciDevicePpiInstallationCallback
+};
+
 /**
   Free the DMA resources allocated by an ATA AHCI controller.
 
@@ -111,33 +164,30 @@ AhciPeimEndOfPei (
 }
 
 /**
-  Entry point of the PEIM.
+  Initialize and install PrivateData PPIs.
 
-  @param[in] FileHandle     Handle of the file being invoked.
-  @param[in] PeiServices    Describes the list of possible PEI Services.
-
-  @retval EFI_SUCCESS    PPI successfully installed.
+  @param[in] MmioBase            MMIO base address of specific AHCI controller
+  @param[in] DevicePath          A pointer to the EFI_DEVICE_PATH_PROTOCOL
+                                 structure.
+  @param[in] DevicePathLength    Length of the device path.
 
+  @retval EFI_SUCCESS  AHCI controller initialized and PPIs installed
+  @retval others       Failed to initialize AHCI controller
 **/
 EFI_STATUS
-EFIAPI
-AtaAhciPeimEntry (
-  IN EFI_PEI_FILE_HANDLE     FileHandle,
-  IN CONST EFI_PEI_SERVICES  **PeiServices
+AtaAhciInitPrivateData (
+  IN UINTN                     MmioBase,
+  IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath,
+  IN UINTN                     DevicePathLength
   )
 {
-  EFI_STATUS                          Status;
-  EFI_BOOT_MODE                       BootMode;
-  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;
-  UINT8                               Controller;
-  UINTN                               MmioBase;
-  UINTN                               DevicePathLength;
-  EFI_DEVICE_PATH_PROTOCOL            *DevicePath;
-  UINT32                              PortBitMap;
-  PEI_AHCI_CONTROLLER_PRIVATE_DATA    *Private;
-  UINT8                               NumberOfPorts;
+  EFI_STATUS                        Status;
+  UINT32                            PortBitMap;
+  UINT8                             NumberOfPorts;
+  PEI_AHCI_CONTROLLER_PRIVATE_DATA  *Private;
+  EFI_BOOT_MODE                     BootMode;
 
-  DEBUG ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "Initializing private data for ATA\n"));
 
   //
   // Get the current boot mode.
@@ -149,19 +199,149 @@ AtaAhciPeimEntry (
   }
 
   //
-  // Locate the ATA AHCI host controller PPI.
-  //
-  Status = PeiServicesLocatePpi (
-             &gEdkiiPeiAtaAhciHostControllerPpiGuid,
-             0,
-             NULL,
-             (VOID **)&AhciHcPpi
-             );
+  // Check validity of the device path of the ATA AHCI controller.
+  //
+  Status = AhciIsHcDevicePathValid (DevicePath, DevicePathLength);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: The device path is invalid.\n",
+      __FUNCTION__
+      ));
+    return Status;
+  }
+
+  //
+  // For S3 resume performance consideration, not all ports on an ATA AHCI
+  // controller will be enumerated/initialized. The driver consumes the
+  // content within S3StorageDeviceInitList LockBox to get the ports that
+  // will be enumerated/initialized during S3 resume.
+  //
+  if (BootMode == BOOT_ON_S3_RESUME) {
+    NumberOfPorts = AhciS3GetEumeratePorts (DevicePath, DevicePathLength, &PortBitMap);
+    if (NumberOfPorts == 0) {
+      return EFI_SUCCESS;
+    }
+  } else {
+    PortBitMap = MAX_UINT32;
+  }
+
+  //
+  // Memory allocation for controller private data.
+  //
+  Private = AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA));
+  if (Private == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Fail to allocate private data.\n",
+      __FUNCTION__
+      ));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  //
+  // Initialize controller private data.
+  //
+  Private->Signature        = AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE;
+  Private->MmioBase         = MmioBase;
+  Private->DevicePathLength = DevicePathLength;
+  Private->DevicePath       = DevicePath;
+  Private->PortBitMap       = PortBitMap;
+  InitializeListHead (&Private->DeviceList);
+
+  Status = AhciModeInitialization (Private);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Failed to locate AtaAhciHostControllerPpi.\n", __FUNCTION__));
-    return EFI_UNSUPPORTED;
+    return Status;
+  }
+
+  Private->AtaPassThruMode.Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |
+                                        EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
+  Private->AtaPassThruMode.IoAlign      = sizeof (UINTN);
+  Private->AtaPassThruPpi.Revision      = EDKII_PEI_ATA_PASS_THRU_PPI_REVISION;
+  Private->AtaPassThruPpi.Mode          = &Private->AtaPassThruMode;
+  Private->AtaPassThruPpi.PassThru      = AhciAtaPassThruPassThru;
+  Private->AtaPassThruPpi.GetNextPort   = AhciAtaPassThruGetNextPort;
+  Private->AtaPassThruPpi.GetNextDevice = AhciAtaPassThruGetNextDevice;
+  Private->AtaPassThruPpi.GetDevicePath = AhciAtaPassThruGetDevicePath;
+  CopyMem (
+    &Private->AtaPassThruPpiList,
+    &mAhciAtaPassThruPpiListTemplate,
+    sizeof (EFI_PEI_PPI_DESCRIPTOR)
+    );
+  Private->AtaPassThruPpiList.Ppi = &Private->AtaPassThruPpi;
+  PeiServicesInstallPpi (&Private->AtaPassThruPpiList);
+
+  Private->BlkIoPpi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo;
+  Private->BlkIoPpi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo;
+  Private->BlkIoPpi.ReadBlocks              = AhciBlockIoReadBlocks;
+  CopyMem (
+    &Private->BlkIoPpiList,
+    &mAhciBlkIoPpiListTemplate,
+    sizeof (EFI_PEI_PPI_DESCRIPTOR)
+    );
+  Private->BlkIoPpiList.Ppi = &Private->BlkIoPpi;
+  PeiServicesInstallPpi (&Private->BlkIoPpiList);
+
+  Private->BlkIo2Ppi.Revision                = EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION;
+  Private->BlkIo2Ppi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo2;
+  Private->BlkIo2Ppi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo2;
+  Private->BlkIo2Ppi.ReadBlocks              = AhciBlockIoReadBlocks2;
+  CopyMem (
+    &Private->BlkIo2PpiList,
+    &mAhciBlkIo2PpiListTemplate,
+    sizeof (EFI_PEI_PPI_DESCRIPTOR)
+    );
+  Private->BlkIo2PpiList.Ppi = &Private->BlkIo2Ppi;
+  PeiServicesInstallPpi (&Private->BlkIo2PpiList);
+
+  if (Private->TrustComputingDevices != 0) {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: Security Security Command PPI will be produced.\n",
+      __FUNCTION__
+      ));
+    Private->StorageSecurityPpi.Revision           = EDKII_STORAGE_SECURITY_PPI_REVISION;
+    Private->StorageSecurityPpi.GetNumberofDevices = AhciStorageSecurityGetDeviceNo;
+    Private->StorageSecurityPpi.GetDevicePath      = AhciStorageSecurityGetDevicePath;
+    Private->StorageSecurityPpi.ReceiveData        = AhciStorageSecurityReceiveData;
+    Private->StorageSecurityPpi.SendData           = AhciStorageSecuritySendData;
+    CopyMem (
+      &Private->StorageSecurityPpiList,
+      &mAhciStorageSecurityPpiListTemplate,
+      sizeof (EFI_PEI_PPI_DESCRIPTOR)
+      );
+    Private->StorageSecurityPpiList.Ppi = &Private->StorageSecurityPpi;
+    PeiServicesInstallPpi (&Private->StorageSecurityPpiList);
   }
 
+  CopyMem (
+    &Private->EndOfPeiNotifyList,
+    &mAhciEndOfPeiNotifyListTemplate,
+    sizeof (EFI_PEI_NOTIFY_DESCRIPTOR)
+    );
+  PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList);
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Initialize AHCI controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI instance.
+
+  @param[in] AhciHcPpi  Pointer to the AHCI Host Controller PPI instance.
+
+  @retval EFI_SUCCESS   PPI successfully installed.
+**/
+EFI_STATUS
+AtaAhciInitPrivateDataFromHostControllerPpi (
+  IN EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi
+  )
+{
+  UINT8                     Controller;
+  UINTN                     MmioBase;
+  UINTN                     DevicePathLength;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+  EFI_STATUS                Status;
+
   Controller = 0;
   MmioBase   = 0;
   while (TRUE) {
@@ -193,65 +373,7 @@ AtaAhciPeimEntry (
       return Status;
     }
 
-    //
-    // Check validity of the device path of the ATA AHCI controller.
-    //
-    Status = AhciIsHcDevicePathValid (DevicePath, DevicePathLength);
-    if (EFI_ERROR (Status)) {
-      DEBUG ((
-        DEBUG_ERROR,
-        "%a: The device path is invalid for Controller %d.\n",
-        __FUNCTION__,
-        Controller
-        ));
-      Controller++;
-      continue;
-    }
-
-    //
-    // For S3 resume performance consideration, not all ports on an ATA AHCI
-    // controller will be enumerated/initialized. The driver consumes the
-    // content within S3StorageDeviceInitList LockBox to get the ports that
-    // will be enumerated/initialized during S3 resume.
-    //
-    if (BootMode == BOOT_ON_S3_RESUME) {
-      NumberOfPorts = AhciS3GetEumeratePorts (DevicePath, DevicePathLength, &PortBitMap);
-      if (NumberOfPorts == 0) {
-        //
-        // No ports need to be enumerated for this controller.
-        //
-        Controller++;
-        continue;
-      }
-    } else {
-      PortBitMap = MAX_UINT32;
-    }
-
-    //
-    // Memory allocation for controller private data.
-    //
-    Private = AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA));
-    if (Private == NULL) {
-      DEBUG ((
-        DEBUG_ERROR,
-        "%a: Fail to allocate private data for Controller %d.\n",
-        __FUNCTION__,
-        Controller
-        ));
-      return EFI_OUT_OF_RESOURCES;
-    }
-
-    //
-    // Initialize controller private data.
-    //
-    Private->Signature        = AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE;
-    Private->MmioBase         = MmioBase;
-    Private->DevicePathLength = DevicePathLength;
-    Private->DevicePath       = DevicePath;
-    Private->PortBitMap       = PortBitMap;
-    InitializeListHead (&Private->DeviceList);
-
-    Status = AhciModeInitialization (Private);
+    Status = AtaAhciInitPrivateData (MmioBase, DevicePath, DevicePathLength);
     if (EFI_ERROR (Status)) {
       DEBUG ((
         DEBUG_ERROR,
@@ -260,86 +382,229 @@ AtaAhciPeimEntry (
         Controller,
         Status
         ));
-      Controller++;
-      continue;
-    }
-
-    Private->AtaPassThruMode.Attributes = EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |
-                                          EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;
-    Private->AtaPassThruMode.IoAlign      = sizeof (UINTN);
-    Private->AtaPassThruPpi.Revision      = EDKII_PEI_ATA_PASS_THRU_PPI_REVISION;
-    Private->AtaPassThruPpi.Mode          = &Private->AtaPassThruMode;
-    Private->AtaPassThruPpi.PassThru      = AhciAtaPassThruPassThru;
-    Private->AtaPassThruPpi.GetNextPort   = AhciAtaPassThruGetNextPort;
-    Private->AtaPassThruPpi.GetNextDevice = AhciAtaPassThruGetNextDevice;
-    Private->AtaPassThruPpi.GetDevicePath = AhciAtaPassThruGetDevicePath;
-    CopyMem (
-      &Private->AtaPassThruPpiList,
-      &mAhciAtaPassThruPpiListTemplate,
-      sizeof (EFI_PEI_PPI_DESCRIPTOR)
-      );
-    Private->AtaPassThruPpiList.Ppi = &Private->AtaPassThruPpi;
-    PeiServicesInstallPpi (&Private->AtaPassThruPpiList);
-
-    Private->BlkIoPpi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo;
-    Private->BlkIoPpi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo;
-    Private->BlkIoPpi.ReadBlocks              = AhciBlockIoReadBlocks;
-    CopyMem (
-      &Private->BlkIoPpiList,
-      &mAhciBlkIoPpiListTemplate,
-      sizeof (EFI_PEI_PPI_DESCRIPTOR)
-      );
-    Private->BlkIoPpiList.Ppi = &Private->BlkIoPpi;
-    PeiServicesInstallPpi (&Private->BlkIoPpiList);
-
-    Private->BlkIo2Ppi.Revision                = EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION;
-    Private->BlkIo2Ppi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo2;
-    Private->BlkIo2Ppi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo2;
-    Private->BlkIo2Ppi.ReadBlocks              = AhciBlockIoReadBlocks2;
-    CopyMem (
-      &Private->BlkIo2PpiList,
-      &mAhciBlkIo2PpiListTemplate,
-      sizeof (EFI_PEI_PPI_DESCRIPTOR)
-      );
-    Private->BlkIo2PpiList.Ppi = &Private->BlkIo2Ppi;
-    PeiServicesInstallPpi (&Private->BlkIo2PpiList);
-
-    if (Private->TrustComputingDevices != 0) {
+    } else {
       DEBUG ((
         DEBUG_INFO,
-        "%a: Security Security Command PPI will be produced for Controller %d.\n",
+        "%a: Controller %d has been successfully initialized.\n",
         __FUNCTION__,
         Controller
         ));
-      Private->StorageSecurityPpi.Revision           = EDKII_STORAGE_SECURITY_PPI_REVISION;
-      Private->StorageSecurityPpi.GetNumberofDevices = AhciStorageSecurityGetDeviceNo;
-      Private->StorageSecurityPpi.GetDevicePath      = AhciStorageSecurityGetDevicePath;
-      Private->StorageSecurityPpi.ReceiveData        = AhciStorageSecurityReceiveData;
-      Private->StorageSecurityPpi.SendData           = AhciStorageSecuritySendData;
-      CopyMem (
-        &Private->StorageSecurityPpiList,
-        &mAhciStorageSecurityPpiListTemplate,
-        sizeof (EFI_PEI_PPI_DESCRIPTOR)
-        );
-      Private->StorageSecurityPpiList.Ppi = &Private->StorageSecurityPpi;
-      PeiServicesInstallPpi (&Private->StorageSecurityPpiList);
     }
 
-    CopyMem (
-      &Private->EndOfPeiNotifyList,
-      &mAhciEndOfPeiNotifyListTemplate,
-      sizeof (EFI_PEI_NOTIFY_DESCRIPTOR)
-      );
-    PeiServicesNotifyPpi (&Private->EndOfPeiNotifyList);
-
-    DEBUG ((
-      DEBUG_INFO,
-      "%a: Controller %d has been successfully initialized.\n",
-      __FUNCTION__,
-      Controller
-      ));
     Controller++;
   }
 
   return EFI_SUCCESS;
 }
+
+/**
+  Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation.
+
+  @param[in] PeiServices         Pointer to PEI Services Table.
+  @param[in] NotifyDescriptor    Pointer to the descriptor for the Notification
+                                 event that caused this function to execute.
+  @param[in] Ppi                 Pointer to the PPI data associated with this function.
+
+  @retval EFI_SUCCESS            The function completes successfully
+  @retval Others                 Cannot initialize AHCI controller for given device
+**/
+EFI_STATUS
+EFIAPI
+AtaAhciHostControllerPpiInstallationCallback (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID                       *Ppi
+  )
+{
+  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;
+
+  if (Ppi == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  AhciHcPpi = (EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *)Ppi;
+
+  return AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi);
+}
+
+/**
+  Initialize AHCI controller from fiven PCI_DEVICE_PPI.
+
+  @param[in] PciDevice  Pointer to the PCI Device PPI instance.
+
+  @retval EFI_SUCCESS      The function completes successfully
+  @retval Others           Cannot initialize AHCI controller for given device
+**/
+EFI_STATUS
+AtaAhciInitPrivateDataFromPciDevice (
+  EDKII_PCI_DEVICE_PPI  *PciDevice
+  )
+{
+  EFI_STATUS                Status;
+  PCI_TYPE00                PciData;
+  UINTN                     MmioBase;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+  UINTN                     DevicePathLength;
+
+  //
+  // Now further check the PCI header: Base Class (offset 0x0B) and
+  // Sub Class (offset 0x0A). This controller should be an SATA controller
+  //
+  Status = PciDevice->PciIo.Pci.Read (
+                                  &PciDevice->PciIo,
+                                  EfiPciIoWidthUint8,
+                                  PCI_CLASSCODE_OFFSET,
+                                  sizeof (PciData.Hdr.ClassCode),
+                                  PciData.Hdr.ClassCode
+                                  );
+  if (EFI_ERROR (Status)) {
+    return EFI_UNSUPPORTED;
+  }
+
+  if (!IS_PCI_IDE (&PciData) && !IS_PCI_SATADPA (&PciData)) {
+    return EFI_UNSUPPORTED;
+  }
+
+  Status = PciDevice->PciIo.Attributes (
+                              &PciDevice->PciIo,
+                              EfiPciIoAttributeOperationSet,
+                              EFI_PCI_DEVICE_ENABLE,
+                              NULL
+                              );
+  if (EFI_ERROR (Status)) {
+    return EFI_UNSUPPORTED;
+  }
+
+  Status = PciDevice->PciIo.Pci.Read (
+                                  &PciDevice->PciIo,
+                                  EfiPciIoWidthUint32,
+                                  0x24,
+                                  sizeof (UINTN),
+                                  &MmioBase
+                                  );
+  if (EFI_ERROR (Status)) {
+    return EFI_UNSUPPORTED;
+  }
+
+  DevicePathLength = GetDevicePathSize (PciDevice->DevicePath);
+  DevicePath       = PciDevice->DevicePath;
+
+  Status = AtaAhciInitPrivateData (MmioBase, DevicePath, DevicePathLength);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: Failed to init controller, with Status - %r\n",
+      __FUNCTION__,
+      Status
+      ));
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Callback for EDKII_PCI_DEVICE_PPI installation.
+
+  @param[in] PeiServices         Pointer to PEI Services Table.
+  @param[in] NotifyDescriptor    Pointer to the descriptor for the Notification
+                                 event that caused this function to execute.
+  @param[in] Ppi                 Pointer to the PPI data associated with this function.
+
+  @retval EFI_SUCCESS            The function completes successfully
+  @retval Others                 Cannot initialize AHCI controller for given device
+**/
+EFI_STATUS
+EFIAPI
+AtaAhciPciDevicePpiInstallationCallback (
+  IN EFI_PEI_SERVICES           **PeiServices,
+  IN EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,
+  IN VOID                       *Ppi
+  )
+{
+  EDKII_PCI_DEVICE_PPI  *PciDevice;
+
+  PciDevice = (EDKII_PCI_DEVICE_PPI *)Ppi;
+
+  return AtaAhciInitPrivateDataFromPciDevice (PciDevice);
+}
+
+/**
+  Initialize AHCI controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI instances found in the system.
+
+  @retval  EFI_SUCCESS    The function completes successfully
+**/
+EFI_STATUS
+AtaAhciInitPrivateDataFromPciDevices (
+  VOID
+  )
+{
+  EFI_STATUS            Status;
+  UINTN                 ControllerIndex;
+  EDKII_PCI_DEVICE_PPI  *PciDevice;
+
+  Status = EFI_SUCCESS;
+  for (ControllerIndex = 0; Status != EFI_NOT_FOUND; ControllerIndex++) {
+    Status = PeiServicesLocatePpi (
+               &gEdkiiPeiPciDevicePpiGuid,
+               ControllerIndex,
+               NULL,
+               (VOID **)&PciDevice
+               );
+    if (EFI_ERROR (Status)) {
+      continue;
+    }
+
+    AtaAhciInitPrivateDataFromPciDevice (PciDevice);
+
+    //
+    // Override the status to continue the for loop
+    //
+    Status = EFI_SUCCESS;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Entry point of the PEIM.
+
+  @param[in] FileHandle     Handle of the file being invoked.
+  @param[in] PeiServices    Describes the list of possible PEI Services.
+
+  @retval EFI_SUCCESS    PPI successfully installed.
+
+**/
+EFI_STATUS
+EFIAPI
+AtaAhciPeimEntry (
+  IN EFI_PEI_FILE_HANDLE     FileHandle,
+  IN CONST EFI_PEI_SERVICES  **PeiServices
+  )
+{
+  EFI_STATUS                          Status;
+  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;
+
+  DEBUG ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__));
+
+  PeiServicesNotifyPpi (&mAtaAhciHostControllerNotify);
+
+  PeiServicesNotifyPpi (&mPciDevicePpiNotify);
+
+  AtaAhciInitPrivateDataFromPciDevices ();
+
+  //
+  // Locate the ATA AHCI host controller PPI.
+  //
+  Status = PeiServicesLocatePpi (
+             &gEdkiiPeiAtaAhciHostControllerPpiGuid,
+             0,
+             NULL,
+             (VOID **)&AhciHcPpi
+             );
+  if (!EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi to initialize private data.\n", __FUNCTION__));
+    Status = AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi);
+  }
+
+  return Status;
+}
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
index 81f8743d40d8..cf0955929dde 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
+++ b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
@@ -38,50 +38,6 @@ EFI_DEVICE_PATH_PROTOCOL  mAhciEndDevicePathNodeTemplate = {
   }
 };
 
-/**
-  Returns the 16-bit Length field of a device path node.
-
-  Returns the 16-bit Length field of the device path node specified by Node.
-  Node is not required to be aligned on a 16-bit boundary, so it is recommended
-  that a function such as ReadUnaligned16() be used to extract the contents of
-  the Length field.
-
-  If Node is NULL, then ASSERT().
-
-  @param  Node      A pointer to a device path node data structure.
-
-  @return The 16-bit Length field of the device path node specified by Node.
-
-**/
-UINTN
-DevicePathNodeLength (
-  IN CONST VOID  *Node
-  )
-{
-  ASSERT (Node != NULL);
-  return ReadUnaligned16 ((UINT16 *)&((EFI_DEVICE_PATH_PROTOCOL *)(Node))->Length[0]);
-}
-
-/**
-  Returns a pointer to the next node in a device path.
-
-  If Node is NULL, then ASSERT().
-
-  @param  Node    A pointer to a device path node data structure.
-
-  @return a pointer to the device path node that follows the device path node
-  specified by Node.
-
-**/
-EFI_DEVICE_PATH_PROTOCOL *
-NextDevicePathNode (
-  IN CONST VOID  *Node
-  )
-{
-  ASSERT (Node != NULL);
-  return (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)(Node) + DevicePathNodeLength (Node));
-}
-
 /**
   Get the size of the current device path instance.
 
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
index 43ad4639bd77..7332c4051a0c 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
@@ -29,6 +29,7 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/IoLib.h>
 #include <Library/TimerLib.h>
+#include <Library/DevicePathLib.h>
 
 //
 // Structure forward declarations
@@ -631,22 +632,6 @@ TrustTransferAtaDevice (
   OUT    UINTN                     *TransferLengthOut
   );
 
-/**
-  Returns a pointer to the next node in a device path.
-
-  If Node is NULL, then ASSERT().
-
-  @param  Node    A pointer to a device path node data structure.
-
-  @return a pointer to the device path node that follows the device path node
-  specified by Node.
-
-**/
-EFI_DEVICE_PATH_PROTOCOL *
-NextDevicePathNode (
-  IN CONST VOID  *Node
-  );
-
 /**
   Get the size of the current device path instance.
 
diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
index 912ff7a8ba4f..788660b33299 100644
--- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
+++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
@@ -50,11 +50,13 @@ [LibraryClasses]
   TimerLib
   LockBoxLib
   PeimEntryPoint
+  DevicePathLib
 
 [Ppis]
   gEdkiiPeiAtaAhciHostControllerPpiGuid          ## CONSUMES
   gEdkiiIoMmuPpiGuid                             ## CONSUMES
   gEfiEndOfPeiSignalPpiGuid                      ## CONSUMES
+  gEdkiiPeiPciDevicePpiGuid                      ## CONSUMES
   gEdkiiPeiAtaPassThruPpiGuid                    ## SOMETIMES_PRODUCES
   gEfiPeiVirtualBlockIoPpiGuid                   ## SOMETIMES_PRODUCES
   gEfiPeiVirtualBlockIo2PpiGuid                  ## SOMETIMES_PRODUCES
@@ -65,8 +67,7 @@ [Guids]
 
 [Depex]
   gEfiPeiMemoryDiscoveredPpiGuid AND
-  gEfiPeiMasterBootModePpiGuid AND
-  gEdkiiPeiAtaAhciHostControllerPpiGuid
+  gEfiPeiMasterBootModePpiGuid
 
 [UserExtensions.TianoCore."ExtraFiles"]
   AhciPeiExtra.uni
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 90a0a7ec4a7c..45a8ec84ad69 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -117,6 +117,7 @@ [LibraryClasses.common.PEIM]
   MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
   ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
   LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
+  DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibBase.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
-- 
2.27.0.windows.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device
  2022-07-27 12:27 ` [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Maciej Czajkowski
@ 2022-07-28  3:29   ` Wu, Hao A
  2022-08-01 17:00     ` Maciej Czajkowski
  0 siblings, 1 reply; 6+ messages in thread
From: Wu, Hao A @ 2022-07-28  3:29 UTC (permalink / raw)
  To: Czajkowski, Maciej, devel@edk2.groups.io; +Cc: Ni, Ray, Gao, Liming

1. In the AHCI PEIM entrypoint, after registering the PPI callbacks for:
* gEdkiiPeiAtaAhciHostControllerPpiGuid and
* gEdkiiPeiPciDevicePpiGuid
My take is that there is no need to locate above PPI instances already installed within system (codes in entry point that come after the callback registration).
The current implementation of PeiServicesNotifyPpi (details can be referred at MdeModulePkg\Core\Pei\Ppi\Ppi.c - InternalPeiNotifyPpi) will invoke the callback functions for all the matched PPI instances already installed.
Could you help to double confirm on this in your unit test? Sorry for missing this on the V1 patch.

2. Please help to move the function declaration for:
AtaAhciHostControllerPpiInstallationCallback()
AtaAhciPciDevicePpiInstallationCallback()
from AhciPei.c to AhciPei.h to keep consistency.

3. The function description comments for:
AtaAhciHostControllerPpiInstallationCallback()
AtaAhciPciDevicePpiInstallationCallback()
do not exactly match between their declaration and definition.

4. During the enabling of the HC in function AtaAhciInitPrivateDataFromPciDevice():
  Status = PciDevice->PciIo.Attributes (
                              &PciDevice->PciIo,
                              EfiPciIoAttributeOperationSet,
                              EFI_PCI_DEVICE_ENABLE,
                              NULL
                              );
Could you help to update the flow to align with AtaAtapiPassThru DXE counterpart when enabling the controller?
MdeModulePkg\Bus\Ata\AtaAtapiPassThru\AtaAtapiPassThru.c - AtaAtapiPassThruStart():
  Status = PciIo->Attributes (
                    PciIo,
                    EfiPciIoAttributeOperationSupported,
                    0,
                    &EnabledPciAttributes
                    );
  if (!EFI_ERROR (Status)) {
    EnabledPciAttributes &= (UINT64)EFI_PCI_DEVICE_ENABLE;
    Status                = PciIo->Attributes (
                                     PciIo,
                                     EfiPciIoAttributeOperationEnable,
                                     EnabledPciAttributes,
                                     NULL
                                     );
  }

5. DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi to initialize private data.\n", __FUNCTION__));
-> DEBUG ((DEBUG_INFO, "%a: Using AtaAhciHostControllerPpi to initialize private data.\n", __FUNCTION__));

6. I think for AtaAhciInitPrivateDataFromPciDevices(), the below code snippet:
    //
    // Override the status to continue the for loop
    //
    Status = EFI_SUCCESS;
is no longer needed. Could you help to check?


Best Regards,
Hao Wu

> -----Original Message-----
> From: Czajkowski, Maciej <maciej.czajkowski@intel.com>
> Sent: Wednesday, July 27, 2022 8:28 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>
> Subject: [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to
> manage AHCI device
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3907
> 
> This change modifies AhciPei library to allow usage both
> EDKII_PCI_DEVICE_PPI and EDKII_PEI_ATA_AHCI_HOST_CONTROLLER_PPI
> to manage ATA HDD working under AHCI mode.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
> ---
> 
> Notes:
>     v2 changes:
>     - added missing function descriptions
>     - moved controller initialization from PCI_DEVICE_PPI to seperate funciton
>     in order to reduce code duplication
>     - added DevicePathLib BASE instance to the MdeModulePkg.dec to allow
> PEIMs to consume it
> 
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c    | 585 ++++++++++++++------
>  MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c |  44 --
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h    |  17 +-
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf  |   5 +-
>  MdeModulePkg/MdeModulePkg.dsc             |   1 +
>  5 files changed, 430 insertions(+), 222 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> index 208b7e9a3606..8ad98dc76bc1 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> @@ -9,6 +9,47 @@
>  **/  #include "AhciPei.h"+#include <Ppi/PciDevice.h>+#include
> <Library/DevicePathLib.h>+#include <IndustryStandard/Pci.h>++/**+
> Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation.++
> @param[in] PeiServices         Pointer to PEI Services Table.+  @param[in]
> NotifyDescriptor    Pointer to the descriptor for the Notification+
> event that caused this function to execute.+  @param[in] Ppi                 Pointer
> to the PPI data associated with this function.++  @retval EFI_SUCCESS    The
> function completes
> successfully++**/+EFI_STATUS+EFIAPI+AtaAhciHostControllerPpiInstallation
> Callback (+  IN EFI_PEI_SERVICES           **PeiServices,+  IN
> EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,+  IN VOID
> *Ppi+  );++/**+  Callback for EDKII_PCI_DEVICE_PPI installation.++
> @param[in] PeiServices         Pointer to PEI Services Table.+  @param[in]
> NotifyDescriptor    Pointer to the descriptor for the Notification+
> event that caused this function to execute.+  @param[in] Ppi                 Pointer
> to the PPI data associated with this function.++  @retval EFI_SUCCESS    The
> function completes
> successfully++**/+EFI_STATUS+EFIAPI+AtaAhciPciDevicePpiInstallationCallb
> ack (+  IN EFI_PEI_SERVICES           **PeiServices,+  IN
> EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,+  IN VOID
> *Ppi+  );  EFI_PEI_PPI_DESCRIPTOR  mAhciAtaPassThruPpiListTemplate =
> {   (EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),@@ -40,6 +81,18 @@
> EFI_PEI_NOTIFY_DESCRIPTOR  mAhciEndOfPeiNotifyListTemplate = {
>    AhciPeimEndOfPei }; +EFI_PEI_NOTIFY_DESCRIPTOR
> mAtaAhciHostControllerNotify = {+
> (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),+
> &gEdkiiPeiAtaAhciHostControllerPpiGuid,+
> AtaAhciHostControllerPpiInstallationCallback+};++EFI_PEI_NOTIFY_DESCRIPT
> OR  mPciDevicePpiNotify = {+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK
> | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),+
> &gEdkiiPeiPciDevicePpiGuid,+  AtaAhciPciDevicePpiInstallationCallback+};+
> /**   Free the DMA resources allocated by an ATA AHCI controller. @@ -
> 111,33 +164,30 @@ AhciPeimEndOfPei (
>  }  /**-  Entry point of the PEIM.+  Initialize and install PrivateData PPIs. -
> @param[in] FileHandle     Handle of the file being invoked.-  @param[in]
> PeiServices    Describes the list of possible PEI Services.--  @retval
> EFI_SUCCESS    PPI successfully installed.+  @param[in] MmioBase
> MMIO base address of specific AHCI controller+  @param[in] DevicePath
> A pointer to the EFI_DEVICE_PATH_PROTOCOL+                                 structure.+
> @param[in] DevicePathLength    Length of the device path. +  @retval
> EFI_SUCCESS  AHCI controller initialized and PPIs installed+  @retval others
> Failed to initialize AHCI controller **/ EFI_STATUS-EFIAPI-AtaAhciPeimEntry
> (-  IN EFI_PEI_FILE_HANDLE     FileHandle,-  IN CONST EFI_PEI_SERVICES
> **PeiServices+AtaAhciInitPrivateData (+  IN UINTN                     MmioBase,+
> IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath,+  IN UINTN
> DevicePathLength   ) {-  EFI_STATUS                          Status;-  EFI_BOOT_MODE
> BootMode;-  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;-  UINT8
> Controller;-  UINTN                               MmioBase;-  UINTN
> DevicePathLength;-  EFI_DEVICE_PATH_PROTOCOL            *DevicePath;-
> UINT32                              PortBitMap;-  PEI_AHCI_CONTROLLER_PRIVATE_DATA
> *Private;-  UINT8                               NumberOfPorts;+  EFI_STATUS
> Status;+  UINT32                            PortBitMap;+  UINT8
> NumberOfPorts;+  PEI_AHCI_CONTROLLER_PRIVATE_DATA  *Private;+
> EFI_BOOT_MODE                     BootMode; -  DEBUG ((DEBUG_INFO, "%a:
> Enters.\n", __FUNCTION__));+  DEBUG ((DEBUG_INFO, "Initializing private
> data for ATA\n"));    //   // Get the current boot mode.@@ -149,19 +199,149
> @@ AtaAhciPeimEntry (
>    }    //-  // Locate the ATA AHCI host controller PPI.-  //-  Status =
> PeiServicesLocatePpi (-             &gEdkiiPeiAtaAhciHostControllerPpiGuid,-
> 0,-             NULL,-             (VOID **)&AhciHcPpi-             );+  // Check validity of
> the device path of the ATA AHCI controller.+  //+  Status =
> AhciIsHcDevicePathValid (DevicePath, DevicePathLength);+  if (EFI_ERROR
> (Status)) {+    DEBUG ((+      DEBUG_ERROR,+      "%a: The device path is
> invalid.\n",+      __FUNCTION__+      ));+    return Status;+  }++  //+  // For S3
> resume performance consideration, not all ports on an ATA AHCI+  //
> controller will be enumerated/initialized. The driver consumes the+  //
> content within S3StorageDeviceInitList LockBox to get the ports that+  // will
> be enumerated/initialized during S3 resume.+  //+  if (BootMode ==
> BOOT_ON_S3_RESUME) {+    NumberOfPorts = AhciS3GetEumeratePorts
> (DevicePath, DevicePathLength, &PortBitMap);+    if (NumberOfPorts == 0)
> {+      return EFI_SUCCESS;+    }+  } else {+    PortBitMap = MAX_UINT32;+  }++
> //+  // Memory allocation for controller private data.+  //+  Private =
> AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA));+  if
> (Private == NULL) {+    DEBUG ((+      DEBUG_ERROR,+      "%a: Fail to allocate
> private data.\n",+      __FUNCTION__+      ));+    return
> EFI_OUT_OF_RESOURCES;+  }++  //+  // Initialize controller private data.+  //+
> Private->Signature        =
> AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE;+  Private->MmioBase
> = MmioBase;+  Private->DevicePathLength = DevicePathLength;+  Private-
> >DevicePath       = DevicePath;+  Private->PortBitMap       = PortBitMap;+
> InitializeListHead (&Private->DeviceList);++  Status = AhciModeInitialization
> (Private);   if (EFI_ERROR (Status)) {-    DEBUG ((DEBUG_ERROR, "%a: Failed to
> locate AtaAhciHostControllerPpi.\n", __FUNCTION__));-    return
> EFI_UNSUPPORTED;+    return Status;+  }++  Private-
> >AtaPassThruMode.Attributes =
> EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |+
> EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;+  Private-
> >AtaPassThruMode.IoAlign      = sizeof (UINTN);+  Private-
> >AtaPassThruPpi.Revision      = EDKII_PEI_ATA_PASS_THRU_PPI_REVISION;+
> Private->AtaPassThruPpi.Mode          = &Private->AtaPassThruMode;+
> Private->AtaPassThruPpi.PassThru      = AhciAtaPassThruPassThru;+  Private-
> >AtaPassThruPpi.GetNextPort   = AhciAtaPassThruGetNextPort;+  Private-
> >AtaPassThruPpi.GetNextDevice = AhciAtaPassThruGetNextDevice;+
> Private->AtaPassThruPpi.GetDevicePath = AhciAtaPassThruGetDevicePath;+
> CopyMem (+    &Private->AtaPassThruPpiList,+
> &mAhciAtaPassThruPpiListTemplate,+    sizeof
> (EFI_PEI_PPI_DESCRIPTOR)+    );+  Private->AtaPassThruPpiList.Ppi =
> &Private->AtaPassThruPpi;+  PeiServicesInstallPpi (&Private-
> >AtaPassThruPpiList);++  Private->BlkIoPpi.GetNumberOfBlockDevices =
> AhciBlockIoGetDeviceNo;+  Private->BlkIoPpi.GetBlockDeviceMediaInfo =
> AhciBlockIoGetMediaInfo;+  Private->BlkIoPpi.ReadBlocks              =
> AhciBlockIoReadBlocks;+  CopyMem (+    &Private->BlkIoPpiList,+
> &mAhciBlkIoPpiListTemplate,+    sizeof (EFI_PEI_PPI_DESCRIPTOR)+    );+
> Private->BlkIoPpiList.Ppi = &Private->BlkIoPpi;+  PeiServicesInstallPpi
> (&Private->BlkIoPpiList);++  Private->BlkIo2Ppi.Revision                =
> EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION;+  Private-
> >BlkIo2Ppi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo2;+
> Private->BlkIo2Ppi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo2;+
> Private->BlkIo2Ppi.ReadBlocks              = AhciBlockIoReadBlocks2;+  CopyMem
> (+    &Private->BlkIo2PpiList,+    &mAhciBlkIo2PpiListTemplate,+    sizeof
> (EFI_PEI_PPI_DESCRIPTOR)+    );+  Private->BlkIo2PpiList.Ppi = &Private-
> >BlkIo2Ppi;+  PeiServicesInstallPpi (&Private->BlkIo2PpiList);++  if (Private-
> >TrustComputingDevices != 0) {+    DEBUG ((+      DEBUG_INFO,+      "%a:
> Security Security Command PPI will be produced.\n",+
> __FUNCTION__+      ));+    Private->StorageSecurityPpi.Revision           =
> EDKII_STORAGE_SECURITY_PPI_REVISION;+    Private-
> >StorageSecurityPpi.GetNumberofDevices =
> AhciStorageSecurityGetDeviceNo;+    Private-
> >StorageSecurityPpi.GetDevicePath      =
> AhciStorageSecurityGetDevicePath;+    Private-
> >StorageSecurityPpi.ReceiveData        = AhciStorageSecurityReceiveData;+
> Private->StorageSecurityPpi.SendData           =
> AhciStorageSecuritySendData;+    CopyMem (+      &Private-
> >StorageSecurityPpiList,+      &mAhciStorageSecurityPpiListTemplate,+
> sizeof (EFI_PEI_PPI_DESCRIPTOR)+      );+    Private-
> >StorageSecurityPpiList.Ppi = &Private->StorageSecurityPpi;+
> PeiServicesInstallPpi (&Private->StorageSecurityPpiList);   } +  CopyMem (+
> &Private->EndOfPeiNotifyList,+    &mAhciEndOfPeiNotifyListTemplate,+
> sizeof (EFI_PEI_NOTIFY_DESCRIPTOR)+    );+  PeiServicesNotifyPpi (&Private-
> >EndOfPeiNotifyList);++  return EFI_SUCCESS;+}++/**+  Initialize AHCI
> controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI instance.++
> @param[in] AhciHcPpi  Pointer to the AHCI Host Controller PPI instance.++
> @retval EFI_SUCCESS   PPI successfully
> installed.+**/+EFI_STATUS+AtaAhciInitPrivateDataFromHostControllerPpi (+
> IN EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi+  )+{+  UINT8
> Controller;+  UINTN                     MmioBase;+  UINTN
> DevicePathLength;+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;+
> EFI_STATUS                Status;+   Controller = 0;   MmioBase   = 0;   while (TRUE)
> {@@ -193,65 +373,7 @@ AtaAhciPeimEntry (
>        return Status;     } -    //-    // Check validity of the device path of the ATA
> AHCI controller.-    //-    Status = AhciIsHcDevicePathValid (DevicePath,
> DevicePathLength);-    if (EFI_ERROR (Status)) {-      DEBUG ((-
> DEBUG_ERROR,-        "%a: The device path is invalid for Controller %d.\n",-
> __FUNCTION__,-        Controller-        ));-      Controller++;-      continue;-    }--
> //-    // For S3 resume performance consideration, not all ports on an ATA
> AHCI-    // controller will be enumerated/initialized. The driver consumes the-
> // content within S3StorageDeviceInitList LockBox to get the ports that-    //
> will be enumerated/initialized during S3 resume.-    //-    if (BootMode ==
> BOOT_ON_S3_RESUME) {-      NumberOfPorts = AhciS3GetEumeratePorts
> (DevicePath, DevicePathLength, &PortBitMap);-      if (NumberOfPorts == 0)
> {-        //-        // No ports need to be enumerated for this controller.-        //-
> Controller++;-        continue;-      }-    } else {-      PortBitMap = MAX_UINT32;-
>     }--    //-    // Memory allocation for controller private data.-    //-    Private =
> AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA));-    if
> (Private == NULL) {-      DEBUG ((-        DEBUG_ERROR,-        "%a: Fail to allocate
> private data for Controller %d.\n",-        __FUNCTION__,-        Controller-
>         ));-      return EFI_OUT_OF_RESOURCES;-    }--    //-    // Initialize controller
> private data.-    //-    Private->Signature        =
> AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE;-    Private->MmioBase
> = MmioBase;-    Private->DevicePathLength = DevicePathLength;-    Private-
> >DevicePath       = DevicePath;-    Private->PortBitMap       = PortBitMap;-
> InitializeListHead (&Private->DeviceList);--    Status = AhciModeInitialization
> (Private);+    Status = AtaAhciInitPrivateData (MmioBase, DevicePath,
> DevicePathLength);     if (EFI_ERROR (Status)) {       DEBUG
> ((         DEBUG_ERROR,@@ -260,86 +382,229 @@ AtaAhciPeimEntry (
>          Controller,         Status         ));-      Controller++;-      continue;-    }--
> Private->AtaPassThruMode.Attributes =
> EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |-
> EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;-    Private-
> >AtaPassThruMode.IoAlign      = sizeof (UINTN);-    Private-
> >AtaPassThruPpi.Revision      = EDKII_PEI_ATA_PASS_THRU_PPI_REVISION;-
> Private->AtaPassThruPpi.Mode          = &Private->AtaPassThruMode;-
> Private->AtaPassThruPpi.PassThru      = AhciAtaPassThruPassThru;-    Private-
> >AtaPassThruPpi.GetNextPort   = AhciAtaPassThruGetNextPort;-    Private-
> >AtaPassThruPpi.GetNextDevice = AhciAtaPassThruGetNextDevice;-
> Private->AtaPassThruPpi.GetDevicePath = AhciAtaPassThruGetDevicePath;-
> CopyMem (-      &Private->AtaPassThruPpiList,-
> &mAhciAtaPassThruPpiListTemplate,-      sizeof (EFI_PEI_PPI_DESCRIPTOR)-
>       );-    Private->AtaPassThruPpiList.Ppi = &Private->AtaPassThruPpi;-
> PeiServicesInstallPpi (&Private->AtaPassThruPpiList);--    Private-
> >BlkIoPpi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo;-    Private-
> >BlkIoPpi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo;-    Private-
> >BlkIoPpi.ReadBlocks              = AhciBlockIoReadBlocks;-    CopyMem (-
> &Private->BlkIoPpiList,-      &mAhciBlkIoPpiListTemplate,-      sizeof
> (EFI_PEI_PPI_DESCRIPTOR)-      );-    Private->BlkIoPpiList.Ppi = &Private-
> >BlkIoPpi;-    PeiServicesInstallPpi (&Private->BlkIoPpiList);--    Private-
> >BlkIo2Ppi.Revision                =
> EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION;-    Private-
> >BlkIo2Ppi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo2;-
> Private->BlkIo2Ppi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo2;-
> Private->BlkIo2Ppi.ReadBlocks              = AhciBlockIoReadBlocks2;-    CopyMem
> (-      &Private->BlkIo2PpiList,-      &mAhciBlkIo2PpiListTemplate,-      sizeof
> (EFI_PEI_PPI_DESCRIPTOR)-      );-    Private->BlkIo2PpiList.Ppi = &Private-
> >BlkIo2Ppi;-    PeiServicesInstallPpi (&Private->BlkIo2PpiList);--    if (Private-
> >TrustComputingDevices != 0) {+    } else {       DEBUG ((         DEBUG_INFO,-
> "%a: Security Security Command PPI will be produced for Controller %d.\n",+
> "%a: Controller %d has been successfully initialized.\n",         __FUNCTION__,
> Controller         ));-      Private->StorageSecurityPpi.Revision           =
> EDKII_STORAGE_SECURITY_PPI_REVISION;-      Private-
> >StorageSecurityPpi.GetNumberofDevices =
> AhciStorageSecurityGetDeviceNo;-      Private-
> >StorageSecurityPpi.GetDevicePath      =
> AhciStorageSecurityGetDevicePath;-      Private-
> >StorageSecurityPpi.ReceiveData        = AhciStorageSecurityReceiveData;-
> Private->StorageSecurityPpi.SendData           = AhciStorageSecuritySendData;-
> CopyMem (-        &Private->StorageSecurityPpiList,-
> &mAhciStorageSecurityPpiListTemplate,-        sizeof
> (EFI_PEI_PPI_DESCRIPTOR)-        );-      Private->StorageSecurityPpiList.Ppi =
> &Private->StorageSecurityPpi;-      PeiServicesInstallPpi (&Private-
> >StorageSecurityPpiList);     } -    CopyMem (-      &Private-
> >EndOfPeiNotifyList,-      &mAhciEndOfPeiNotifyListTemplate,-      sizeof
> (EFI_PEI_NOTIFY_DESCRIPTOR)-      );-    PeiServicesNotifyPpi (&Private-
> >EndOfPeiNotifyList);--    DEBUG ((-      DEBUG_INFO,-      "%a: Controller %d
> has been successfully initialized.\n",-      __FUNCTION__,-      Controller-      ));
> Controller++;   }    return EFI_SUCCESS; }++/**+  Callback for
> EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation.++  @param[in]
> PeiServices         Pointer to PEI Services Table.+  @param[in] NotifyDescriptor
> Pointer to the descriptor for the Notification+                                 event that
> caused this function to execute.+  @param[in] Ppi                 Pointer to the PPI
> data associated with this function.++  @retval EFI_SUCCESS            The
> function completes successfully+  @retval Others                 Cannot initialize
> AHCI controller for given
> device+**/+EFI_STATUS+EFIAPI+AtaAhciHostControllerPpiInstallationCallbac
> k (+  IN EFI_PEI_SERVICES           **PeiServices,+  IN
> EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,+  IN VOID
> *Ppi+  )+{+  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;++  if (Ppi
> == NULL) {+    return EFI_INVALID_PARAMETER;+  }++  AhciHcPpi =
> (EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *)Ppi;++  return
> AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi);+}++/**+  Initialize
> AHCI controller from fiven PCI_DEVICE_PPI.++  @param[in] PciDevice
> Pointer to the PCI Device PPI instance.++  @retval EFI_SUCCESS      The
> function completes successfully+  @retval Others           Cannot initialize AHCI
> controller for given
> device+**/+EFI_STATUS+AtaAhciInitPrivateDataFromPciDevice (+
> EDKII_PCI_DEVICE_PPI  *PciDevice+  )+{+  EFI_STATUS                Status;+
> PCI_TYPE00                PciData;+  UINTN                     MmioBase;+
> EFI_DEVICE_PATH_PROTOCOL  *DevicePath;+  UINTN
> DevicePathLength;++  //+  // Now further check the PCI header: Base Class
> (offset 0x0B) and+  // Sub Class (offset 0x0A). This controller should be an
> SATA controller+  //+  Status = PciDevice->PciIo.Pci.Read (+
> &PciDevice->PciIo,+                                  EfiPciIoWidthUint8,+
> PCI_CLASSCODE_OFFSET,+                                  sizeof (PciData.Hdr.ClassCode),+
> PciData.Hdr.ClassCode+                                  );+  if (EFI_ERROR (Status)) {+
> return EFI_UNSUPPORTED;+  }++  if (!IS_PCI_IDE (&PciData)
> && !IS_PCI_SATADPA (&PciData)) {+    return EFI_UNSUPPORTED;+  }++
> Status = PciDevice->PciIo.Attributes (+                              &PciDevice->PciIo,+
> EfiPciIoAttributeOperationSet,+                              EFI_PCI_DEVICE_ENABLE,+
> NULL+                              );+  if (EFI_ERROR (Status)) {+    return
> EFI_UNSUPPORTED;+  }++  Status = PciDevice->PciIo.Pci.Read (+
> &PciDevice->PciIo,+                                  EfiPciIoWidthUint32,+
> 0x24,+                                  sizeof (UINTN),+
> &MmioBase+                                  );+  if (EFI_ERROR (Status)) {+    return
> EFI_UNSUPPORTED;+  }++  DevicePathLength = GetDevicePathSize
> (PciDevice->DevicePath);+  DevicePath       = PciDevice->DevicePath;++
> Status = AtaAhciInitPrivateData (MmioBase, DevicePath,
> DevicePathLength);+  if (EFI_ERROR (Status)) {+    DEBUG ((+
> DEBUG_INFO,+      "%a: Failed to init controller, with Status - %r\n",+
> __FUNCTION__,+      Status+      ));+  }++  return EFI_SUCCESS;+}++/**+
> Callback for EDKII_PCI_DEVICE_PPI installation.++  @param[in] PeiServices
> Pointer to PEI Services Table.+  @param[in] NotifyDescriptor    Pointer to the
> descriptor for the Notification+                                 event that caused this
> function to execute.+  @param[in] Ppi                 Pointer to the PPI data
> associated with this function.++  @retval EFI_SUCCESS            The function
> completes successfully+  @retval Others                 Cannot initialize AHCI
> controller for given
> device+**/+EFI_STATUS+EFIAPI+AtaAhciPciDevicePpiInstallationCallback (+
> IN EFI_PEI_SERVICES           **PeiServices,+  IN EFI_PEI_NOTIFY_DESCRIPTOR
> *NotifyDescriptor,+  IN VOID                       *Ppi+  )+{+  EDKII_PCI_DEVICE_PPI
> *PciDevice;++  PciDevice = (EDKII_PCI_DEVICE_PPI *)Ppi;++  return
> AtaAhciInitPrivateDataFromPciDevice (PciDevice);+}++/**+  Initialize AHCI
> controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI instances found in
> the system.++  @retval  EFI_SUCCESS    The function completes
> successfully+**/+EFI_STATUS+AtaAhciInitPrivateDataFromPciDevices (+
> VOID+  )+{+  EFI_STATUS            Status;+  UINTN                 ControllerIndex;+
> EDKII_PCI_DEVICE_PPI  *PciDevice;++  Status = EFI_SUCCESS;+  for
> (ControllerIndex = 0; Status != EFI_NOT_FOUND; ControllerIndex++) {+
> Status = PeiServicesLocatePpi (+               &gEdkiiPeiPciDevicePpiGuid,+
> ControllerIndex,+               NULL,+               (VOID **)&PciDevice+               );+    if
> (EFI_ERROR (Status)) {+      continue;+    }++
> AtaAhciInitPrivateDataFromPciDevice (PciDevice);++    //+    // Override the
> status to continue the for loop+    //+    Status = EFI_SUCCESS;+  }++  return
> EFI_SUCCESS;+}++/**+  Entry point of the PEIM.++  @param[in] FileHandle
> Handle of the file being invoked.+  @param[in] PeiServices    Describes the
> list of possible PEI Services.++  @retval EFI_SUCCESS    PPI successfully
> installed.++**/+EFI_STATUS+EFIAPI+AtaAhciPeimEntry (+  IN
> EFI_PEI_FILE_HANDLE     FileHandle,+  IN CONST EFI_PEI_SERVICES
> **PeiServices+  )+{+  EFI_STATUS                          Status;+
> EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;++  DEBUG
> ((DEBUG_INFO, "%a: Enters.\n", __FUNCTION__));++  PeiServicesNotifyPpi
> (&mAtaAhciHostControllerNotify);++  PeiServicesNotifyPpi
> (&mPciDevicePpiNotify);++  AtaAhciInitPrivateDataFromPciDevices ();++  //+
> // Locate the ATA AHCI host controller PPI.+  //+  Status =
> PeiServicesLocatePpi (+             &gEdkiiPeiAtaAhciHostControllerPpiGuid,+
> 0,+             NULL,+             (VOID **)&AhciHcPpi+             );+  if (!EFI_ERROR
> (Status)) {+    DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi
> to initialize private data.\n", __FUNCTION__));+    Status =
> AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi);+  }++  return
> Status;+}diff --git a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> index 81f8743d40d8..cf0955929dde 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> @@ -38,50 +38,6 @@ EFI_DEVICE_PATH_PROTOCOL
> mAhciEndDevicePathNodeTemplate = {
>    } }; -/**-  Returns the 16-bit Length field of a device path node.--  Returns
> the 16-bit Length field of the device path node specified by Node.-  Node is
> not required to be aligned on a 16-bit boundary, so it is recommended-  that
> a function such as ReadUnaligned16() be used to extract the contents of-
> the Length field.--  If Node is NULL, then ASSERT().--  @param  Node      A
> pointer to a device path node data structure.--  @return The 16-bit Length
> field of the device path node specified by Node.--**/-UINTN-
> DevicePathNodeLength (-  IN CONST VOID  *Node-  )-{-  ASSERT (Node !=
> NULL);-  return ReadUnaligned16 ((UINT16
> *)&((EFI_DEVICE_PATH_PROTOCOL *)(Node))->Length[0]);-}--/**-  Returns
> a pointer to the next node in a device path.--  If Node is NULL, then
> ASSERT().--  @param  Node    A pointer to a device path node data structure.-
> -  @return a pointer to the device path node that follows the device path
> node-  specified by Node.--**/-EFI_DEVICE_PATH_PROTOCOL *-
> NextDevicePathNode (-  IN CONST VOID  *Node-  )-{-  ASSERT (Node !=
> NULL);-  return (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)(Node) +
> DevicePathNodeLength (Node));-}- /**   Get the size of the current device
> path instance. diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 43ad4639bd77..7332c4051a0c 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -29,6 +29,7 @@
>  #include <Library/BaseMemoryLib.h> #include <Library/IoLib.h> #include
> <Library/TimerLib.h>+#include <Library/DevicePathLib.h>  // // Structure
> forward declarations@@ -631,22 +632,6 @@ TrustTransferAtaDevice (
>    OUT    UINTN                     *TransferLengthOut   ); -/**-  Returns a pointer to
> the next node in a device path.--  If Node is NULL, then ASSERT().--  @param
> Node    A pointer to a device path node data structure.--  @return a pointer
> to the device path node that follows the device path node-  specified by
> Node.--**/-EFI_DEVICE_PATH_PROTOCOL *-NextDevicePathNode (-  IN
> CONST VOID  *Node-  );- /**   Get the size of the current device path
> instance. diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> index 912ff7a8ba4f..788660b33299 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> @@ -50,11 +50,13 @@ [LibraryClasses]
>    TimerLib   LockBoxLib   PeimEntryPoint+  DevicePathLib  [Ppis]
> gEdkiiPeiAtaAhciHostControllerPpiGuid          ## CONSUMES
> gEdkiiIoMmuPpiGuid                             ## CONSUMES
> gEfiEndOfPeiSignalPpiGuid                      ## CONSUMES+
> gEdkiiPeiPciDevicePpiGuid                      ## CONSUMES
> gEdkiiPeiAtaPassThruPpiGuid                    ## SOMETIMES_PRODUCES
> gEfiPeiVirtualBlockIoPpiGuid                   ## SOMETIMES_PRODUCES
> gEfiPeiVirtualBlockIo2PpiGuid                  ## SOMETIMES_PRODUCES@@ -65,8
> +67,7 @@ [Guids]
>   [Depex]   gEfiPeiMemoryDiscoveredPpiGuid AND-
> gEfiPeiMasterBootModePpiGuid AND-
> gEdkiiPeiAtaAhciHostControllerPpiGuid+  gEfiPeiMasterBootModePpiGuid
> [UserExtensions.TianoCore."ExtraFiles"]   AhciPeiExtra.unidiff --git
> a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc
> index 90a0a7ec4a7c..45a8ec84ad69 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -117,6 +117,7 @@ [LibraryClasses.common.PEIM]
> 
> MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemory
> AllocationLib.inf
> ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiEx
> tractGuidedSectionLib.inf
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
> +
> DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibBase.i
> nf  [LibraryClasses.common.DXE_CORE]
> HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf--
> 2.27.0.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition
  2022-07-27 12:27 ` [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition Maciej Czajkowski
@ 2022-07-28  3:30   ` Wu, Hao A
  0 siblings, 0 replies; 6+ messages in thread
From: Wu, Hao A @ 2022-07-28  3:30 UTC (permalink / raw)
  To: Ni, Ray, Gao, Liming, Czajkowski, Maciej, devel@edk2.groups.io

As previously mentioned in https://edk2.groups.io/g/devel/message/90345, the PPI interface looks good from the device controllers perspective:
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Sorry Ray and Liming, do you have additional comments for this newly introduced EDKII_PCI_DEVICE_PPI?

Best Regards,
Hao Wu

> -----Original Message-----
> From: Czajkowski, Maciej <maciej.czajkowski@intel.com>
> Sent: Wednesday, July 27, 2022 8:28 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao,
> Liming <gaoliming@byosoft.com.cn>
> Subject: [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI
> definition
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3907
> 
> This commit introduces EDKII_PCI_DEVICE_PPI. The purpose of this PPI is to
> provide a way of accessing PCI devices to drvice drivers such as
> NvmExpressPei or AhciPei.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
> Acked-by: Hao A Wu <hao.a.wu@intel.com>
> ---
> 
> Notes:
>     v2 changes:
>     - added acked-by tag
>     - minor change - modified header guard to follow coding style
> 
>  MdeModulePkg/Include/Ppi/PciDevice.h | 32 ++++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec        |  3 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Ppi/PciDevice.h
> b/MdeModulePkg/Include/Ppi/PciDevice.h
> new file mode 100644
> index 000000000000..6750ae6ce394
> --- /dev/null
> +++ b/MdeModulePkg/Include/Ppi/PciDevice.h
> @@ -0,0 +1,32 @@
> +/** @file++  Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>+
> SPDX-License-Identifier: BSD-2-Clause-Patent++**/++#ifndef
> EDKII_PCI_DEVICE_PPI_H_+#define EDKII_PCI_DEVICE_PPI_H_++#include
> <Protocol/PciIo.h>+#include <Protocol/DevicePath.h>++///+/// Global ID for
> the EDKII_PCI_DEVICE_PPI_GUID.+///+#define
> EDKII_PCI_DEVICE_PPI_GUID \+  { \+    0x1597ab4f, 0xd542, 0x4efe, { 0x9a,
> 0xf7, 0xb2, 0x44, 0xec, 0x54, 0x4c, 0x0b } \+  }++///+/// PCI Device PPI
> structure.+///+typedef struct {+  EFI_PCI_IO_PROTOCOL         PciIo;+
> EFI_DEVICE_PATH_PROTOCOL    *DevicePath;+}
> EDKII_PCI_DEVICE_PPI;++extern EFI_GUID
> gEdkiiPeiPciDevicePpiGuid;++#endif // EDKII_PCI_DEVICE_PPI_H_diff --git
> a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> index 2bcb9f9453af..7d989108324a 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -509,6 +509,9 @@ [Ppis]
>    ## Include/Ppi/NvmExpressPassThru.h
> gEdkiiPeiNvmExpressPassThruPpiGuid    = { 0x6af31b2c, 0x3be, 0x46c1, { 0xb1,
> 0x2d, 0xea, 0x4a, 0x36, 0xdf, 0xa7, 0x4c } } +  ## Include/Ppi/PciDevice.h+
> gEdkiiPeiPciDevicePpiGuid                 = { 0x1597ab4f, 0xd542, 0x4efe, { 0x9a,
> 0xf7, 0xb2, 0x44, 0xec, 0x54, 0x4c, 0x0b } }+   ## Include/Ppi/CapsuleOnDisk.h
> gEdkiiPeiCapsuleOnDiskPpiGuid             = { 0x71a9ea61, 0x5a35, 0x4a5d, { 0xac,
> 0xef, 0x9c, 0xf8, 0x6d, 0x6d, 0x67, 0xe0 } }
> gEdkiiPeiBootInCapsuleOnDiskModePpiGuid   = { 0xb08a11e4, 0xe2b7,
> 0x4b75, { 0xb5, 0x15, 0xaf, 0x61, 0x6, 0x68, 0xbf, 0xd1  } }--
> 2.27.0.windows.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device
  2022-07-28  3:29   ` Wu, Hao A
@ 2022-08-01 17:00     ` Maciej Czajkowski
  0 siblings, 0 replies; 6+ messages in thread
From: Maciej Czajkowski @ 2022-08-01 17:00 UTC (permalink / raw)
  To: Wu, Hao A, devel@edk2.groups.io; +Cc: Ni, Ray, Gao, Liming

Hello,

1. I checked that and you are right. It will invoke callback functions when PPIs are already installed. I removed unnecessary function calls in v3 patch.
2. Moved in v3.
3. Corrected in v3.
4. Aligned in v3.
5. Whole debug removed because of point 1.
6. Code removed - point 1.

Thanks,
Maciej 

-----Original Message-----
From: Wu, Hao A <hao.a.wu@intel.com> 
Sent: czwartek, 28 lipca 2022 05:30
To: Czajkowski, Maciej <maciej.czajkowski@intel.com>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>
Subject: RE: [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device

1. In the AHCI PEIM entrypoint, after registering the PPI callbacks for:
* gEdkiiPeiAtaAhciHostControllerPpiGuid and
* gEdkiiPeiPciDevicePpiGuid
My take is that there is no need to locate above PPI instances already installed within system (codes in entry point that come after the callback registration).
The current implementation of PeiServicesNotifyPpi (details can be referred at MdeModulePkg\Core\Pei\Ppi\Ppi.c - InternalPeiNotifyPpi) will invoke the callback functions for all the matched PPI instances already installed.
Could you help to double confirm on this in your unit test? Sorry for missing this on the V1 patch.

2. Please help to move the function declaration for:
AtaAhciHostControllerPpiInstallationCallback()
AtaAhciPciDevicePpiInstallationCallback()
from AhciPei.c to AhciPei.h to keep consistency.

3. The function description comments for:
AtaAhciHostControllerPpiInstallationCallback()
AtaAhciPciDevicePpiInstallationCallback()
do not exactly match between their declaration and definition.

4. During the enabling of the HC in function AtaAhciInitPrivateDataFromPciDevice():
  Status = PciDevice->PciIo.Attributes (
                              &PciDevice->PciIo,
                              EfiPciIoAttributeOperationSet,
                              EFI_PCI_DEVICE_ENABLE,
                              NULL
                              );
Could you help to update the flow to align with AtaAtapiPassThru DXE counterpart when enabling the controller?
MdeModulePkg\Bus\Ata\AtaAtapiPassThru\AtaAtapiPassThru.c - AtaAtapiPassThruStart():
  Status = PciIo->Attributes (
                    PciIo,
                    EfiPciIoAttributeOperationSupported,
                    0,
                    &EnabledPciAttributes
                    );
  if (!EFI_ERROR (Status)) {
    EnabledPciAttributes &= (UINT64)EFI_PCI_DEVICE_ENABLE;
    Status                = PciIo->Attributes (
                                     PciIo,
                                     EfiPciIoAttributeOperationEnable,
                                     EnabledPciAttributes,
                                     NULL
                                     );
  }

5. DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi to initialize private data.\n", __FUNCTION__));
-> DEBUG ((DEBUG_INFO, "%a: Using AtaAhciHostControllerPpi to initialize 
-> private data.\n", __FUNCTION__));

6. I think for AtaAhciInitPrivateDataFromPciDevices(), the below code snippet:
    //
    // Override the status to continue the for loop
    //
    Status = EFI_SUCCESS;
is no longer needed. Could you help to check?


Best Regards,
Hao Wu

> -----Original Message-----
> From: Czajkowski, Maciej <maciej.czajkowski@intel.com>
> Sent: Wednesday, July 27, 2022 8:28 PM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Gao, 
> Liming <gaoliming@byosoft.com.cn>
> Subject: [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to 
> manage AHCI device
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3907
> 
> This change modifies AhciPei library to allow usage both 
> EDKII_PCI_DEVICE_PPI and EDKII_PEI_ATA_AHCI_HOST_CONTROLLER_PPI
> to manage ATA HDD working under AHCI mode.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
> ---
> 
> Notes:
>     v2 changes:
>     - added missing function descriptions
>     - moved controller initialization from PCI_DEVICE_PPI to seperate funciton
>     in order to reduce code duplication
>     - added DevicePathLib BASE instance to the MdeModulePkg.dec to 
> allow PEIMs to consume it
> 
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c    | 585 ++++++++++++++------
>  MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c |  44 --
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h    |  17 +-
>  MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf  |   5 +-
>  MdeModulePkg/MdeModulePkg.dsc             |   1 +
>  5 files changed, 430 insertions(+), 222 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> index 208b7e9a3606..8ad98dc76bc1 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> @@ -9,6 +9,47 @@
>  **/  #include "AhciPei.h"+#include <Ppi/PciDevice.h>+#include 
> <Library/DevicePathLib.h>+#include <IndustryStandard/Pci.h>++/**+ 
> Callback for EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation.++
> @param[in] PeiServices         Pointer to PEI Services Table.+  @param[in]
> NotifyDescriptor    Pointer to the descriptor for the Notification+
> event that caused this function to execute.+  @param[in] Ppi                 Pointer
> to the PPI data associated with this function.++  @retval EFI_SUCCESS    The
> function completes
> successfully++**/+EFI_STATUS+EFIAPI+AtaAhciHostControllerPpiInstallati
> successfully++on
> Callback (+  IN EFI_PEI_SERVICES           **PeiServices,+  IN
> EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,+  IN VOID *Ppi+  
> );++/**+  Callback for EDKII_PCI_DEVICE_PPI installation.++
> @param[in] PeiServices         Pointer to PEI Services Table.+  @param[in]
> NotifyDescriptor    Pointer to the descriptor for the Notification+
> event that caused this function to execute.+  @param[in] Ppi                 Pointer
> to the PPI data associated with this function.++  @retval EFI_SUCCESS    The
> function completes
> successfully++**/+EFI_STATUS+EFIAPI+AtaAhciPciDevicePpiInstallationCal
> successfully++lb
> ack (+  IN EFI_PEI_SERVICES           **PeiServices,+  IN
> EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,+  IN VOID *Ppi+  );  
> EFI_PEI_PPI_DESCRIPTOR  mAhciAtaPassThruPpiListTemplate =
> {   (EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),@@ -40,6 +81,18 @@ 
> EFI_PEI_NOTIFY_DESCRIPTOR  mAhciEndOfPeiNotifyListTemplate = {
>    AhciPeimEndOfPei }; +EFI_PEI_NOTIFY_DESCRIPTOR 
> mAtaAhciHostControllerNotify = {+ 
> (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK | 
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),+
> &gEdkiiPeiAtaAhciHostControllerPpiGuid,+
> AtaAhciHostControllerPpiInstallationCallback+};++EFI_PEI_NOTIFY_DESCRI
> AtaAhciHostControllerPpiInstallationCallback+PT
> OR  mPciDevicePpiNotify = {+  (EFI_PEI_PPI_DESCRIPTOR_NOTIFY_CALLBACK
> | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),+
> &gEdkiiPeiPciDevicePpiGuid,+  AtaAhciPciDevicePpiInstallationCallback+};+
> /**   Free the DMA resources allocated by an ATA AHCI controller. @@ -
> 111,33 +164,30 @@ AhciPeimEndOfPei (
>  }  /**-  Entry point of the PEIM.+  Initialize and install PrivateData PPIs. -
> @param[in] FileHandle     Handle of the file being invoked.-  @param[in]
> PeiServices    Describes the list of possible PEI Services.--  @retval
> EFI_SUCCESS    PPI successfully installed.+  @param[in] MmioBase
> MMIO base address of specific AHCI controller+  @param[in] DevicePath
> A pointer to the EFI_DEVICE_PATH_PROTOCOL+                                 structure.+
> @param[in] DevicePathLength    Length of the device path. +  @retval
> EFI_SUCCESS  AHCI controller initialized and PPIs installed+  @retval 
> others Failed to initialize AHCI controller **/ EFI_STATUS-EFIAPI-AtaAhciPeimEntry
> (-  IN EFI_PEI_FILE_HANDLE     FileHandle,-  IN CONST EFI_PEI_SERVICES
> **PeiServices+AtaAhciInitPrivateData (+  IN UINTN                     MmioBase,+
> IN EFI_DEVICE_PATH_PROTOCOL  *DevicePath,+  IN UINTN
> DevicePathLength   ) {-  EFI_STATUS                          Status;-  EFI_BOOT_MODE
> BootMode;-  EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;-  UINT8
> Controller;-  UINTN                               MmioBase;-  UINTN
> DevicePathLength;-  EFI_DEVICE_PATH_PROTOCOL            *DevicePath;-
> UINT32                              PortBitMap;-  PEI_AHCI_CONTROLLER_PRIVATE_DATA
> *Private;-  UINT8                               NumberOfPorts;+  EFI_STATUS
> Status;+  UINT32                            PortBitMap;+  UINT8
> NumberOfPorts;+  PEI_AHCI_CONTROLLER_PRIVATE_DATA  *Private;+
> EFI_BOOT_MODE                     BootMode; -  DEBUG ((DEBUG_INFO, "%a:
> Enters.\n", __FUNCTION__));+  DEBUG ((DEBUG_INFO, "Initializing private
> data for ATA\n"));    //   // Get the current boot mode.@@ -149,19 +199,149
> @@ AtaAhciPeimEntry (
>    }    //-  // Locate the ATA AHCI host controller PPI.-  //-  Status =
> PeiServicesLocatePpi (-             &gEdkiiPeiAtaAhciHostControllerPpiGuid,-
> 0,-             NULL,-             (VOID **)&AhciHcPpi-             );+  // Check validity of
> the device path of the ATA AHCI controller.+  //+  Status = 
> AhciIsHcDevicePathValid (DevicePath, DevicePathLength);+  if (EFI_ERROR
> (Status)) {+    DEBUG ((+      DEBUG_ERROR,+      "%a: The device path is
> invalid.\n",+      __FUNCTION__+      ));+    return Status;+  }++  //+  // For S3
> resume performance consideration, not all ports on an ATA AHCI+  // 
> controller will be enumerated/initialized. The driver consumes the+  
> // content within S3StorageDeviceInitList LockBox to get the ports 
> that+  // will be enumerated/initialized during S3 resume.+  //+  if (BootMode ==
> BOOT_ON_S3_RESUME) {+    NumberOfPorts = AhciS3GetEumeratePorts
> (DevicePath, DevicePathLength, &PortBitMap);+    if (NumberOfPorts == 0)
> {+      return EFI_SUCCESS;+    }+  } else {+    PortBitMap = MAX_UINT32;+  }++
> //+  // Memory allocation for controller private data.+  //+  Private 
> = AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA));+  if
> (Private == NULL) {+    DEBUG ((+      DEBUG_ERROR,+      "%a: Fail to allocate
> private data.\n",+      __FUNCTION__+      ));+    return
> EFI_OUT_OF_RESOURCES;+  }++  //+  // Initialize controller private 
> data.+  //+
> Private->Signature        =
> AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE;+  Private->MmioBase = 
> MmioBase;+  Private->DevicePathLength = DevicePathLength;+  Private-
> >DevicePath       = DevicePath;+  Private->PortBitMap       = PortBitMap;+
> InitializeListHead (&Private->DeviceList);++  Status = AhciModeInitialization
> (Private);   if (EFI_ERROR (Status)) {-    DEBUG ((DEBUG_ERROR, "%a: Failed to
> locate AtaAhciHostControllerPpi.\n", __FUNCTION__));-    return
> EFI_UNSUPPORTED;+    return Status;+  }++  Private-
> >AtaPassThruMode.Attributes =
> EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |+ 
> EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;+  Private-
> >AtaPassThruMode.IoAlign      = sizeof (UINTN);+  Private-
> >AtaPassThruPpi.Revision      = EDKII_PEI_ATA_PASS_THRU_PPI_REVISION;+
> Private->AtaPassThruPpi.Mode          = &Private->AtaPassThruMode;+
> Private->AtaPassThruPpi.PassThru      = AhciAtaPassThruPassThru;+  Private-
> >AtaPassThruPpi.GetNextPort   = AhciAtaPassThruGetNextPort;+  Private-
> >AtaPassThruPpi.GetNextDevice = AhciAtaPassThruGetNextDevice;+
> Private->AtaPassThruPpi.GetDevicePath = AhciAtaPassThruGetDevicePath;+
> CopyMem (+    &Private->AtaPassThruPpiList,+
> &mAhciAtaPassThruPpiListTemplate,+    sizeof
> (EFI_PEI_PPI_DESCRIPTOR)+    );+  Private->AtaPassThruPpiList.Ppi =
> &Private->AtaPassThruPpi;+  PeiServicesInstallPpi (&Private-
> >AtaPassThruPpiList);++  Private->BlkIoPpi.GetNumberOfBlockDevices =
> AhciBlockIoGetDeviceNo;+  Private->BlkIoPpi.GetBlockDeviceMediaInfo =
> AhciBlockIoGetMediaInfo;+  Private->BlkIoPpi.ReadBlocks              =
> AhciBlockIoReadBlocks;+  CopyMem (+    &Private->BlkIoPpiList,+
> &mAhciBlkIoPpiListTemplate,+    sizeof (EFI_PEI_PPI_DESCRIPTOR)+    );+
> Private->BlkIoPpiList.Ppi = &Private->BlkIoPpi;+  
> Private->PeiServicesInstallPpi
> (&Private->BlkIoPpiList);++  Private->BlkIo2Ppi.Revision                =
> EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION;+  Private-
> >BlkIo2Ppi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo2;+
> Private->BlkIo2Ppi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo2;+
> Private->BlkIo2Ppi.ReadBlocks              = AhciBlockIoReadBlocks2;+  CopyMem
> (+    &Private->BlkIo2PpiList,+    &mAhciBlkIo2PpiListTemplate,+    sizeof
> (EFI_PEI_PPI_DESCRIPTOR)+    );+  Private->BlkIo2PpiList.Ppi = &Private-
> >BlkIo2Ppi;+  PeiServicesInstallPpi (&Private->BlkIo2PpiList);++  if (Private-
> >TrustComputingDevices != 0) {+    DEBUG ((+      DEBUG_INFO,+      "%a:
> Security Security Command PPI will be produced.\n",+
> __FUNCTION__+      ));+    Private->StorageSecurityPpi.Revision           =
> EDKII_STORAGE_SECURITY_PPI_REVISION;+    Private-
> >StorageSecurityPpi.GetNumberofDevices =
> AhciStorageSecurityGetDeviceNo;+    Private-
> >StorageSecurityPpi.GetDevicePath      =
> AhciStorageSecurityGetDevicePath;+    Private-
> >StorageSecurityPpi.ReceiveData        = AhciStorageSecurityReceiveData;+
> Private->StorageSecurityPpi.SendData           =
> AhciStorageSecuritySendData;+    CopyMem (+      &Private-
> >StorageSecurityPpiList,+      &mAhciStorageSecurityPpiListTemplate,+
> sizeof (EFI_PEI_PPI_DESCRIPTOR)+      );+    Private-
> >StorageSecurityPpiList.Ppi = &Private->StorageSecurityPpi;+
> PeiServicesInstallPpi (&Private->StorageSecurityPpiList);   } +  CopyMem (+
> &Private->EndOfPeiNotifyList,+    &mAhciEndOfPeiNotifyListTemplate,+
> sizeof (EFI_PEI_NOTIFY_DESCRIPTOR)+    );+  PeiServicesNotifyPpi (&Private-
> >EndOfPeiNotifyList);++  return EFI_SUCCESS;+}++/**+  Initialize AHCI
> controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI instance.++ 
> @param[in] AhciHcPpi  Pointer to the AHCI Host Controller PPI instance.++
> @retval EFI_SUCCESS   PPI successfully
> installed.+**/+EFI_STATUS+AtaAhciInitPrivateDataFromHostControllerPpi 
> (+ IN EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi+  )+{+  UINT8
> Controller;+  UINTN                     MmioBase;+  UINTN
> DevicePathLength;+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;+
> EFI_STATUS                Status;+   Controller = 0;   MmioBase   = 0;   while (TRUE)
> {@@ -193,65 +373,7 @@ AtaAhciPeimEntry (
>        return Status;     } -    //-    // Check validity of the device path of the ATA
> AHCI controller.-    //-    Status = AhciIsHcDevicePathValid (DevicePath,
> DevicePathLength);-    if (EFI_ERROR (Status)) {-      DEBUG ((-
> DEBUG_ERROR,-        "%a: The device path is invalid for Controller %d.\n",-
> __FUNCTION__,-        Controller-        ));-      Controller++;-      continue;-    }--
> //-    // For S3 resume performance consideration, not all ports on an ATA
> AHCI-    // controller will be enumerated/initialized. The driver consumes the-
> // content within S3StorageDeviceInitList LockBox to get the ports that-    //
> will be enumerated/initialized during S3 resume.-    //-    if (BootMode ==
> BOOT_ON_S3_RESUME) {-      NumberOfPorts = AhciS3GetEumeratePorts
> (DevicePath, DevicePathLength, &PortBitMap);-      if (NumberOfPorts == 0)
> {-        //-        // No ports need to be enumerated for this controller.-        //-
> Controller++;-        continue;-      }-    } else {-      PortBitMap = MAX_UINT32;-
>     }--    //-    // Memory allocation for controller private data.-    //-    Private =
> AllocateZeroPool (sizeof (PEI_AHCI_CONTROLLER_PRIVATE_DATA));-    if
> (Private == NULL) {-      DEBUG ((-        DEBUG_ERROR,-        "%a: Fail to allocate
> private data for Controller %d.\n",-        __FUNCTION__,-        Controller-
>         ));-      return EFI_OUT_OF_RESOURCES;-    }--    //-    // Initialize controller
> private data.-    //-    Private->Signature        =
> AHCI_PEI_CONTROLLER_PRIVATE_DATA_SIGNATURE;-    Private->MmioBase
> = MmioBase;-    Private->DevicePathLength = DevicePathLength;-    Private-
> >DevicePath       = DevicePath;-    Private->PortBitMap       = PortBitMap;-
> InitializeListHead (&Private->DeviceList);--    Status = AhciModeInitialization
> (Private);+    Status = AtaAhciInitPrivateData (MmioBase, DevicePath,
> DevicePathLength);     if (EFI_ERROR (Status)) {       DEBUG
> ((         DEBUG_ERROR,@@ -260,86 +382,229 @@ AtaAhciPeimEntry (
>          Controller,         Status         ));-      Controller++;-      continue;-    }--
> Private->AtaPassThruMode.Attributes =
> EFI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL |-
> EFI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL;-    Private-
> >AtaPassThruMode.IoAlign      = sizeof (UINTN);-    Private-
> >AtaPassThruPpi.Revision      = EDKII_PEI_ATA_PASS_THRU_PPI_REVISION;-
> Private->AtaPassThruPpi.Mode          = &Private->AtaPassThruMode;-
> Private->AtaPassThruPpi.PassThru      = AhciAtaPassThruPassThru;-    Private-
> >AtaPassThruPpi.GetNextPort   = AhciAtaPassThruGetNextPort;-    Private-
> >AtaPassThruPpi.GetNextDevice = AhciAtaPassThruGetNextDevice;-
> Private->AtaPassThruPpi.GetDevicePath = AhciAtaPassThruGetDevicePath;-
> CopyMem (-      &Private->AtaPassThruPpiList,-
> &mAhciAtaPassThruPpiListTemplate,-      sizeof (EFI_PEI_PPI_DESCRIPTOR)-
>       );-    Private->AtaPassThruPpiList.Ppi = &Private->AtaPassThruPpi;-
> PeiServicesInstallPpi (&Private->AtaPassThruPpiList);--    Private-
> >BlkIoPpi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo;-    Private-
> >BlkIoPpi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo;-    Private-
> >BlkIoPpi.ReadBlocks              = AhciBlockIoReadBlocks;-    CopyMem (-
> &Private->BlkIoPpiList,-      &mAhciBlkIoPpiListTemplate,-      sizeof
> (EFI_PEI_PPI_DESCRIPTOR)-      );-    Private->BlkIoPpiList.Ppi = &Private-
> >BlkIoPpi;-    PeiServicesInstallPpi (&Private->BlkIoPpiList);--    Private-
> >BlkIo2Ppi.Revision                =
> EFI_PEI_RECOVERY_BLOCK_IO2_PPI_REVISION;-    Private-
> >BlkIo2Ppi.GetNumberOfBlockDevices = AhciBlockIoGetDeviceNo2;-
> Private->BlkIo2Ppi.GetBlockDeviceMediaInfo = AhciBlockIoGetMediaInfo2;-
> Private->BlkIo2Ppi.ReadBlocks              = AhciBlockIoReadBlocks2;-    CopyMem
> (-      &Private->BlkIo2PpiList,-      &mAhciBlkIo2PpiListTemplate,-      sizeof
> (EFI_PEI_PPI_DESCRIPTOR)-      );-    Private->BlkIo2PpiList.Ppi = &Private-
> >BlkIo2Ppi;-    PeiServicesInstallPpi (&Private->BlkIo2PpiList);--    if (Private-
> >TrustComputingDevices != 0) {+    } else {       DEBUG ((         DEBUG_INFO,-
> "%a: Security Security Command PPI will be produced for Controller %d.\n",+
> "%a: Controller %d has been successfully initialized.\n",         __FUNCTION__,
> Controller         ));-      Private->StorageSecurityPpi.Revision           =
> EDKII_STORAGE_SECURITY_PPI_REVISION;-      Private-
> >StorageSecurityPpi.GetNumberofDevices =
> AhciStorageSecurityGetDeviceNo;-      Private-
> >StorageSecurityPpi.GetDevicePath      =
> AhciStorageSecurityGetDevicePath;-      Private-
> >StorageSecurityPpi.ReceiveData        = AhciStorageSecurityReceiveData;-
> Private->StorageSecurityPpi.SendData           = AhciStorageSecuritySendData;-
> CopyMem (-        &Private->StorageSecurityPpiList,-
> &mAhciStorageSecurityPpiListTemplate,-        sizeof
> (EFI_PEI_PPI_DESCRIPTOR)-        );-      Private->StorageSecurityPpiList.Ppi =
> &Private->StorageSecurityPpi;-      PeiServicesInstallPpi (&Private-
> >StorageSecurityPpiList);     } -    CopyMem (-      &Private-
> >EndOfPeiNotifyList,-      &mAhciEndOfPeiNotifyListTemplate,-      sizeof
> (EFI_PEI_NOTIFY_DESCRIPTOR)-      );-    PeiServicesNotifyPpi (&Private-
> >EndOfPeiNotifyList);--    DEBUG ((-      DEBUG_INFO,-      "%a: Controller %d
> has been successfully initialized.\n",-      __FUNCTION__,-      Controller-      ));
> Controller++;   }    return EFI_SUCCESS; }++/**+  Callback for
> EDKII_ATA_AHCI_HOST_CONTROLLER_PPI installation.++  @param[in]
> PeiServices         Pointer to PEI Services Table.+  @param[in] NotifyDescriptor
> Pointer to the descriptor for the Notification+                                 event that
> caused this function to execute.+  @param[in] Ppi                 Pointer to the PPI
> data associated with this function.++  @retval EFI_SUCCESS            The
> function completes successfully+  @retval Others                 Cannot initialize
> AHCI controller for given
> device+**/+EFI_STATUS+EFIAPI+AtaAhciHostControllerPpiInstallationCallb
> device+ac
> k (+  IN EFI_PEI_SERVICES           **PeiServices,+  IN
> EFI_PEI_NOTIFY_DESCRIPTOR  *NotifyDescriptor,+  IN VOID *Ppi+  )+{+  
> EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;++  if (Ppi
> == NULL) {+    return EFI_INVALID_PARAMETER;+  }++  AhciHcPpi =
> (EDKII_ATA_AHCI_HOST_CONTROLLER_PPI *)Ppi;++  return 
> AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi);+}++/**+  
> Initialize AHCI controller from fiven PCI_DEVICE_PPI.++  @param[in] PciDevice
> Pointer to the PCI Device PPI instance.++  @retval EFI_SUCCESS      The
> function completes successfully+  @retval Others           Cannot initialize AHCI
> controller for given
> device+**/+EFI_STATUS+AtaAhciInitPrivateDataFromPciDevice (+
> EDKII_PCI_DEVICE_PPI  *PciDevice+  )+{+  EFI_STATUS                Status;+
> PCI_TYPE00                PciData;+  UINTN                     MmioBase;+
> EFI_DEVICE_PATH_PROTOCOL  *DevicePath;+  UINTN DevicePathLength;++  
> //+  // Now further check the PCI header: Base Class (offset 0x0B) 
> and+  // Sub Class (offset 0x0A). This controller should be an SATA 
> controller+  //+  Status = PciDevice->PciIo.Pci.Read (+
> &PciDevice->PciIo,+                                  EfiPciIoWidthUint8,+
> PCI_CLASSCODE_OFFSET,+                                  sizeof (PciData.Hdr.ClassCode),+
> PciData.Hdr.ClassCode+                                  );+  if (EFI_ERROR (Status)) {+
> return EFI_UNSUPPORTED;+  }++  if (!IS_PCI_IDE (&PciData)
> && !IS_PCI_SATADPA (&PciData)) {+    return EFI_UNSUPPORTED;+  }++
> Status = PciDevice->PciIo.Attributes (+                              &PciDevice->PciIo,+
> EfiPciIoAttributeOperationSet,+                              EFI_PCI_DEVICE_ENABLE,+
> NULL+                              );+  if (EFI_ERROR (Status)) {+    return
> EFI_UNSUPPORTED;+  }++  Status = PciDevice->PciIo.Pci.Read (+
> &PciDevice->PciIo,+                                  EfiPciIoWidthUint32,+
> 0x24,+                                  sizeof (UINTN),+
> &MmioBase+                                  );+  if (EFI_ERROR (Status)) {+    return
> EFI_UNSUPPORTED;+  }++  DevicePathLength = GetDevicePathSize
> (PciDevice->DevicePath);+  DevicePath       = PciDevice->DevicePath;++
> Status = AtaAhciInitPrivateData (MmioBase, DevicePath,
> DevicePathLength);+  if (EFI_ERROR (Status)) {+    DEBUG ((+
> DEBUG_INFO,+      "%a: Failed to init controller, with Status - %r\n",+
> __FUNCTION__,+      Status+      ));+  }++  return EFI_SUCCESS;+}++/**+
> Callback for EDKII_PCI_DEVICE_PPI installation.++  @param[in] PeiServices
> Pointer to PEI Services Table.+  @param[in] NotifyDescriptor    Pointer to the
> descriptor for the Notification+                                 event that caused this
> function to execute.+  @param[in] Ppi                 Pointer to the PPI data
> associated with this function.++  @retval EFI_SUCCESS            The function
> completes successfully+  @retval Others                 Cannot initialize AHCI
> controller for given
> device+**/+EFI_STATUS+EFIAPI+AtaAhciPciDevicePpiInstallationCallback 
> device+(+
> IN EFI_PEI_SERVICES           **PeiServices,+  IN EFI_PEI_NOTIFY_DESCRIPTOR
> *NotifyDescriptor,+  IN VOID                       *Ppi+  )+{+  EDKII_PCI_DEVICE_PPI
> *PciDevice;++  PciDevice = (EDKII_PCI_DEVICE_PPI *)Ppi;++  return 
> AtaAhciInitPrivateDataFromPciDevice (PciDevice);+}++/**+  Initialize 
> AHCI controller from EDKII_ATA_AHCI_HOST_CONTROLLER_PPI instances found in
> the system.++  @retval  EFI_SUCCESS    The function completes
> successfully+**/+EFI_STATUS+AtaAhciInitPrivateDataFromPciDevices (+
> VOID+  )+{+  EFI_STATUS            Status;+  UINTN                 ControllerIndex;+
> EDKII_PCI_DEVICE_PPI  *PciDevice;++  Status = EFI_SUCCESS;+  for 
> (ControllerIndex = 0; Status != EFI_NOT_FOUND; ControllerIndex++) {+
> Status = PeiServicesLocatePpi (+               &gEdkiiPeiPciDevicePpiGuid,+
> ControllerIndex,+               NULL,+               (VOID **)&PciDevice+               );+    if
> (EFI_ERROR (Status)) {+      continue;+    }++
> AtaAhciInitPrivateDataFromPciDevice (PciDevice);++    //+    // Override the
> status to continue the for loop+    //+    Status = EFI_SUCCESS;+  }++  return
> EFI_SUCCESS;+}++/**+  Entry point of the PEIM.++  @param[in] FileHandle
> Handle of the file being invoked.+  @param[in] PeiServices    Describes the
> list of possible PEI Services.++  @retval EFI_SUCCESS    PPI successfully
> installed.++**/+EFI_STATUS+EFIAPI+AtaAhciPeimEntry (+  IN
> EFI_PEI_FILE_HANDLE     FileHandle,+  IN CONST EFI_PEI_SERVICES
> **PeiServices+  )+{+  EFI_STATUS                          Status;+
> EDKII_ATA_AHCI_HOST_CONTROLLER_PPI  *AhciHcPpi;++  DEBUG ((DEBUG_INFO, 
> "%a: Enters.\n", __FUNCTION__));++  PeiServicesNotifyPpi 
> (&mAtaAhciHostControllerNotify);++  PeiServicesNotifyPpi 
> (&mPciDevicePpiNotify);++  AtaAhciInitPrivateDataFromPciDevices ();++  
> //+ // Locate the ATA AHCI host controller PPI.+  //+  Status =
> PeiServicesLocatePpi (+             &gEdkiiPeiAtaAhciHostControllerPpiGuid,+
> 0,+             NULL,+             (VOID **)&AhciHcPpi+             );+  if (!EFI_ERROR
> (Status)) {+    DEBUG ((DEBUG_ERROR, "%a: Using AtaAhciHostControllerPpi
> to initialize private data.\n", __FUNCTION__));+    Status =
> AtaAhciInitPrivateDataFromHostControllerPpi (AhciHcPpi);+  }++  return 
> Status;+}diff --git a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> index 81f8743d40d8..cf0955929dde 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c
> @@ -38,50 +38,6 @@ EFI_DEVICE_PATH_PROTOCOL 
> mAhciEndDevicePathNodeTemplate = {
>    } }; -/**-  Returns the 16-bit Length field of a device path 
> node.--  Returns the 16-bit Length field of the device path node 
> specified by Node.-  Node is not required to be aligned on a 16-bit 
> boundary, so it is recommended-  that a function such as ReadUnaligned16() be used to extract the contents of-
> the Length field.--  If Node is NULL, then ASSERT().--  @param  Node      A
> pointer to a device path node data structure.--  @return The 16-bit 
> Length field of the device path node specified by Node.--**/-UINTN- 
> DevicePathNodeLength (-  IN CONST VOID  *Node-  )-{-  ASSERT (Node !=
> NULL);-  return ReadUnaligned16 ((UINT16 *)&((EFI_DEVICE_PATH_PROTOCOL 
> *)(Node))->Length[0]);-}--/**-  Returns a pointer to the next node in 
> a device path.--  If Node is NULL, then
> ASSERT().--  @param  Node    A pointer to a device path node data structure.-
> -  @return a pointer to the device path node that follows the device 
> path
> node-  specified by Node.--**/-EFI_DEVICE_PATH_PROTOCOL *- 
> NextDevicePathNode (-  IN CONST VOID  *Node-  )-{-  ASSERT (Node !=
> NULL);-  return (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)(Node) +
> DevicePathNodeLength (Node));-}- /**   Get the size of the current device
> path instance. diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> index 43ad4639bd77..7332c4051a0c 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.h
> @@ -29,6 +29,7 @@
>  #include <Library/BaseMemoryLib.h> #include <Library/IoLib.h> 
> #include <Library/TimerLib.h>+#include <Library/DevicePathLib.h>  // 
> // Structure forward declarations@@ -631,22 +632,6 @@ TrustTransferAtaDevice (
>    OUT    UINTN                     *TransferLengthOut   ); -/**-  Returns a pointer to
> the next node in a device path.--  If Node is NULL, then ASSERT().--  @param
> Node    A pointer to a device path node data structure.--  @return a pointer
> to the device path node that follows the device path node-  specified 
> by Node.--**/-EFI_DEVICE_PATH_PROTOCOL *-NextDevicePathNode (-  IN
> CONST VOID  *Node-  );- /**   Get the size of the current device path
> instance. diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> index 912ff7a8ba4f..788660b33299 100644
> --- a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> +++ b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf
> @@ -50,11 +50,13 @@ [LibraryClasses]
>    TimerLib   LockBoxLib   PeimEntryPoint+  DevicePathLib  [Ppis]
> gEdkiiPeiAtaAhciHostControllerPpiGuid          ## CONSUMES
> gEdkiiIoMmuPpiGuid                             ## CONSUMES
> gEfiEndOfPeiSignalPpiGuid                      ## CONSUMES+
> gEdkiiPeiPciDevicePpiGuid                      ## CONSUMES
> gEdkiiPeiAtaPassThruPpiGuid                    ## SOMETIMES_PRODUCES
> gEfiPeiVirtualBlockIoPpiGuid                   ## SOMETIMES_PRODUCES
> gEfiPeiVirtualBlockIo2PpiGuid                  ## SOMETIMES_PRODUCES@@ -65,8
> +67,7 @@ [Guids]
>   [Depex]   gEfiPeiMemoryDiscoveredPpiGuid AND-
> gEfiPeiMasterBootModePpiGuid AND-
> gEdkiiPeiAtaAhciHostControllerPpiGuid+  gEfiPeiMasterBootModePpiGuid
> [UserExtensions.TianoCore."ExtraFiles"]   AhciPeiExtra.unidiff --git
> a/MdeModulePkg/MdeModulePkg.dsc
> b/MdeModulePkg/MdeModulePkg.dsc
> index 90a0a7ec4a7c..45a8ec84ad69 100644
> --- a/MdeModulePkg/MdeModulePkg.dsc
> +++ b/MdeModulePkg/MdeModulePkg.dsc
> @@ -117,6 +117,7 @@ [LibraryClasses.common.PEIM]
> 
> MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemory
> AllocationLib.inf
> ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiE
> ExtractGuidedSectionLib|x
> tractGuidedSectionLib.inf
> LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
> +
> DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibBase.i
> nf  [LibraryClasses.common.DXE_CORE]
> HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf--
> 2.27.0.windows.1

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-08-01 17:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-27 12:27 [PATCH v2 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2 Maciej Czajkowski
2022-07-27 12:27 ` [PATCH v2 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition Maciej Czajkowski
2022-07-28  3:30   ` Wu, Hao A
2022-07-27 12:27 ` [PATCH v2 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Maciej Czajkowski
2022-07-28  3:29   ` Wu, Hao A
2022-08-01 17:00     ` Maciej Czajkowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox