From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.15377.1594125982875117902 for ; Tue, 07 Jul 2020 05:46:23 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=H8+HZCml; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1594125982; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Dx+ZhaS8IV0sNH9LlDyZf90gqPaVtDev43lLS8Ro0/E=; b=H8+HZCmlbc9cb1JxSf5VqeICw1eg0/3ugMchs/jfPsWo4LQl/wggkjFpUPsksuIP9WCksn UumLuB1VFI80Ay1PcjQ/d5xdnBYDlsL80PmMzU6AWK+11Hxa9z58uwIGYtjVR9vLI68s0J pb2gmmzc26C0ajUO2TYve4fwNMyuvIw= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-368-VQxj4HvaPIWGfHEQJibKNA-1; Tue, 07 Jul 2020 08:46:18 -0400 X-MC-Unique: VQxj4HvaPIWGfHEQJibKNA-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CCA88107ACCD; Tue, 7 Jul 2020 12:46:16 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-90.ams2.redhat.com [10.36.114.90]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6972770119; Tue, 7 Jul 2020 12:46:15 +0000 (UTC) Subject: Re: [PATCH 10/11] OvmfPkg/LsiScsiDxe: Process the SCSI Request Packet To: Gary Lin , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel References: <20200701040448.14871-1-glin@suse.com> <20200701040448.14871-11-glin@suse.com> From: "Laszlo Ersek" Message-ID: <79b57aa5-c531-0953-2485-fef4b52005db@redhat.com> Date: Tue, 7 Jul 2020 14:46:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200701040448.14871-11-glin@suse.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 07/01/20 06:04, Gary Lin wrote: > This is the second part of LsiScsiPassThru(). LsiScsiProcessRequest() is > added to translate the SCSI Request Packet into the LSI 53C895A > commands. This function utilizes the so-called Script buffer to transmit > a series of commands to the chip and then polls the DMA Status (DSTAT) > register until the Scripts Interrupt Instruction Received (SIR) bit > sets. Once the script is done, the SCSI Request Packet will be modified > to reflect the result of the script. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Gary Lin > --- > OvmfPkg/Include/IndustryStandard/LsiScsi.h | 39 +++ > OvmfPkg/LsiScsiDxe/LsiScsi.c | 308 +++++++++++++++++++++ > OvmfPkg/LsiScsiDxe/LsiScsi.h | 21 ++ > OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf | 3 + > OvmfPkg/OvmfPkg.dec | 3 + > 5 files changed, 374 insertions(+) > > diff --git a/OvmfPkg/Include/IndustryStandard/LsiScsi.h b/OvmfPkg/Include/IndustryStandard/LsiScsi.h > index 60e527f1c6a7..cbf049c18310 100644 > --- a/OvmfPkg/Include/IndustryStandard/LsiScsi.h > +++ b/OvmfPkg/Include/IndustryStandard/LsiScsi.h > @@ -26,6 +26,18 @@ > #define LSI_REG_SIST0 0x42 > #define LSI_REG_SIST1 0x43 > > +// > +// The status bits for DMA Status (DSTAT) > +// > +#define LSI_DSTAT_IID 0x01 > +#define LSI_DSTAT_R 0x02 > +#define LSI_DSTAT_SIR 0x04 > +#define LSI_DSTAT_SSI 0x08 > +#define LSI_DSTAT_ABRT 0x10 > +#define LSI_DSTAT_BF 0x20 > +#define LSI_DSTAT_MDPE 0x40 > +#define LSI_DSTAT_DFE 0x80 > + (1) Please use the BITx macros. > // > // The status bits for Interrupt Status Zero (ISTAT0) > // > @@ -38,4 +50,31 @@ > #define LSI_ISTAT0_SRST 0x40 > #define LSI_ISTAT0_ABRT 0x80 > > +// > +// LSI 53C895A Script Instructions > +// > +#define LSI_INS_TYPE_BLK 0x00000000 > +#define LSI_INS_TYPE_IO 0x40000000 > +#define LSI_INS_TYPE_TC 0x80000000 > + > +#define LSI_INS_BLK_SCSIP_DAT_OUT 0x00000000 > +#define LSI_INS_BLK_SCSIP_DAT_IN 0x01000000 > +#define LSI_INS_BLK_SCSIP_CMD 0x02000000 > +#define LSI_INS_BLK_SCSIP_STAT 0x03000000 > +#define LSI_INS_BLK_SCSIP_MSG_OUT 0x06000000 > +#define LSI_INS_BLK_SCSIP_MSG_IN 0x07000000 > + > +#define LSI_INS_IO_OPC_SEL 0x00000000 > +#define LSI_INS_IO_OPC_WAIT_RESEL 0x10000000 > + > +#define LSI_INS_TC_CP 0x00020000 > +#define LSI_INS_TC_JMP 0x00080000 > +#define LSI_INS_TC_RA 0x00800000 > + > +#define LSI_INS_TC_OPC_JMP 0x00000000 > +#define LSI_INS_TC_OPC_INT 0x18000000 > + > +#define LSI_INS_TC_SCSIP_DAT_OUT 0x00000000 > +#define LSI_INS_TC_SCSIP_MSG_IN 0x07000000 > + > #endif // _LSI_SCSI_H_ > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsi.c b/OvmfPkg/LsiScsiDxe/LsiScsi.c > index 1bcebd92e455..090d7df15b34 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsi.c > +++ b/OvmfPkg/LsiScsiDxe/LsiScsi.c > @@ -43,6 +43,42 @@ Out8 ( > ); > } > > +STATIC > +EFI_STATUS > +Out32 ( > + IN LSI_SCSI_DEV *Dev, > + IN UINT32 Addr, > + IN UINT32 Data > + ) > +{ > + return Dev->PciIo->Io.Write ( > + Dev->PciIo, > + EfiPciIoWidthUint32, > + PCI_BAR_IDX0, > + Addr, > + 1, > + &Data > + ); > +} > + > +STATIC > +EFI_STATUS > +In8 ( > + IN LSI_SCSI_DEV *Dev, > + IN UINT32 Addr, > + OUT UINT8 *Data > + ) > +{ > + return Dev->PciIo->Io.Read ( > + Dev->PciIo, > + EfiPciIoWidthUint8, > + PCI_BAR_IDX0, > + Addr, > + 1, > + Data > + ); > +} > + > STATIC > EFI_STATUS > LsiScsiReset ( > @@ -141,6 +177,272 @@ LsiScsiCheckRequest ( > return EFI_SUCCESS; > } > > +/** > + > + Interpret the request packet from the Extended SCSI Pass Thru Protocol and > + compose the script to submit the command and data to the contorller. (2) s/contorller/controller/ > + > + @param[in] Dev The LSI 53C895A SCSI device the packet targets. > + > + @param[in] Target The SCSI target controlled by the LSI 53C895A SCSI > + device. > + > + @param[in] Lun The Logical Unit Number under the SCSI target. > + > + @param[in out] Packet The Extended SCSI Pass Thru Protocol packet. > + > + > + @retval EFI_SUCCESS The Extended SCSI Pass Thru Protocol packet was valid. > + > + @return Otherwise, invalid or unsupported parameters were > + detected. Status codes are meant for direct forwarding > + by the EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() > + implementation. > + > + **/ > +STATIC > +EFI_STATUS > +LsiScsiProcessRequest ( > + IN LSI_SCSI_DEV *Dev, > + IN UINT8 Target, > + IN UINT64 Lun, > + IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet > + ) > +{ > + EFI_STATUS Status; > + UINT32 *Script; > + UINT8 *Cdb; > + UINT8 *MsgOut; > + UINT8 *MsgIn; > + UINT8 *ScsiStatus; > + UINT8 *Data; > + UINT8 DStat; > + UINT8 SIst0; > + UINT8 SIst1; > + > + Script = Dev->Dma->Script; > + Cdb = Dev->Dma->Cdb; > + Data = Dev->Dma->Data; > + MsgIn = Dev->Dma->MsgIn; > + MsgOut = &Dev->Dma->MsgOut; > + ScsiStatus = &Dev->Dma->Status; > + > + *ScsiStatus = 0xFF; > + > + SetMem (Cdb, sizeof Dev->Dma->Cdb, 0x00); > + CopyMem (Cdb, Packet->Cdb, Packet->CdbLength); > + > + // > + // Clean up the DMA buffer for the script. > + // > + SetMem (Script, sizeof Dev->Dma->Script, 0x00); > + > + // > + // Compose the script to transfer data between the host and the device. > + // > + // Reference: > + // LSI53C895A PCI to Ultra2 SCSI Controller Version 2.2 > + // - Chapter 5 SCSI SCRIPT Instruction Set > + // > + // All instructions used here consist of 2 32bit words. The first word > + // contains the command to execute. The second word is loaded into the > + // DMA SCRIPTS Pointer Save (DSPS) register as either the DMA address > + // for data transmission or the address/offset for the jump command. > + // Some commands, such as the selection of the target, don't need to > + // transfer data through DMA or jump to another instruction, then DSPS > + // has to be zero. > + // > + // There are 3 major parts in this script. The first part (1~3) contains > + // the instructions to select target and LUN and send the SCSI command > + // from the request packet. The second part (4~7) is to handle the > + // potential disconnection and prepare for the data transmission. The > + // instructions in the third part (8~10) transmit the given data and > + // collect the result. Instruction 11 raises the interrupt and marks the > + // end of the script. > + // > + > + // > + // 1. Select target. > + // > + *Script++ = LSI_INS_TYPE_IO | LSI_INS_IO_OPC_SEL | (UINT32)Target << 16; > + *Script++ = 0x00000000; > + > + // > + // 2. Select LUN. > + // > + *MsgOut = 0x80 | (UINT8) Lun; // 0x80: Identify bit This implies that the maximum (inclusive) LUN can be 127. (3) I suggest adding a STATIC_ASSERT() to LsiScsiControllerStart(), where you fetch "PcdLsiScsiMaxLunLimit". (If the device has a stricter limit on LUNs than 127, then please use that limit.) > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_MSG_OUT | \ > + sizeof Dev->Dma->MsgOut; (4) Please cast the result of the "sizeof" operator to UINT32 explicitly. (5) Please drop the backslash; it is not needed. Note that (4) and (5) apply to multiple locations in this function; please update each location in the composition of the script. > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, MsgOut); This opcode seems to mean that the device is not capable of taking the LUN selector message from outside of the 32-bit address space. (6) If that's the case, then please remove DUAL_ADDRESS_CYCLE from "[PATCH 08/11] OvmfPkg/LsiScsiDxe: Map DMA buffer". Otherwise, AllocateBuffer() could theoretically allocate the buffer structure (including the MsgOut member) such that LSI_SCSI_DMA_ADDR_LOW() would truncate the address. (7) Once you remove DUAL_ADDRESS_CYCLE (i.e. once we guarantee that the allocation will be satisfied from the 32-bit address space), you can drop the _LOW suffix as well, from the macro name. Now, if the device is actually 64-bit capable, only it would take a more complex script, and you don't want that, that's fine -- the above steps remain the same. (IOW it doesn't matter whether the device is 64-bit uncapable, or you don't want to write a 64-bit aware command script -- you should remove DUAL_ADDRESS_CYCLE just the same.) > + > + // > + // 3. Send the SCSI Command. > + // > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_CMD | \ > + sizeof Dev->Dma->Cdb; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, Cdb); > + > + // > + // 4. Check whether the current SCSI phase is "Message In" or not > + // and jump to 8 if it is. > + // > + *Script++ = LSI_INS_TYPE_TC | LSI_INS_TC_OPC_JMP | \ > + LSI_INS_TC_SCSIP_MSG_IN | LSI_INS_TC_RA | \ > + LSI_INS_TC_CP; > + *Script++ = 0x00000018; (8) The constant 0x18 is obscure. Can you extend the comment? How is "step 8" connected to value 0x18? (Step 8 starts at command offset 7 in the script, meaning UINT32 offset 14, or byte offset 56. But 0x18 is 24, not matching any of those constants.) > + > + // > + // 5. Read "Message" from the initiator to trigger reselect. > + // > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_MSG_IN | \ > + sizeof Dev->Dma->MsgIn; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, MsgIn); > + > + // > + // 6. Wait reselect. > + // > + *Script++ = LSI_INS_TYPE_IO | LSI_INS_IO_OPC_WAIT_RESEL; > + *Script++ = 0x00000000; > + > + // > + // 7. Read "Message" from the initiator again > + // > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_MSG_IN | \ > + sizeof Dev->Dma->MsgIn; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, MsgIn); > + > + // > + // 8. Set the DMA command for the read/write operations. > + // > + if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ && > + Packet->InTransferLength > 0) { > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_DAT_IN | \ > + Packet->InTransferLength; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, Data); > + } else if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_WRITE && > + Packet->OutTransferLength > 0) { > + // LsiScsiCheckRequest() guarantees that OutTransferLength is no > + // larger than sizeof Dev->Dma->Data, so we can safely copy the > + // the data to Dev->Dma->Data. > + CopyMem (Data, Packet->OutDataBuffer, Packet->OutTransferLength); > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_DAT_OUT | \ > + Packet->OutTransferLength; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, Data); > + } The comment is not bad, but it could be improved. First, the comment style is not 100% correct (empty "//" lines are missing before and after.) Second, instead of a long comment, I'd suggest a short comment, plus an ASSERT. Third, the transfer length in the packet ("in" and "out" alike) isn't *only* relevant due to buffer overflow concerns, but also because the command opcode has some bits set (in particular, LSI_INS_BLK_SCSIP_DAT_IN) that effectively limit the transfer length to a bitmask that's narrower than a UINT32. Another observation is that control can flow through this snippet without taking *either* branch. (For example if the data direction is WRITE, but OutTransferLength is zero. That's a condition that LsiScsiCheckRequest() does not catch. My point is not that LsiScsiCheckRequest() should catch it -- I think it's not an invalid request --, but that LsiScsiProcessRequest() should deal with it.) If we take neither branch, then (minimally) the jump address under step 4. will be wrong -- there's not going to be a DAT_IN / DAT_OUT command to jump to. (9) So, for addressing all of the above, I suggest: // // 8. Set the DMA command for the read/write operations. // // LsiScsiCheckRequest() prevents both integer overflows in the command // opcodes, and buffer overflows. // if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) { ASSERT (Packet->InTransferLength <= sizeof Dev->Dma->Data); *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_DAT_IN | Packet->InTransferLength; } else { ASSERT (Packet->OutTransferLength <= sizeof Dev->Dma->Data); CopyMem (Data, Packet->OutDataBuffer, Packet->OutTransferLength); *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_DAT_OUT | Packet->OutTransferLength; } *Script++ = LSI_SCSI_DMA_ADDR (Dev, Data); > + > + // > + // 9. Get the SCSI status. > + // > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_STAT | \ > + sizeof Dev->Dma->Status; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, Status); > + > + // > + // 10. Get the SCSI message. > + // > + *Script++ = LSI_INS_TYPE_BLK | LSI_INS_BLK_SCSIP_MSG_IN | \ > + sizeof Dev->Dma->MsgIn; > + *Script++ = LSI_SCSI_DMA_ADDR_LOW (Dev, MsgIn); > + > + // > + // 11. Raise the interrupt to end the script. > + // > + *Script++ = LSI_INS_TYPE_TC | LSI_INS_TC_OPC_INT | \ > + LSI_INS_TC_SCSIP_DAT_OUT | LSI_INS_TC_JMP; > + *Script++ = 0x00000000; > + > + // > + // Make sure the size of the script doesn't exceed the buffer. > + // > + ASSERT (Script < Dev->Dma->Script + sizeof Dev->Dma->Script); (10) Please replace "<" with "<=". (11) Please replace "sizeof" with ARRAY_SIZE(). > + > + // > + // The controller starts to execute the script once the DMA Script > + // Pointer (DSP) register is set. > + // > + Status = Out32 (Dev, LSI_REG_DSP, LSI_SCSI_DMA_ADDR_LOW(Dev, Script)); (12) Missing space after "LSI_SCSI_DMA_ADDR". > + if (EFI_ERROR (Status)) { > + return Status; > + } This return statement doesn't seem right. We do not translate the error code to one of the error codes specified for EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru(). We also do not set any output field in Packet. (13) I think we should jump to the "Error" label from here. > + > + // > + // Poll the device registers (DSTAT, SIST0, and SIST1) until the SIR > + // bit sets. > + // > + for(;;) { > + Status = In8 (Dev, LSI_REG_DSTAT, &DStat); > + if (EFI_ERROR (Status)) { > + goto Error; > + } > + Status = In8 (Dev, LSI_REG_SIST0, &SIst0); > + if (EFI_ERROR (Status)) { > + goto Error; > + } > + Status = In8 (Dev, LSI_REG_SIST1, &SIst1); > + if (EFI_ERROR (Status)) { > + goto Error; > + } > + > + if (SIst0 != 0 || SIst1 != 0) { > + goto Error; > + } > + > + // > + // Check the SIR (SCRIPTS Interrupt Instruction Received) bit. > + // > + if (DStat & LSI_DSTAT_SIR) { > + break; > + } > + > + gBS->Stall (Dev->StallPerPollUsec); > + } > + > + // > + // Check if everything is good. > + // SCSI Message Code 0x00: COMMAND COMPLETE > + // SCSI Status Code 0x00: Good > + // > + if (MsgIn[0] == 0 && *ScsiStatus == 0) { (14) Please negate this condition, and unnest the success branch: if (MsgIn[0] != 0 || *ScsiStatus != 0) { goto Error; } /* ... */ return EFI_SUCCESS; > + // > + // Copy Data to InDataBuffer if necessary. > + // > + if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) { > + CopyMem (Packet->InDataBuffer, Data, Packet->InTransferLength); > + } > + > + // > + // The controller doesn't return sense data when replying "TEST UNIT READY", > + // so we have to set SenseDataLength to 0 to notify ScsiIo to issue > + // "REQUEST SENSE" for the sense data. > + // > + if (Cdb[0] == 0x00) { > + Packet->SenseDataLength = 0; > + } > + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK; > + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD; > + > + return EFI_SUCCESS; > + } The mapping on success does not deal with short transfers (in or out) -- we're supposed to update InTransferLength / OutTransferLength on output. (15) Does the device report short transfers somehow? The spec says about SenseDataLength, "On input, the length in bytes of the SenseData buffer. On output, the number of bytes written to the SenseData buffer". But we only set SenseDataLength conditionally. In case we do *not* set it, the caller may attempt to parse garbage from the "SenseData" buffer. (16) We should either get real sense data back to the caller, or always zero out "SenseDataLength". > + > +Error: > + DEBUG((DEBUG_VERBOSE, "%a: dstat: %02X, sist0: %02X, sist1: %02X\n", > + __FUNCTION__, DStat, SIst0, SIst1)); (17) missing space after "DEBUG". (18) The line starting with __FUNCTION__ is not correctly indented. > + // > + // Update the request packet to reflect the status. > + // > + if (*ScsiStatus != 0xFF) { > + Packet->TargetStatus = *ScsiStatus; > + } else { > + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_TASK_ABORTED; > + } > + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER; > + Packet->InTransferLength = 0; > + Packet->OutTransferLength = 0; > + Packet->SenseDataLength = 0; > + > + return EFI_DEVICE_ERROR; > +} > + > // > // The next seven functions implement EFI_EXT_SCSI_PASS_THRU_PROTOCOL > // for the LSI 53C895A SCSI Controller. Refer to UEFI Spec 2.3.1 + Errata C, > @@ -168,6 +470,11 @@ LsiScsiPassThru ( > return Status; > } > > + Status = LsiScsiProcessRequest (Dev, *Target, Lun, Packet); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > return EFI_SUCCESS; > } > (19) I think this can be simplified -- just return Status after calling LsiScsiProcessRequest(). > @@ -469,6 +776,7 @@ LsiScsiControllerStart ( > > Dev->MaxTarget = PcdGet8 (PcdLsiScsiMaxTargetLimit); > Dev->MaxLun = PcdGet8 (PcdLsiScsiMaxLunLimit); > + Dev->StallPerPollUsec = PcdGet32 (PcdLsiScsiStallPerPollUsec); > > Status = gBS->OpenProtocol ( > ControllerHandle, > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsi.h b/OvmfPkg/LsiScsiDxe/LsiScsi.h > index 9272eb7506c7..1a16ef9f7795 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsi.h > +++ b/OvmfPkg/LsiScsiDxe/LsiScsi.h > @@ -13,6 +13,11 @@ > #define _LSI_SCSI_DXE_H_ > > typedef struct { > + // > + // Allocate 32 UINT32 entries for the script and it's sufficient for > + // 16 instructions. > + // > + UINT32 Script[32]; > // > // The max size of CDB is 32. > // > @@ -21,6 +26,18 @@ typedef struct { > // Allocate 64KB for read/write buffer. > // > UINT8 Data[0x10000]; > + // > + // For SCSI Message In phase > + // > + UINT8 MsgIn[2]; > + // > + // For SCSI Message Out phase > + // > + UINT8 MsgOut; > + // > + // For SCSI Status phase > + // > + UINT8 Status; > } LSI_SCSI_DMA_BUFFER; > > typedef struct { > @@ -30,6 +47,7 @@ typedef struct { > EFI_PCI_IO_PROTOCOL *PciIo; > UINT8 MaxTarget; > UINT8 MaxLun; > + UINT32 StallPerPollUsec; > LSI_SCSI_DMA_BUFFER *Dma; > EFI_PHYSICAL_ADDRESS DmaPhysical; > VOID *DmaMapping; > @@ -42,6 +60,9 @@ typedef struct { > #define LSI_SCSI_FROM_PASS_THRU(PassThruPtr) \ > CR (PassThruPtr, LSI_SCSI_DEV, PassThru, LSI_SCSI_DEV_SIGNATURE) > > +#define LSI_SCSI_DMA_ADDR_LOW(Dev, MemberName) \ > + ((UINT32)(Dev->DmaPhysical + OFFSET_OF (LSI_SCSI_DMA_BUFFER, MemberName))) > + > > // > // Probe, start and stop functions of this driver, called by the DXE core for > diff --git a/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf b/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf > index 68844c6772e3..cbd7294573ac 100644 > --- a/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf > +++ b/OvmfPkg/LsiScsiDxe/LsiScsiDxe.inf > @@ -41,3 +41,6 @@ [Protocols] > [FixedPcd] > gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxTargetLimit ## CONSUMES > gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxLunLimit ## CONSUMES > + > +[Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiStallPerPollUsec ## CONSUMES > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index ae7d1d648d22..57e418d4b8a9 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -179,6 +179,9 @@ [PcdsFixedAtBuild] > gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxTargetLimit|7|UINT8|0x3b > gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiMaxLunLimit|0|UINT8|0x3c > > + ## Microseconds to stall between polling for LsiScsi request result > + gUefiOvmfPkgTokenSpaceGuid.PcdLsiScsiStallPerPollUsec|5|UINT32|0x3d > + (20) The token value should be 0x3c here, according to my request (5) under "[PATCH 06/11] OvmfPkg/LsiScsiDxe: Report Targets and LUNs". > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa > Thanks Laszlo