Re: [PATCH v4 2/6] EmbeddedPkg: SiI3132: Add ScsiProtocol callbacks

[Leif Lindholm <leif.lindholm@linaro.org>]

On Tue, Mar 07, 2017 at 04:15:07PM -0600, Jeremy Linton wrote:
> Create a new module that adds the callbacks to support
> the EFI SCSI pass-through protocol. These callbacks
> wrap around the existing ATA pass-through callbacks.
> In particular the SCSI command submission routine takes
> the SCSI command and wraps it with an SATA FIS and
> sets the protocol to ATAPI. It then forwards the FIS to
> a new routine we will break out of the ATA pass-through
> callback that manages the FIS submission to the adapter.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>  .../Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c   | 424 +++++++++++++++++++++
>  1 file changed, 424 insertions(+)
>  create mode 100644 EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
> 
> diff --git a/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
> new file mode 100644
> index 0000000..66dc473
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
> @@ -0,0 +1,424 @@
> +/** @file
> +*  ATAPI support for the Silicon Image I3132
> +*
> +*  Copyright (c) 2017, ARM Limited. All rights reserved.
> +*
> +*  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.
> +*
> +**/
> +
> +#include "SataSiI3132.h"
> +
> +#include <IndustryStandard/Atapi.h>
> +#include <IndustryStandard/Scsi.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +STATIC
> +EFI_STATUS
> +SiI3132IDiscoverAtapi (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This,
> +  IN INT32 Port
> +  )
> +{
> +  SATA_SI3132_INSTANCE  *SataInstance;
> +  SATA_SI3132_PORT      *SataPort;
> +  EFI_ATA_PASS_THRU_COMMAND_PACKET Packet;
> +  ATA_IDENTIFY_DATA     *Data;
> +  EFI_STATUS            Status;
> +  EFI_ATA_STATUS_BLOCK  *Asb;
> +  EFI_ATA_COMMAND_BLOCK *Acb;
> +
> +  SataInstance = INSTANCE_FROM_SCSIPASSTHRU_THIS (This);
> +  SataPort = &SataInstance->Ports[Port];
> +
> +  Asb = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)));
> +  Acb = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_COMMAND_BLOCK)));
> +  Data = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (ATA_IDENTIFY_DATA)));
> +  ZeroMem (Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
> +  Acb->AtaCommand = ATA_CMD_IDENTIFY_DEVICE;
> +  Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 );

What does setting these three bits signify?
I agree it's slightly nicer than the "| 0xe0" in various places in
MdeModulePkg, but as someone who doesn't really know anything about
the ATA protocol, this leaves me no guidance as to what its purpose
is. And some quick googling isn't helping :)

Is there a coherent description that could be used for a #define in
Protocol/AtaPassThru.h?

> +
> +  ZeroMem (&Packet, sizeof (EFI_ATA_PASS_THRU_COMMAND_PACKET));
> +  Packet.Acb = Acb;
> +  Packet.Asb = Asb;
> +  Packet.InDataBuffer = Data;
> +  Packet.InTransferLength = sizeof (ATA_IDENTIFY_DATA);
> +  Packet.Protocol = EFI_ATA_PASS_THRU_PROTOCOL_PIO_DATA_IN;
> +  Packet.Length   = EFI_ATA_PASS_THRU_LENGTH_BYTES | EFI_ATA_PASS_THRU_LENGTH_SECTOR_COUNT;
> +
> +  Status = SiI3132AtaPassThruCommand (SataInstance, SataPort, 0, &Packet, NULL);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_WARN, "SiI ATAPI IDENTIFY DEVICE FAILURE %d\n", Status));
> +  } else {
> +    Data->ModelName[39] = 0;

  Data->ModelName[sizeof (Data->ModelName) - 1] = 0; ?

Uglier, but clearer that there isn't anything really hacky going on.

> +  }
> +
> +  FreeAlignedPages (Data, EFI_SIZE_TO_PAGES (sizeof (ATA_IDENTIFY_DATA)));
> +  FreeAlignedPages (Acb, EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_COMMAND_BLOCK)));
> +  FreeAlignedPages (Asb, EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)));
> +
> +  return Status;
> +}
> +
> +STATIC
> +EFI_STATUS
> +SiI3132ScsiPassRead (
> +  IN SATA_SI3132_INSTANCE *SataInstance,
> +  IN SATA_SI3132_PORT *SataPort,
> +  IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet)
> +{
> +  EFI_STATUS              Status;
> +  EFI_PCI_IO_PROTOCOL     *PciIo;
> +  VOID*                   PciAllocMapping;
> +  EFI_PHYSICAL_ADDRESS    PhysBuffer;
> +  UINTN                   InDataBufferLength;
> +  VOID                    *AtaSense;
> +  BOOLEAN                 RequestSense;
> +
> +  Status = EFI_SUCCESS;
> +  PciIo = SataInstance->PciIo;
> +  PciAllocMapping = NULL;
> +  InDataBufferLength = Packet->InTransferLength;
> +  AtaSense = AllocatePages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)));
> +  RequestSense = FALSE;
> +
> +  DEBUG ((DEBUG_VERBOSE, "SiI3132ScsiPassRead() CDB[0]:%X len=%d\n",
> +          ((UINT8*)Packet->Cdb)[0], Packet->InTransferLength));
> +
> +  if (AtaSense != 0) {
> +    if (Packet->InTransferLength != 0) {
> +      Status = PciIo->Map (SataInstance->PciIo,
> +                           EfiPciIoOperationBusMasterRead,
> +                           Packet->InDataBuffer,
> +                           &InDataBufferLength,
> +                           &PhysBuffer,
> +                           &PciAllocMapping);
> +
> +      if (EFI_ERROR (Status)) {
> +        DEBUG ((DEBUG_ERROR, "SiI map() failure %d\n", Status));
> +        return Status;
> +      }
> +    } else {
> +      PhysBuffer = 0;
> +    }
> +    do {
> +      // SI "The host driver must populate the area normaly used for the first SGE
> +      // with the desired ATAPI command". AKA, put the SCSI CDB itself (not the address)
> +      // in the 12 bytes comprising the SGE[0].
> +      ZeroMem (&SataPort->HostPRB->Sge[0], sizeof (SATA_SI3132_SGE));
> +      CopyMem (&SataPort->HostPRB->Sge[0], (UINT8*)Packet->Cdb, Packet->CdbLength);
> +
> +      // The SGE for the data buffer
> +      SataPort->HostPRB->Sge[1].DataAddressLow  = (UINT64)PhysBuffer;

 & MAX_UINT32
 
to prevent some toolchains from complaining about narrowing conversion?

/
    Leif

> +      SataPort->HostPRB->Sge[1].DataAddressHigh = ((UINT64)(PhysBuffer) >> 32);
> +      SataPort->HostPRB->Sge[1].Attributes      = SGE_TRM;
> +      SataPort->HostPRB->Sge[1].DataCount       = Packet->InTransferLength;
> +
> +      // Create the ATA FIS
> +      if (Packet->InTransferLength != 0) {
> +        SataPort->HostPRB->Control = PRB_CTRL_PKT_READ;
> +      } else {
> +        SataPort->HostPRB->Control = 0;
> +      }
> +
> +      SataPort->HostPRB->ProtocolOverride = 0;
> +
> +      // This is an ATA PACKET command, which encapuslates the ATAPI(sorta SCSI) command
> +      SataPort->HostPRB->Fis.FisType  = SII_FIS_REGISTER_H2D; // Register - Host to Device FIS
> +      SataPort->HostPRB->Fis.Control  = SII_FIS_CONTROL_CMD;  // Is a command
> +      SataPort->HostPRB->Fis.Command  = ATA_CMD_PACKET;
> +      SataPort->HostPRB->Fis.Features = 1;
> +      SataPort->HostPRB->Fis.Fis[0]   = 0;
> +      SataPort->HostPRB->Fis.Fis[1]   = (UINT8) (((UINT64)PhysBuffer) & 0x00ff);
> +      SataPort->HostPRB->Fis.Fis[2]   = (UINT8) (((UINT64)PhysBuffer) >> 8);
> +      SataPort->HostPRB->Fis.Fis[3]   = 0x40;
> +
> +      // Issue this as an ATA command
> +      Status = SiI3132IssueCommand (SataPort, PciIo, Packet->Timeout, AtaSense);
> +
> +      if (RequestSense) {
> +        // If we were trying to send a request sense in response to a failure
> +        // Check conditions are a normal part of SCSI operation, its expected
> +        // that most devices will do a 6/2900 (reset) and 2/2800 (media change)
> +        // at startup.
> +
> +        RequestSense              = FALSE;
> +        Packet->InTransferLength  = 0;
> +        Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
> +        Packet->TargetStatus      = EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION;
> +        Status = EFI_SUCCESS;
> +      } else if (!EFI_ERROR (Status)) {
> +        Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
> +        Packet->TargetStatus      = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
> +        ZeroMem (Packet->SenseData, Packet->SenseDataLength);
> +        Packet->SenseDataLength   = 0;
> +      } else if (Status == EFI_TIMEOUT) {
> +        SiI3132HwResetPort (SataPort);
> +        Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
> +        Packet->SenseDataLength   = 0;
> +        Packet->InTransferLength  = 0;
> +      } else {
> +        // Assume for now, that if we didn't succeed that it was a check condition.
> +        // ATAPI can't autosense on SiI, so lets emulate it with an explicit
> +        // request sense into the sense buffer.
> +        Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
> +        if ((RequestSense == FALSE) && (Packet->SenseDataLength)) {
> +          if (Packet->SenseDataLength >= SI_MAX_SENSE) {
> +            Packet->SenseDataLength = SI_MAX_SENSE - 1;
> +          }
> +          Packet->CdbLength = 6; //Request Sense is a 6 byte CDB
> +          ZeroMem (Packet->Cdb, SI_MAX_CDB);
> +          ((char*)Packet->Cdb)[0] = EFI_SCSI_OP_REQUEST_SENSE;
> +          ((char*)Packet->Cdb)[4] = Packet->SenseDataLength;
> +
> +          ZeroMem (SataPort->HostPRB, sizeof (SATA_SI3132_PRB));
> +          if (PciAllocMapping) {
> +            PciIo->Unmap (PciIo, PciAllocMapping);
> +          }
> +
> +          Packet->InTransferLength = Packet->SenseDataLength;
> +          InDataBufferLength = Packet->SenseDataLength;
> +          Status = PciIo->Map (SataInstance->PciIo,
> +                               EfiPciIoOperationBusMasterRead,
> +                               Packet->SenseData,
> +                               &InDataBufferLength,
> +                               &PhysBuffer,
> +                               &PciAllocMapping);
> +          if (EFI_ERROR (Status)) {
> +            DEBUG ((DEBUG_ERROR, "SiI map() sense failure %d\n", Status));
> +            Packet->SenseDataLength = 0;
> +          } else {
> +            // Everything seems ok, lets issue a SCSI sense
> +            RequestSense = TRUE;
> +          }
> +        }
> +      }
> +    } while (RequestSense);
> +
> +    if (PciAllocMapping) {
> +      PciIo->Unmap (PciIo, PciAllocMapping);
> +    }
> +  } else {
> +    DEBUG ((DEBUG_ERROR, "SCSI PassThru Unable to allocate sense buffer\n"));
> +    Status = EFI_OUT_OF_RESOURCES;
> +  }
> +  return Status;
> +}
> +
> +EFI_STATUS
> +SiI3132ScsiPassThru (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This,
> +  IN UINT8 *Target,
> +  IN UINT64 Lun,
> +  IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet,
> +  IN EFI_EVENT  Event OPTIONAL
> +  )
> +{
> +  EFI_STATUS              Status;
> +  SATA_SI3132_INSTANCE    *SataInstance;
> +  SATA_SI3132_PORT        *SataPort;
> +
> +  Status = EFI_SUCCESS;
> +  SataInstance = INSTANCE_FROM_SCSIPASSTHRU_THIS (This);
> +  SataPort = &SataInstance->Ports[Target[0]];
> +  ZeroMem (SataPort->HostPRB, sizeof (SATA_SI3132_PRB));
> +
> +  switch (Packet->DataDirection) {
> +  case EFI_EXT_SCSI_DATA_DIRECTION_READ:
> +    Status = SiI3132ScsiPassRead (SataInstance,SataPort, Packet);
> +    break;
> +  case EFI_EXT_SCSI_DATA_DIRECTION_WRITE:
> +    // TODO, fill this in if we ever want to connect something
> +    // besides a simple CDROM/DVDROM
> +  default:
> +    DEBUG ((DEBUG_ERROR, "SCSI PassThru Unsupported direction\n"));
> +  }
> +
> +  return Status;
> +}
> +
> +STATIC UINT8 mScsiId[TARGET_MAX_BYTES] = {
> +  0xFF, 0xFF, 0xFF, 0xFF,
> +  0xFF, 0xFF, 0xFF, 0xFF,
> +  0xFF, 0xFF, 0xFF, 0xFF,
> +  0xFF, 0xFF, 0xFF, 0xFF
> +};
> +
> +// With ATA, there are potentially three levels of addressing: port,
> +// portmultiplier and LUN. The common disk/cdroms generally are
> +// single LUN devices. Further, the standard ATAPI spec doesn't
> +// support LUN's. So, lets skip any LUN scanning/addressing.
> +//
> +// This means we are mapping the SCSI "target" to port/multiplier
> +// combinations and leaving the lun alone. Thus, target[0] is the port
> +// index, and target[1] is the multiplier index.
> +EFI_STATUS
> +SiI3132GetNextTargetLun (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This,
> +  IN OUT UINT8                       **Target,
> +  IN OUT UINT64                      *Lun
> +  )
> +{
> +  EFI_STATUS            Status;
> +  SATA_SI3132_INSTANCE  *SataInstance;
> +  UINT8                 *Target8;
> +  SATA_SI3132_PORT      *SataPort;
> +  LIST_ENTRY            *List;
> +  INT32                 Multiplier;
> +  SATA_SI3132_DEVICE    *SataDevice;
> +  BOOLEAN               Found;
> +  INT32                 Index;
> +
> +  Status = EFI_NOT_FOUND;
> +  SataInstance = INSTANCE_FROM_SCSIPASSTHRU_THIS (This);
> +  DEBUG ((DEBUG_INFO, "SCSI GetNextTargetLun L:%d\n",*Lun));
> +  if (!SataInstance) {
> +    DEBUG ((DEBUG_ERROR, "SCSI GetNextTargetLun no instance\n",Lun));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (Target == NULL || Lun == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +  if (*Target == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (*Lun > 0) {
> +    DEBUG ((DEBUG_ERROR, "SCSI GetNextTargetLun Only supports lun0 at the moment\n",Lun));
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Target8 = *Target;
> +
> +  if (CompareMem (Target8, mScsiId, TARGET_MAX_BYTES) == 0) {
> +      // This is the first time we have been called
> +      // start looking for devices on the first port.
> +      Index = 0;
> +  } else {
> +      // otherwise start where we left off
> +      Index = Target8[0] + 1; 
> +  }
> +  Found = FALSE;
> +
> +  while  ((Index < SATA_SII3132_MAXPORT) && (!Found)) {
> +    SataPort = &(SataInstance->Ports[Index]);
> +    List = SataPort->Devices.ForwardLink;
> +    Multiplier = 0;
> +    while ((List != &SataPort->Devices) && (!Found)) {
> +      SataDevice = (SATA_SI3132_DEVICE*)List;
> +      if (SataDevice->Atapi) {
> +        Found = TRUE;
> +        Target8[0] = Index;
> +        Target8[1] = Multiplier; //the device on this port (for port multipliers)
> +        DEBUG ((DEBUG_INFO, "SCSI GetNextTargetLun found device at %d %d\n",Index,Multiplier));
> +        SiI3132IDiscoverAtapi (This, Index);
> +
> +        Status = EFI_SUCCESS;
> +        break;
> +      }
> +      List = List->ForwardLink;
> +      Multiplier++;
> +    }
> +    Index++;
> +  }
> +
> +  return Status;
> +}
> +
> +EFI_STATUS
> +SiI3132ScsiBuildDevicePath (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL  *This,
> +  IN UINT8                            *Target,
> +  IN UINT64                           Lun,
> +  IN OUT EFI_DEVICE_PATH_PROTOCOL     **DevicePath
> +  )
> +{
> +  SATA_SI3132_INSTANCE *SataInstance;
> +
> +  SataInstance = INSTANCE_FROM_SCSIPASSTHRU_THIS (This);
> +  DEBUG ((DEBUG_INFO, "SCSI BuildDevicePath T:%d L:%d\n", *Target, Lun));
> +
> +  if (Lun < 1) {
> +    return SiI3132BuildDevicePath (&SataInstance->AtaPassThruProtocol, Target[0], Target[1], DevicePath);
> +  }
> +  return EFI_NOT_FOUND;
> +}
> +
> +EFI_STATUS
> +SiI3132GetTargetLun (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL  *This,
> +  IN EFI_DEVICE_PATH_PROTOCOL         *DevicePath,
> +  OUT UINT8                           **Target,
> +  OUT UINT64                          *Lun
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  Status = EFI_UNSUPPORTED;
> +  DEBUG ((DEBUG_ERROR, "SCSI GetNextTarget T:%d L:%d\n",*Target,Lun));
> +  return Status;
> +}
> +
> +// So normally we would want to do a ATA port reset here (which is generally
> +// frowned on with modern SCSI transports (sas, fc, etc) unless all the
> +// attached devices are in an error state). But the EFI SCSI protocol isn't
> +// specific enough to specify a device for which we want to reset the port.
> +// This means we are basically stuck simulating it by resetting all the ports
> +// which is bad karma. So lets just claim its unsupported and if we discover
> +// that port resets are needed as part of the target/lun reset then consider
> +// doing it automatically as part of that path.
> +EFI_STATUS
> +SiI3132ResetChannel (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This
> +  )
> +{
> +  EFI_STATUS Status;
> +  Status = EFI_UNSUPPORTED;
> +
> +  DEBUG ((DEBUG_ERROR, "SCSI ResetChannel\n"));
> +  return Status;
> +}
> +
> +// Just do a device reset here, in the future if we find out that is insufficient
> +// try to just reset the SATA port the device is attached to as well.
> +EFI_STATUS
> +SiI3132ResetTargetLun (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL               *This,
> +  IN UINT8                                         *Target,
> +  IN UINT64                                        Lun
> +  )
> +{
> +  EFI_STATUS Status;
> +  SATA_SI3132_INSTANCE *SataInstance;
> +
> +  Status = EFI_NOT_FOUND;
> +  SataInstance = INSTANCE_FROM_SCSIPASSTHRU_THIS (This);
> +
> +  DEBUG ((DEBUG_ERROR, "SCSI ResetTargetLun\n"));
> +
> +  if (Lun < 1) {
> +    Status = SiI3132ResetDevice (&SataInstance->AtaPassThruProtocol,
> +                                 Target[0], Target[1]);
> +  }
> +  return Status;
> +}
> +
> +EFI_STATUS
> +SiI3132GetNextTarget (
> +  IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL               *This,
> +  IN OUT UINT8                                     **Target
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  Status = EFI_UNSUPPORTED;
> +  DEBUG ((DEBUG_VERBOSE, "SCSI GetNextTarget\n"));
> +  return Status;
> +}
> -- 
> 2.9.3
>