From: "Wu, Hao A" <hao.a.wu@intel.com>
To: "Czajkowski, Maciej" <maciej.czajkowski@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Ni, Ray" <ray.ni@intel.com>
Subject: Re: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device
Date: Tue, 14 Jun 2022 01:26:24 +0000 [thread overview]
Message-ID: <DM6PR11MB4025126E9C809FBB47C1E0ABCAAA9@DM6PR11MB4025.namprd11.prod.outlook.com> (raw)
In-Reply-To: <BYAPR11MB30475618BE5FB060FAFF0C6EF8AB9@BYAPR11MB3047.namprd11.prod.outlook.com>
Thanks.
For 2 (DevicePathLib PEIM instance), please ensure it is done before merging this series.
For 3 (IOMMU codes in storage device PEIMs):
Yes, you are right that we still need to consider the EDKII_ATA_AHCI_HOST_CONTROLLER_PPI case.
As far as I can recall, this PPI was added for the support of OPAL/HDD Password S3 unlock feature. So my take is that the PPI producers are mainly Intel Client platforms.
I think in order to add a library (or directly use services in EDKII_PCI_DEVICE_PPI) to abstract the IOMMU related logics in AhciPei, both 2 conditions below should be met:
a) Retire the EDKII_ATA_AHCI_HOST_CONTROLLER_PPI
This should be no hard, as there are not many producers of this PPI.
b) A public reference PciBusPei module is needed in edk2 like PciBusDxe for DXE case
As I mentioned earlier, an enforcement for producers of EDKII_PCI_DEVICE_PPI to add IOMMU support is required.
If there is no reference module (like PciBusDxe for DXE), such enforcement cannot be guaranteed.
Meanwhile, adding a reference PciBusPei implementation might not be easy.
As I went through the RFC discussion (https://edk2.groups.io/g/rfc/topic/86658203), it seems to me that the PEI phase enumeration requirement varies dramatically between platforms.
Not sure what is the long-term goal for the current silicon code that produces of EDKII_PCI_DEVICE_PPI, if the target is to eventually put it in edk2 (not edk2-platform), then I see the feasibility of handling PEI phase IOMMU in one common place.
For 5 (GitHub Pull Request to trigger the CI tests), since 2 is not done yet, my take is that the build tests are likely to fail. Need to wait for DevicePathLib PEIM instance being merged for this.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Czajkowski, Maciej <maciej.czajkowski@intel.com>
> Sent: Monday, June 13, 2022 9:20 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use
> PCI_DEVICE_PPI to manage AHCI device
>
> Hello,
>
> 1. Yes, I will try to fix that in the v2 patch.
> 2. We have a review opened to add such instance -
> https://edk2.groups.io/g/devel/message/89970
> 3. For now it will be implemented in the silicon code, so you are right - we
> should keep them. Also, it would require a larger library refactor to consume
> such code from PCI_DEVICE_PPI if we are going to still support both
> PCI_DEVICE_PPI and AHCI_HOST_CONTROLLER_PPI. However, what are you
> thoughts about future of the library? If we can get rid of
> AHCI_HOST_CONTROLLER_PPI, I think that it is possible to remove the
> IOMMU code.
> 4. It has been run in the simulation environment, and a BlockIo read has been
> performed in PEI phase - and it was performed successfully.
> 5. Sure, will do that for v2 patch.
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: czwartek, 9 czerwca 2022 05:08
> To: devel@edk2.groups.io; Wu, Hao A <hao.a.wu@intel.com>; Czajkowski,
> Maciej <maciej.czajkowski@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>
> Subject: RE: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use
> PCI_DEVICE_PPI to manage AHCI device
>
> For "3) Could you help to check if the DMA memory related codes in
> MdeModulePkg\Bus\Ata\AhciPei\DmaMem.c can be covered by the 'PciIo'
> service in EDKII_PCI_DEVICE_PPI?"
> After a second thought, my take is that there will be no PciBusPei
> implementation added in edk2.
> So there will be no enforcement for producers of EDKII_PCI_DEVICE_PPI to
> add IOMMU support like in PciBusDxe.
>
> If my above understanding is correct, then I think we might still need to keep
> those IOMMU support codes in AhciPei PEIM.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu,
> Hao
> > A
> > Sent: Thursday, June 9, 2022 10:48 AM
> > To: Czajkowski, Maciej <maciej.czajkowski@intel.com>;
> > devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>
> > Subject: Re: [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use
> > PCI_DEVICE_PPI to manage AHCI device
> >
> > Couple of general level comments/questions:
> > 1) The implementation of functions
> > AtaAhciPciDevicePpiInstallationCallback() &
> > AtaAhciInitPrivateDataFromPciDevice() has many duplications. Is it
> > possible to abstract a separate function to reduce duplicated codes?
> > 2) What DevicePathLib instance should be used for the PEI case? As far
> > as I know, current DevicePathLib instances in edk2 do not support PEIM.
> > 3) Could you help to check if the DMA memory related codes in
> > MdeModulePkg\Bus\Ata\AhciPei\DmaMem.c can be covered by the 'PciIo'
> > service in EDKII_PCI_DEVICE_PPI?
> > 4) May I know what kind of tests are performed for this patch? Would
> > like to ensure the origin gEdkiiPeiAtaAhciHostControllerPpiGuid path is not
> broken.
> > 5) Could you help to create a GitHub Pull Request to trigger the CI
> > tests for this series?
> >
> > More inline comments below:
> >
> >
> > > -----Original Message-----
> > > From: Czajkowski, Maciej <maciej.czajkowski@intel.com>
> > > Sent: Monday, June 6, 2022 8:45 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>
> > > Subject: [edk2-devel][PATCH v1 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>
> > > Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
> > > ---
> > > MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c | 615 +++++++++++++++--
> ---
> > > MdeModulePkg/Bus/Ata/AhciPei/DevicePath.c | 44 --
> > > MdeModulePkg/Bus/Ata/AhciPei/AhciPei.inf | 5 +-
> > > 3 files changed, 458 insertions(+), 206 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> > > b/MdeModulePkg/Bus/Ata/AhciPei/AhciPei.c
> > > index 208b7e9a3606..31bb3c0760ab 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
> > >
> > > + );
> >
> >
> > Could you help to put the above 2 function declarations in AhciPei.h
> > to keep consistency?
>
> Sure, will fix that in v2 patch.
>
> >
> >
> > >
> > >
> > >
> > > 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,28 @@ 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] Private A pointer to the
> > > PEI_AHCI_CONTROLLER_PRIVATE_DATA data
> > >
> > > + structure.
> > >
> > >
> > >
> > > + @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 +197,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 +371,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 +380,261 @@ 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
> >
> >
> > Please help to add the descriptions for function returning error status.
>
> Sure, will fix that in v2 patch.
>
> >
> >
> > >
> > > +
> > >
> > > +**/
> > >
> > > +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);
> > >
> > > +}
> > >
> > > +
> > >
> > > +/**
> > >
> > > + 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
> >
> >
> > Please help to add the descriptions for function returning error status.
>
> Sure, will fix that in v2 patch.
>
> >
> >
> > >
> > > +
> > >
> > > +**/
> > >
> > > +EFI_STATUS
> > >
> > > +EFIAPI
> > >
> > > +AtaAhciPciDevicePpiInstallationCallback (
> > >
> > > + IN EFI_PEI_SERVICES **PeiServices,
> > >
> > > + IN EFI_PEI_NOTIFY_DESCRIPTOR *NotifyDescriptor,
> > >
> > > + IN VOID *Ppi
> > >
> > > + )
> > >
> > > +{
> > >
> > > + EDKII_PCI_DEVICE_PPI *PciDevice;
> > >
> > > + PCI_TYPE00 PciData;
> > >
> > > + UINTN MmioBase;
> > >
> > > + EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> > >
> > > + UINTN DevicePathLength;
> > >
> > > + EFI_STATUS Status;
> > >
> > > +
> > >
> > > + PciDevice = (EDKII_PCI_DEVICE_PPI*) Ppi;
> > >
> > > +
> > >
> > > + //
> > >
> > > + // 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
> > >
> > > + );
> >
> >
> > Do you think we need 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
> > );
> > }
>
> Yes, I think it is a good idea. Will be changed in v2 patch.
> >
> >
> > >
> > > + 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;
> > >
> > > +}
> > >
> > > +
> > >
> >
> >
> > Missing function description comments for
> > AtaAhciInitPrivateDataFromPciDevice.
>
> I will fix that in v2 patch.
>
> >
> >
> > > +EFI_STATUS
> > >
> > > +AtaAhciInitPrivateDataFromPciDevice (
> > >
> > > + VOID
> > >
> > > + )
> > >
> > > +{
> > >
> > > + EFI_STATUS Status;
> > >
> > > + UINTN ControllerIndex;
> > >
> > > + EDKII_PCI_DEVICE_PPI *PciDevice;
> > >
> > > + PCI_TYPE00 PciData;
> > >
> > > + UINTN MmioBase;
> > >
> > > + EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> > >
> > > + UINTN DevicePathLength;
> > >
> > > +
> > >
> > > + Status = EFI_SUCCESS;
> > >
> > > + for (ControllerIndex = 0; Status != EFI_NOT_FOUND;
> > > + ControllerIndex++ ) {
> > >
> > > + Status = PeiServicesLocatePpi (
> > >
> > > + &gEdkiiPeiPciDevicePpiGuid,
> > >
> > > + ControllerIndex,
> > >
> > > + NULL,
> > >
> > > + (VOID**) &PciDevice
> > >
> > > + );
> > >
> > > + if (EFI_ERROR (Status)) {
> > >
> > > + continue;
> > >
> > > + }
> > >
> > > +
> > >
> > > + //
> > >
> > > + // 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)) {
> > >
> > > + continue;
> > >
> > > + }
> > >
> > > +
> > >
> > > + if (!IS_PCI_IDE (&PciData) && !IS_PCI_SATADPA (&PciData)) {
> > >
> > > + continue;
> > >
> > > + }
> > >
> > > +
> > >
> > > + Status = PciDevice->PciIo.Attributes (
> > >
> > > + &PciDevice->PciIo,
> > >
> > > + EfiPciIoAttributeOperationSet,
> > >
> > > + EFI_PCI_DEVICE_ENABLE,
> > >
> > > + NULL
> > >
> > > + );
> > >
> > > + if (EFI_ERROR (Status)) {
> > >
> > > + continue;
> > >
> > > + }
> > >
> > > +
> > >
> > > + Status = PciDevice->PciIo.Pci.Read (
> > >
> > > + &PciDevice->PciIo,
> > >
> > > + EfiPciIoWidthUint32,
> > >
> > > + 0x24,
> > >
> > > + sizeof (UINTN),
> > >
> > > + &MmioBase
> > >
> > > + );
> > >
> > > + if (EFI_ERROR (Status)) {
> > >
> > > + continue;
> > >
> > > + }
> > >
> > > +
> > >
> > > + 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
> > >
> > > + ));
> > >
> > > + }
> > >
> > > + //
> > >
> > > + // 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);
> > >
> > > +
> > >
> > > + Status = AtaAhciInitPrivateDataFromPciDevice ();
> > >
> > > +
> > >
> > > + //
> > >
> > > + // 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__));
> >
> >
> > Suggest to use DEBUG_INFO here.
>
> Ok, will fix that in v2 patch.
>
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > + 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.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
> > >
> > > --
> > > 2.27.0.windows.1
> >
> >
> >
> >
> >
next prev parent reply other threads:[~2022-06-14 1:26 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-06 12:45 [edk2-devel][PATCH v1 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2 Maciej Czajkowski
2022-06-06 12:45 ` [edk2-devel][PATCH v1 1/2] MdeModulePkg: Add EDKII_PCI_DEVICE_PPI definition Maciej Czajkowski
2022-06-09 2:47 ` Wu, Hao A
2022-06-06 12:45 ` [edk2-devel][PATCH v1 2/2] MdeModulePkg/AhciPei: Use PCI_DEVICE_PPI to manage AHCI device Maciej Czajkowski
2022-06-09 2:47 ` Wu, Hao A
2022-06-09 3:08 ` Wu, Hao A
2022-06-13 13:19 ` Maciej Czajkowski
2022-06-14 1:26 ` Wu, Hao A [this message]
2022-06-09 2:47 ` [edk2-devel][PATCH v1 0/2] Add EDKII_PCI_DEVICE_PPI support to EDK2 Wu, Hao A
2022-06-13 13:19 ` Maciej Czajkowski
2022-06-14 1:26 ` Wu, Hao A
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB4025126E9C809FBB47C1E0ABCAAA9@DM6PR11MB4025.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox