Re: [PATCH v2 02/12] MdeModulePkg: Add definitions for EDKII PEI ATA PassThru PPI

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: Ni, Ray
> Sent: Thursday, January 31, 2019 11:23 AM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Wang, Jian J; Dong, Eric
> Subject: RE: [PATCH v2 02/12] MdeModulePkg: Add definitions for EDKII PEI
> ATA PassThru PPI
> 
> Hao,
> You could re-use the structure definition in AtaPassThru protocol.
> For example: EFI_ATA_PASS_THRU_MODE v.s.
> EDKII_PEI_ATA_PASS_THRU_MODE
> I see more duplicated structure definitions. We can avoid them.

Got it.
I will refine this patch in the next series.

Best Regards,
Hao Wu

> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Wu, Hao A <hao.a.wu@intel.com>
> > Sent: Thursday, January 31, 2019 10:49 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> > Ni, Ray <ray.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
> > Subject: [PATCH v2 02/12] MdeModulePkg: Add definitions for EDKII PEI ATA
> > PassThru PPI
> >
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1409
> >
> > This commit will add the definitions for EDKII PEI ATA PassThru PPI. This PPI
> > will provide services that allow ATA commands to be sent to ATA devices
> > attached to an ATA controller in the PEI phase.
> >
> > More specifically, the PPI will provide services to:
> >
> > * Send ATA commands to an ATA device (by service 'PassThru');
> > * Get the list of the attached ATA device on a controller (by services
> >   'GetNextPort' and 'GetNextDevice');
> > * Get the identification information (DevicePath) of the underlying ATA
> >   host controller (by service 'GetDevicePath').
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  MdeModulePkg/MdeModulePkg.dec          |   3 +
> >  MdeModulePkg/Include/Ppi/AtaPassThru.h | 359 ++++++++++++++++++++
> >  2 files changed, 362 insertions(+)
> >
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> > b/MdeModulePkg/MdeModulePkg.dec index 4411185073..8efb19e626
> > 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -483,6 +483,9 @@
> >    ## Include/Ppi/AtaAhciController.h
> >    gEdkiiPeiAtaAhciHostControllerPpiGuid     = { 0x61dd33ea, 0x421f, 0x4cc0,
> > { 0x89, 0x29, 0xff, 0xee, 0xa9, 0xa1, 0xa2, 0x61 } }
> >
> > +  ## Include/Ppi/AtaPassThru.h
> > +  gEdkiiPeiAtaPassThruPpiGuid               = { 0xa16473fd, 0xd474, 0x4c89,
> { 0xae,
> > 0xc7, 0x90, 0xb8, 0x3c, 0x73, 0x86, 0x9  } }
> > +
> >  [Protocols]
> >    ## Load File protocol provides capability to load and unload EFI image into
> > memory and execute it.
> >    #  Include/Protocol/LoadPe32Image.h
> > diff --git a/MdeModulePkg/Include/Ppi/AtaPassThru.h
> > b/MdeModulePkg/Include/Ppi/AtaPassThru.h
> > new file mode 100644
> > index 0000000000..9281f0b833
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Ppi/AtaPassThru.h
> > @@ -0,0 +1,359 @@
> > +/** @file
> > +
> > +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>  This
> > + program and the accompanying materials  are licensed and made
> > + available under the terms and conditions of the BSD License  which
> > + accompanies this distribution.  The full text of the license may be
> > + found at  http://opensource.org/licenses/bsd-license.php
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#ifndef _EDKII_ATA_PASS_THRU_PPI_H_
> > +#define _EDKII_ATA_PASS_THRU_PPI_H_
> > +
> > +#include <Protocol/DevicePath.h>
> > +
> > +///
> > +/// Global ID for the EDKII_PEI_ATA_PASS_THRU_PPI.
> > +///
> > +#define EDKII_PEI_ATA_PASS_THRU_PPI_GUID \
> > +  { \
> > +    0xa16473fd, 0xd474, 0x4c89, { 0xae, 0xc7, 0x90, 0xb8, 0x3c, 0x73,
> > +0x86, 0x9 } \
> > +  }
> > +
> > +//
> > +// Forward declaration for the EDKII_PEI_ATA_PASS_THRU_PPI.
> > +//
> > +typedef struct _EDKII_PEI_ATA_PASS_THRU_PPI
> > +EDKII_PEI_ATA_PASS_THRU_PPI;
> > +
> > +//
> > +// Revision The revision to which the ATA Pass Thru PPI interface adheres.
> > +//          All future revisions must be backwards compatible.
> > +//          If a future version is not back wards compatible it is not the same
> > GUID.
> > +//
> > +#define EDKII_PEI_ATA_PASS_THRU_PPI_REVISION                 0x00010000
> > +
> > +typedef struct {
> > +  UINT32    Attributes;
> > +  UINT32    IoAlign;
> > +} EDKII_PEI_ATA_PASS_THRU_MODE;
> > +
> > +///
> > +/// If this bit is set, then the EDKII_PEI_ATA_PASS_THRU_PPI interface
> > +is for /// physical devices on the ATA controller.
> > +///
> > +#define EDKII_PEI_ATA_PASS_THRU_ATTRIBUTES_PHYSICAL          0x0001
> > +///
> > +/// If this bit is set, then the EDKII_PEI_ATA_PASS_THRU_PPI interface
> > +is for /// logical devices on the ATA controller.
> > +///
> > +#define EDKII_PEI_ATA_PASS_THRU_ATTRIBUTES_LOGICAL           0x0002
> > +
> > +typedef struct {
> > +  UINT8    Reserved1[2];
> > +  UINT8    AtaStatus;
> > +  UINT8    AtaError;
> > +  UINT8    AtaSectorNumber;
> > +  UINT8    AtaCylinderLow;
> > +  UINT8    AtaCylinderHigh;
> > +  UINT8    AtaDeviceHead;
> > +  UINT8    AtaSectorNumberExp;
> > +  UINT8    AtaCylinderLowExp;
> > +  UINT8    AtaCylinderHighExp;
> > +  UINT8    Reserved2;
> > +  UINT8    AtaSectorCount;
> > +  UINT8    AtaSectorCountExp;
> > +  UINT8    Reserved3[6];
> > +} EDKII_PEI_ATA_STATUS_BLOCK;
> > +
> > +typedef struct {
> > +  UINT8    Reserved1[2];
> > +  UINT8    AtaCommand;
> > +  UINT8    AtaFeatures;
> > +  UINT8    AtaSectorNumber;
> > +  UINT8    AtaCylinderLow;
> > +  UINT8    AtaCylinderHigh;
> > +  UINT8    AtaDeviceHead;
> > +  UINT8    AtaSectorNumberExp;
> > +  UINT8    AtaCylinderLowExp;
> > +  UINT8    AtaCylinderHighExp;
> > +  UINT8    AtaFeaturesExp;
> > +  UINT8    AtaSectorCount;
> > +  UINT8    AtaSectorCountExp;
> > +  UINT8    Reserved2[6];
> > +} EDKII_PEI_ATA_COMMAND_BLOCK;
> > +
> > +typedef UINT8 EDKII_PEI_ATA_PASS_THRU_CMD_PROTOCOL;
> > +
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_ATA_HARDWARE_RESET
> > 0x00
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_ATA_SOFTWARE_RESET
> > 0x01
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_ATA_NON_DATA
> > 0x02
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_PIO_DATA_IN         0x04
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_PIO_DATA_OUT        0x05
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_DMA                 0x06
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_DMA_QUEUED          0x07
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_DEVICE_DIAGNOSTIC
> > 0x08
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_DEVICE_RESET        0x09
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_UDMA_DATA_IN
> > 0x0A
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_UDMA_DATA_OUT
> > 0x0B
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_FPDMA               0x0C
> > +#define EDKII_PEI_ATA_PASS_THRU_PROTOCOL_RETURN_RESPONSE
> > 0xFF
> > +
> > +typedef UINT8 EDKII_PEI_ATA_PASS_THRU_LENGTH;
> > +
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_BYTES                 0x80
> > +
> > +
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_MASK                  0x70
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_NO_DATA_TRANSFER
> > 0x00
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_FEATURES              0x10
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_SECTOR_COUNT          0x20
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_TPSIU                 0x30
> > +
> > +#define EDKII_PEI_ATA_PASS_THRU_LENGTH_COUNT                 0x0F
> > +
> > +typedef struct {
> > +  //
> > +  // A pointer to the sense data that was generated by the execution of
> > +the ATA
> > +  // command. It must be aligned to the boundary specified in the
> > +IoAlign field
> > +  // in the EDKII_PEI_ATA_PASS_THRU_PASSTHRU structure.
> > +  //
> > +  EDKII_PEI_ATA_STATUS_BLOCK              *Asb;
> > +  //
> > +  // A pointer to buffer that contains the Command Data Block to send
> > +to the ATA
> > +  // device specified by Port and PortMultiplierPort.
> > +  //
> > +  EDKII_PEI_ATA_COMMAND_BLOCK             *Acb;
> > +  //
> > +  // The timeout, in 100 ns units, to use for the execution of this ATA
> > command.
> > +  // A Timeout value of 0 means that this function will wait
> > +indefinitely for the
> > +  // ATA command to execute. If Timeout is greater than zero, then this
> > +function
> > +  // will return EFI_TIMEOUT if the time required to execute the ATA
> > +command is
> > +  // greater than Timeout.
> > +  //
> > +  UINT64                                  Timeout;
> > +  //
> > +  // A pointer to the data buffer to transfer between the ATA
> > +controller and the
> > +  // ATA device for read and bidirectional commands. For all write and
> > +non data
> > +  // commands where InTransferLength is 0 this field is optional and may be
> > NULL.
> > +  // If this field is not NULL, then it must be aligned on the boundary
> > +specified
> > +  // by the IoAlign field in the EDKII_PEI_ATA_PASS_THRU_PASSTHRU
> > structure.
> > +  //
> > +  VOID                                    *InDataBuffer;
> > +  //
> > +  // A pointer to the data buffer to transfer between the ATA
> > +controller and the
> > +  // ATA device for write or bidirectional commands. For all read and
> > +non data
> > +  // commands where OutTransferLength is 0 this field is optional and may
> > be NULL.
> > +  // If this field is not NULL, then it must be aligned on the boundary
> > +specified
> > +  // by the IoAlign field in the EDKII_PEI_ATA_PASS_THRU_PASSTHRU
> > structure.
> > +  //
> > +  VOID                                    *OutDataBuffer;
> > +  //
> > +  // On input, the size, in bytes, of InDataBuffer. On output, the
> > +number of bytes
> > +  // transferred between the ATA controller and the ATA device. If
> > +InTransferLength
> > +  // is larger than the ATA controller can handle, no data will be
> > +transferred,
> > +  // InTransferLength will be updated to contain the number of bytes
> > +that the ATA
> > +  // controller is able to transfer, and EFI_BAD_BUFFER_SIZE will be returned.
> > +  //
> > +  UINT32                                  InTransferLength;
> > +  //
> > +  // On Input, the size, in bytes of OutDataBuffer. On Output, the
> > +Number of bytes
> > +  // transferred between ATA Controller and the ATA device. If
> > +OutTransferLength is
> > +  // larger than the ATA controller can handle, no data will be
> > +transferred,
> > +  // OutTransferLength will be updated to contain the number of bytes
> > +that the ATA
> > +  // controller is able to transfer, and EFI_BAD_BUFFER_SIZE will be returned.
> > +  //
> > +  UINT32                                  OutTransferLength;
> > +  //
> > +  // Specifies the PPI used when the ATA device executes the command.
> > +  //
> > +  EDKII_PEI_ATA_PASS_THRU_CMD_PROTOCOL    Protocol;
> > +  //
> > +  // Specifies the way in which the ATA command length is encoded.
> > +  //
> > +  EDKII_PEI_ATA_PASS_THRU_LENGTH          Length;
> > +} EDKII_PEI_ATA_PASS_THRU_COMMAND_PACKET;
> > +
> > +
> > +/**
> > +  Sends an ATA command to an ATA device that is attached to the ATA
> > controller.
> > +
> > +  @param[in]     This                  The PPI instance pointer.
> > +  @param[in]     Port                  The port number of the ATA device to send
> > +                                       the command.
> > +  @param[in]     PortMultiplierPort    The port multiplier port number of the
> > ATA
> > +                                       device to send the command.
> > +                                       If there is no port multiplier, then specify
> > +                                       0xFFFF.
> > +  @param[in,out] Packet                A pointer to the ATA command to send to
> > +                                       the ATA device specified by Port and
> > +                                       PortMultiplierPort.
> > +
> > +  @retval EFI_SUCCESS              The ATA command was sent by the host. For
> > +                                   bi-directional commands, InTransferLength bytes
> > +                                   were transferred from InDataBuffer. For write
> > +                                   and bi-directional commands, OutTransferLength
> > +                                   bytes were transferred by OutDataBuffer.
> > +  @retval EFI_NOT_FOUND            The specified ATA device is not found.
> > +  @retval EFI_INVALID_PARAMETER    The contents of Acb are invalid. The
> > ATA command
> > +                                   was not sent, so no additional status information
> > +                                   is available.
> > +  @retval EFI_BAD_BUFFER_SIZE      The ATA command was not executed.
> > The number
> > +                                   of bytes that could be transferred is returned
> > +                                   in InTransferLength. For write and bi-directional
> > +                                   commands, OutTransferLength bytes were transferred
> > +                                   by OutDataBuffer.
> > +  @retval EFI_NOT_READY            The ATA command could not be sent
> > because there
> > +                                   are too many ATA commands already queued. The
> > +                                   caller may retry again later.
> > +  @retval EFI_DEVICE_ERROR         A device error occurred while attempting
> > to
> > +                                   send the ATA command.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_ATA_PASS_THRU_PASSTHRU) (
> > +  IN     EDKII_PEI_ATA_PASS_THRU_PPI               *This,
> > +  IN     UINT16                                    Port,
> > +  IN     UINT16                                    PortMultiplierPort,
> > +  IN OUT EDKII_PEI_ATA_PASS_THRU_COMMAND_PACKET    *Packet
> > +  );
> > +
> > +/**
> > +  Used to retrieve the list of legal port numbers for ATA devices on an ATA
> > controller.
> > +  These can either be the list of ports where ATA devices are actually
> > +present or the
> > +  list of legal port numbers for the ATA controller. Regardless, the
> > +caller of this
> > +  function must probe the port number returned to see if an ATA device
> > +is actually
> > +  present at that location on the ATA controller.
> > +
> > +  The GetNextPort() function retrieves the port number on an ATA
> > + controller. If on  input Port is 0xFFFF, then the port number of the
> > + first port on the ATA controller  is returned in Port and EFI_SUCCESS is
> > returned.
> > +
> > +  If Port is a port number that was returned on a previous call to
> > + GetNextPort(),  then the port number of the next port on the ATA
> > + controller is returned in Port,  and EFI_SUCCESS is returned. If Port
> > + is not 0xFFFF and Port was not returned on  a previous call to
> > GetNextPort(), then EFI_INVALID_PARAMETER is returned.
> > +
> > +  If Port is the port number of the last port on the ATA controller,
> > + then EFI_NOT_FOUND  is returned.
> > +
> > +  @param[in]     This    The PPI instance pointer.
> > +  @param[in,out] Port    On input, a pointer to the port number on the ATA
> > controller.
> > +                         On output, a pointer to the next port number on the ATA
> > +                         controller. An input value of 0xFFFF retrieves the first
> > +                         port number on the ATA controller.
> > +
> > +  @retval EFI_SUCCESS              The next port number on the ATA controller
> > was
> > +                                   returned in Port.
> > +  @retval EFI_NOT_FOUND            There are no more ports on this ATA
> > controller.
> > +  @retval EFI_INVALID_PARAMETER    Port is not 0xFFFF and Port was not
> > returned
> > +                                   on a previous call to GetNextPort().
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_ATA_PASS_THRU_THRU_GET_NEXT_PORT) (
> > +  IN     EDKII_PEI_ATA_PASS_THRU_PPI    *This,
> > +  IN OUT UINT16                         *Port
> > +  );
> > +
> > +/**
> > +  Used to retrieve the list of legal port multiplier port numbers for
> > +ATA devices
> > +  on a port of an ATA controller. These can either be the list of port
> > +multiplier
> > +  ports where ATA devices are actually present on port or the list of
> > +legal port
> > +  multiplier ports on that port. Regardless, the caller of this
> > +function must probe
> > +  the port number and port multiplier port number returned to see if an
> > +ATA device
> > +  is actually present.
> > +
> > +  The GetNextDevice() function retrieves the port multiplier port
> > + number of an ATA  device present on a port of an ATA controller.
> > +
> > +  If PortMultiplierPort points to a port multiplier port number value
> > + that was  returned on a previous call to GetNextDevice(), then the
> > + port multiplier port  number of the next ATA device on the port of the
> > + ATA controller is returned in  PortMultiplierPort, and EFI_SUCCESS is
> > returned.
> > +
> > +  If PortMultiplierPort points to 0xFFFF, then the port multiplier port
> > + number  of the first ATA device on port of the ATA controller is
> > + returned in PortMultiplierPort  and EFI_SUCCESS is returned.
> > +
> > +  If PortMultiplierPort is not 0xFFFF and the value pointed to by
> > + PortMultiplierPort  was not returned on a previous call to
> > + GetNextDevice(), then EFI_INVALID_PARAMETER  is returned.
> > +
> > +  If PortMultiplierPort is the port multiplier port number of the last
> > + ATA device  on the port of the ATA controller, then EFI_NOT_FOUND is
> > returned.
> > +
> > +  @param[in]     This                  The PPI instance pointer.
> > +  @param[in]     Port                  The port number present on the ATA
> controller.
> > +  @param[in,out] PortMultiplierPort    On input, a pointer to the port
> > multiplier
> > +                                       port number of an ATA device present on the
> > +                                       ATA controller. If on input a PortMultiplierPort
> > +                                       of 0xFFFF is specified, then the port multiplier
> > +                                       port number of the first ATA device is returned.
> > +                                       On output, a pointer to the port multiplier port
> > +                                       number of the next ATA device present on an ATA
> > +                                       controller.
> > +
> > +  @retval EFI_SUCCESS              The port multiplier port number of the next
> > ATA
> > +                                   device on the port of the ATA controller was
> > +                                   returned in PortMultiplierPort.
> > +  @retval EFI_NOT_FOUND            There are no more ATA devices on this
> > port of
> > +                                   the ATA controller.
> > +  @retval EFI_INVALID_PARAMETER    PortMultiplierPort is not 0xFFFF, and
> > PortMultiplierPort
> > +                                   was not returned on a previous call to GetNextDevice().
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_ATA_PASS_THRU_GET_NEXT_DEVICE) (
> > +  IN     EDKII_PEI_ATA_PASS_THRU_PPI    *This,
> > +  IN     UINT16                         Port,
> > +  IN OUT UINT16                         *PortMultiplierPort
> > +  );
> > +
> > +/**
> > +  Gets the device path information of the underlying ATA host controller.
> > +
> > +  @param[in]  This                The PPI instance pointer.
> > +  @param[out] DevicePathLength    The length of the device path in bytes
> > specified
> > +                                  by DevicePath.
> > +  @param[out] DevicePath          The device path of the underlying ATA host
> > controller.
> > +                                  This field re-uses EFI Device Path Protocol as
> > +                                  defined by Section 10.2 EFI Device Path Protocol
> > +                                  of UEFI 2.7 Specification.
> > +
> > +  @retval EFI_SUCCESS              The device path of the ATA host controller
> has
> > +                                   been successfully returned.
> > +  @retval EFI_INVALID_PARAMETER    DevicePathLength or DevicePath is
> > NULL.
> > +  @retval EFI_OUT_OF_RESOURCES     Not enough resource to return the
> > device path.
> > +
> > +**/
> > +typedef
> > +EFI_STATUS
> > +(EFIAPI *EDKII_PEI_ATA_PASS_THRU_GET_DEVICE_PATH) (
> > +  IN  EDKII_PEI_ATA_PASS_THRU_PPI    *This,
> > +  OUT UINTN                          *DevicePathLength,
> > +  OUT EFI_DEVICE_PATH_PROTOCOL       **DevicePath
> > +  );
> > +
> > +//
> > +// EDKII_PEI_ATA_PASS_THRU_PPI provides the services that are required
> > +to send // ATA commands to an ATA device during PEI.
> > +//
> > +struct _EDKII_PEI_ATA_PASS_THRU_PPI {
> > +  UINT64                                        Revision;
> > +  EDKII_PEI_ATA_PASS_THRU_MODE                  *Mode;
> > +  EDKII_PEI_ATA_PASS_THRU_PASSTHRU              PassThru;
> > +  EDKII_PEI_ATA_PASS_THRU_THRU_GET_NEXT_PORT    GetNextPort;
> > +  EDKII_PEI_ATA_PASS_THRU_GET_NEXT_DEVICE       GetNextDevice;
> > +  EDKII_PEI_ATA_PASS_THRU_GET_DEVICE_PATH       GetDevicePath;
> > +};
> > +
> > +extern EFI_GUID gEdkiiPeiAtaPassThruPpiGuid;
> > +
> > +#endif
> > --
> > 2.12.0.windows.1