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 | 431 +++++++++++++++++++++
1 file changed, 431 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..6858426
--- /dev/null
+++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
@@ -0,0 +1,431 @@
+/** @file
+* ATAPI support for the Silicon Image I3132
+*
+* Copyright (c) 2016, 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>
+
+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=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)), EFI_PAGE_SIZE);
+ Acb=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_COMMAND_BLOCK)), EFI_PAGE_SIZE);
+ Data=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (ATA_IDENTIFY_DATA)), EFI_PAGE_SIZE);
+ ZeroMem (Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
+ Acb->AtaCommand = ATA_CMD_IDENTIFY_DEVICE;
+ Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 );
+
+ 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;
+ }
+
+ 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;
+}
+
+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 = AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)),
+ EFI_PAGE_SIZE);;
+ RequestSense = FALSE;
+
+ DEBUG ((DEBUG_VERBOSE, "SiI3132ScsiPassRead() CDB[0]:%X len=%d\n",
+ ((UINT8*)Packet->Cdb)[0], Packet->InTransferLength));
+
+ if (AtaSense) {
+ if (Packet->InTransferLength) {
+ 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;
+ 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
+ SataPort->HostPRB->Control = (Packet->InTransferLength ? PRB_CTRL_PKT_READ: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.
+ UINT8 *sense;
+
+ sense = (UINT8 *)Packet->SenseData;
+ RequestSense=FALSE;
+ Packet->InTransferLength = 0;
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION;
+ if ((Packet->SenseDataLength > 13) &&
+ (sense[0] & EFI_SCSI_REQUEST_SENSE_ERROR)) {
+ DEBUG ((DEBUG_INFO, "SiI3132ScsiPassRead() Key %X ASC(Q) %02X%02X\n",
+ EFI_SCSI_SK_VALUE (sense[2]), sense[12], sense[13]));
+ }
+ 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;
+}
+
+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, lun
+// although I'm not sure there are any actual multilun devices anymore (cd autoloaders/etc)
+// on ATA the common disk/cdroms generally are single lun devices. So, lets skip the LUN
+// scanning/addressing. Futher, the standard ATAPI spec doesn't support lun's.
+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;
+
+ 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;
+
+ // look for first device
+ if (CompareMem (Target8, mScsiId, TARGET_MAX_BYTES) == 0) {
+ BOOLEAN Found;
+ INT32 Index;
+
+ Found = FALSE;
+ Target8 = *Target;
+ for (Index = 0; ((Index < SATA_SII3132_MAXPORT) && (!Found)); Index++) {
+ SATA_SI3132_PORT *SataPort;
+ LIST_ENTRY *List;
+ INT32 Multiplier;
+
+ SataPort = &(SataInstance->Ports[Index]);
+ List = SataPort->Devices.ForwardLink;
+ Multiplier = 0;
+ while ((List != &SataPort->Devices) && (!Found)) {
+ SATA_SI3132_DEVICE* SataDevice;
+
+ 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++;
+ }
+ }
+ } else {
+ //todo find the next device,
+ }
+
+ return Status;
+}
+
+EFI_STATUS
+SiI3132GetNextTargetLun2 (
+ IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This,
+ IN UINT8 *Target,
+ IN UINT64 Lun,
+ IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath
+ )
+{
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+ DEBUG ((DEBUG_INFO, "SCSI GetNextTargetLun2\n"));
+ 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_SUCCESS;
+ DEBUG ((DEBUG_INFO, "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_SUCCESS;
+ DEBUG ((DEBUG_VERBOSE, "SCSI GetNextTarget\n"));
+ return Status;
+}
--
2.9.3
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 23 February 2017 at 22:33, Jeremy Linton <jeremy.linton@arm.com> 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 | 431 +++++++++++++++++++++
> 1 file changed, 431 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..6858426
> --- /dev/null
> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
> @@ -0,0 +1,431 @@
> +/** @file
> +* ATAPI support for the Silicon Image I3132
> +*
> +* Copyright (c) 2016, ARM Limited. All rights reserved.
> +*
2017?
> +* 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=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)), EFI_PAGE_SIZE);
> + Acb=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_COMMAND_BLOCK)), EFI_PAGE_SIZE);
> + Data=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (ATA_IDENTIFY_DATA)), EFI_PAGE_SIZE);
Apologies if I did not spot this the first time around, but page
allocations are always page aligned so no point in using the 'aligned'
flavor here
And please use spaces before and after =
> + ZeroMem (Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
> + Acb->AtaCommand = ATA_CMD_IDENTIFY_DEVICE;
> + Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 );
> +
> + ZeroMem (&Packet, sizeof (EFI_ATA_PASS_THRU_COMMAND_PACKET));
> + Packet.Acb=Acb;
> + Packet.Asb=Asb;
Same here
> + 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
> + {
} else {
> + Data->ModelName[39]=0;
> + }
> +
> + 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 = AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)),
AllocatePages()
> + EFI_PAGE_SIZE);;
Double ;;
> + RequestSense = FALSE;
> +
> + DEBUG ((DEBUG_VERBOSE, "SiI3132ScsiPassRead() CDB[0]:%X len=%d\n",
> + ((UINT8*)Packet->Cdb)[0], Packet->InTransferLength));
> +
> + if (AtaSense) {
> + if (Packet->InTransferLength) {
!= 0 and != NULL are more idiomatic for EDK2
> + 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;
Spaces
> + }
> + 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;
> + 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
> + SataPort->HostPRB->Control = (Packet->InTransferLength ? PRB_CTRL_PKT_READ:0);
Fine to use () with a ternary but not in this way. Also, spaces around :
> + 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.
> + UINT8 *sense;
> +
Sense
> + sense = (UINT8 *)Packet->SenseData;
> + RequestSense=FALSE;
Spaces
> + Packet->InTransferLength = 0;
> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
> + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION;
> + if ((Packet->SenseDataLength > 13) &&
What's a '13' ?
> + (sense[0] & EFI_SCSI_REQUEST_SENSE_ERROR)) {
> + DEBUG ((DEBUG_INFO, "SiI3132ScsiPassRead() Key %X ASC(Q) %02X%02X\n",
> + EFI_SCSI_SK_VALUE (sense[2]), sense[12], sense[13]));
> + }
> + 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
I think this is fine for now
> + default:
> + DEBUG ((DEBUG_ERROR, "SCSI PassThru Unsupported direction\n"));
> + }
> +
> + return Status;
> +}
> +
> +UINT8 mScsiId[TARGET_MAX_BYTES] = {
STATIC?
> + 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, lun
> +// although I'm not sure there are any actual multilun devices anymore (cd autoloaders/etc)
> +// on ATA the common disk/cdroms generally are single lun devices. So, lets skip the LUN
> +// scanning/addressing. Futher, the standard ATAPI spec doesn't support lun's.
> +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;
> +
> + 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;
> +
> + // look for first device
> + if (CompareMem (Target8, mScsiId, TARGET_MAX_BYTES) == 0) {
> + BOOLEAN Found;
> + INT32 Index;
> +
> + Found = FALSE;
> + Target8 = *Target;
> + for (Index = 0; ((Index < SATA_SII3132_MAXPORT) && (!Found)); Index++) {
> + SATA_SI3132_PORT *SataPort;
> + LIST_ENTRY *List;
> + INT32 Multiplier;
> +
> + SataPort = &(SataInstance->Ports[Index]);
> + List = SataPort->Devices.ForwardLink;
> + Multiplier = 0;
> + while ((List != &SataPort->Devices) && (!Found)) {
> + SATA_SI3132_DEVICE* SataDevice;
Please move this to the top
> +
> + 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++;
> + }
> + }
> + } else {
> + //todo find the next device,
What does this mean? In what way does this limit the current functionality?
> + }
> +
> + return Status;
> +}
> +
> +EFI_STATUS
> +SiI3132GetNextTargetLun2 (
> + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This,
> + IN UINT8 *Target,
> + IN UINT64 Lun,
> + IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = EFI_SUCCESS;
> + DEBUG ((DEBUG_INFO, "SCSI GetNextTargetLun2\n"));
> + return Status;
> +}
> +
This has an OUT parameter, and you are returning success. This means
the caller may try to dereference the pointer in *DevicePath,
potentially causing a crash
> +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_SUCCESS;
> + DEBUG ((DEBUG_INFO, "SCSI GetNextTarget T:%d L:%d\n",*Target,Lun));
> + return Status;
> +}
> +
Same here
> +// 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_SUCCESS;
> + DEBUG ((DEBUG_VERBOSE, "SCSI GetNextTarget\n"));
> + return Status;
> +}
> --
> 2.9.3
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Hi,
On 02/24/2017 11:08 AM, Ard Biesheuvel wrote:
> On 23 February 2017 at 22:33, Jeremy Linton <jeremy.linton@arm.com> 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 | 431 +++++++++++++++++++++
>> 1 file changed, 431 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..6858426
>> --- /dev/null
>> +++ b/EmbeddedPkg/Drivers/SataSiI3132Dxe/SiI3132ScsiPassThru.c
>> @@ -0,0 +1,431 @@
>> +/** @file
>> +* ATAPI support for the Silicon Image I3132
>> +*
>> +* Copyright (c) 2016, ARM Limited. All rights reserved.
>> +*
>
> 2017?
>
>> +* 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=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)), EFI_PAGE_SIZE);
>> + Acb=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_COMMAND_BLOCK)), EFI_PAGE_SIZE);
>> + Data=AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (ATA_IDENTIFY_DATA)), EFI_PAGE_SIZE);
>
> Apologies if I did not spot this the first time around, but page
> allocations are always page aligned so no point in using the 'aligned'
> flavor here
>
> And please use spaces before and after =
I'm sort of blind to most low level code formatting after many years
staring at dozens of formats. Anyway, I though the updated patch checker
had caught all of them. I just did a global search to find and fix them.
>
>> + ZeroMem (Acb, sizeof (EFI_ATA_COMMAND_BLOCK));
>> + Acb->AtaCommand = ATA_CMD_IDENTIFY_DEVICE;
>> + Acb->AtaDeviceHead = (UINT8) (BIT7 | BIT6 | BIT5 );
>> +
>> + ZeroMem (&Packet, sizeof (EFI_ATA_PASS_THRU_COMMAND_PACKET));
>> + Packet.Acb=Acb;
>> + Packet.Asb=Asb;
>
> Same here
>
>> + 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
>> + {
>
> } else {
>
>> + Data->ModelName[39]=0;
>> + }
>> +
>> + 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 = AllocateAlignedPages (EFI_SIZE_TO_PAGES (sizeof (EFI_ATA_STATUS_BLOCK)),
>
> AllocatePages()
>
>> + EFI_PAGE_SIZE);;
>
> Double ;;
>
>> + RequestSense = FALSE;
>> +
>> + DEBUG ((DEBUG_VERBOSE, "SiI3132ScsiPassRead() CDB[0]:%X len=%d\n",
>> + ((UINT8*)Packet->Cdb)[0], Packet->InTransferLength));
>> +
>> + if (AtaSense) {
>> + if (Packet->InTransferLength) {
>
> != 0 and != NULL are more idiomatic for EDK2
>
>> + 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;
>
> Spaces
>
>> + }
>> + 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;
>> + 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
>> + SataPort->HostPRB->Control = (Packet->InTransferLength ? PRB_CTRL_PKT_READ:0);
>
> Fine to use () with a ternary but not in this way. Also, spaces around :
>
>> + 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.
>> + UINT8 *sense;
>> +
>
> Sense
I've removed the whole print the error sense block.
>
>> + sense = (UINT8 *)Packet->SenseData;
>> + RequestSense=FALSE;
>
> Spaces
>
>> + Packet->InTransferLength = 0;
>> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
>> + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_CHECK_CONDITION;
>> + if ((Packet->SenseDataLength > 13) &&
>
> What's a '13' ?
>
>> + (sense[0] & EFI_SCSI_REQUEST_SENSE_ERROR)) {
>> + DEBUG ((DEBUG_INFO, "SiI3132ScsiPassRead() Key %X ASC(Q) %02X%02X\n",
>> + EFI_SCSI_SK_VALUE (sense[2]), sense[12], sense[13]));
>> + }
>> + 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
>
> I think this is fine for now
>
>> + default:
>> + DEBUG ((DEBUG_ERROR, "SCSI PassThru Unsupported direction\n"));
>> + }
>> +
>> + return Status;
>> +}
>> +
>> +UINT8 mScsiId[TARGET_MAX_BYTES] = {
>
> STATIC?
>
>> + 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, lun
>> +// although I'm not sure there are any actual multilun devices anymore (cd autoloaders/etc)
>> +// on ATA the common disk/cdroms generally are single lun devices. So, lets skip the LUN
>> +// scanning/addressing. Futher, the standard ATAPI spec doesn't support lun's.
>> +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;
>> +
>> + 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;
>> +
>> + // look for first device
>> + if (CompareMem (Target8, mScsiId, TARGET_MAX_BYTES) == 0) {
>> + BOOLEAN Found;
>> + INT32 Index;
>> +
>> + Found = FALSE;
>> + Target8 = *Target;
>> + for (Index = 0; ((Index < SATA_SII3132_MAXPORT) && (!Found)); Index++) {
>> + SATA_SI3132_PORT *SataPort;
>> + LIST_ENTRY *List;
>> + INT32 Multiplier;
>> +
>> + SataPort = &(SataInstance->Ports[Index]);
>> + List = SataPort->Devices.ForwardLink;
>> + Multiplier = 0;
>> + while ((List != &SataPort->Devices) && (!Found)) {
>> + SATA_SI3132_DEVICE* SataDevice;
>
> Please move this to the top
>
>> +
>> + 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++;
>> + }
>> + }
>> + } else {
>> + //todo find the next device,
>
> What does this mean? In what way does this limit the current functionality?
I think my original plan was to scan LUNs there but that is sort of
pointless as it actually violates the spec AFAIK.
>
>> + }
>> +
>> + return Status;
>> +}
>> +
>> +EFI_STATUS
>> +SiI3132GetNextTargetLun2 (
>> + IN EFI_EXT_SCSI_PASS_THRU_PROTOCOL *This,
>> + IN UINT8 *Target,
>> + IN UINT64 Lun,
>> + IN OUT EFI_DEVICE_PATH_PROTOCOL **DevicePath
>> + )
>> +{
>> + EFI_STATUS Status;
>> +
>> + Status = EFI_SUCCESS;
>> + DEBUG ((DEBUG_INFO, "SCSI GetNextTargetLun2\n"));
>> + return Status;
>> +}
>> +
>
> This has an OUT parameter, and you are returning success. This means
> the caller may try to dereference the pointer in *DevicePath,
> potentially causing a crash
I think this is a dead function at this point (see next item).
>
>> +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_SUCCESS;
>> + DEBUG ((DEBUG_INFO, "SCSI GetNextTarget T:%d L:%d\n",*Target,Lun));
>> + return Status;
>> +}
>> +
>
> Same here
This sent me digging up the spec, because I actually thought those were
basically IN/OUT parameters and it says that I should actually be
verifying they are !=NULL before writing into them. So, I should tweak
that too. Oddly, it should probably be resetting the target/lun to 0.
This likely is a real bug if more than one ATAPI device is connected.
>
>> +// 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_SUCCESS;
>> + DEBUG ((DEBUG_VERBOSE, "SCSI GetNextTarget\n"));
>> + return Status;
>> +}
>> --
>> 2.9.3
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.