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

["Ni, Ray" <ray.ni@intel.com>]

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.

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