Machines should be able to boot after this commit. Tested with different
Linux distributions (Ubuntu, CentOS) and different Windows
versions (Windows 7, Windows 10, Server 2016).
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
---
.../Include/IndustryStandard/FusionMptScsi.h | 9 +
OvmfPkg/MptScsiDxe/MptScsi.c | 409 +++++++++++++++++-
OvmfPkg/MptScsiDxe/MptScsiDxe.inf | 3 +
OvmfPkg/OvmfPkg.dec | 3 +
4 files changed, 422 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h b/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
index 655d629d902e..99778d1537da 100644
--- a/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
+++ b/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
@@ -44,6 +44,15 @@
#define MPT_IOC_WHOINIT_ROM_BIOS 0x02
+#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_NONE (0x00 << 24)
+#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_WRITE (0x01 << 24)
+#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_READ (0x02 << 24)
+
+#define MPT_SCSI_IOCSTATUS_SUCCESS 0x0000
+#define MPT_SCSI_IOCSTATUS_DEVICE_NOT_THERE 0x0043
+#define MPT_SCSI_IOCSTATUS_DATA_OVERRUN 0x0044
+#define MPT_SCSI_IOCSTATUS_DATA_UNDERRUN 0x0045
+
//
// Device structures
//
diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
index 15d671b544c2..9cb5088bfbf9 100644
--- a/OvmfPkg/MptScsiDxe/MptScsi.c
+++ b/OvmfPkg/MptScsiDxe/MptScsi.c
@@ -17,6 +17,7 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/UefiLib.h>
#include <Protocol/PciIo.h>
+#include <Protocol/PciRootBridgeIo.h>
#include <Protocol/ScsiPassThruExt.h>
#include <Uefi/UefiSpec.h>
@@ -30,19 +31,50 @@
// Runtime Structures
//
+typedef struct {
+ MPT_SCSI_REQUEST_ALIGNED IoRequest;
+ MPT_SCSI_IO_REPLY_ALIGNED IoReply;
+ //
+ // As EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET.SenseDataLength is defined
+ // as UINT8, defining here SenseData size to MAX_UINT8 will guarantee it
+ // cannot overflow when passed to device.
+ //
+ UINT8 Sense[MAX_UINT8];
+ //
+ // This size of the data is arbitrarily chosen.
+ // It seems to be sufficient for all I/O requests sent through
+ // EFI_SCSI_PASS_THRU_PROTOCOL.PassThru() for common boot scenarios.
+ //
+ UINT8 Data[0x2000];
+} MPT_SCSI_DMA_BUFFER;
+
#define MPT_SCSI_DEV_SIGNATURE SIGNATURE_32 ('M','P','T','S')
typedef struct {
UINT32 Signature;
EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode;
UINT8 MaxTarget;
+ UINT32 StallPerPollUsec;
EFI_PCI_IO_PROTOCOL *PciIo;
UINT64 OriginalPciAttributes;
+ MPT_SCSI_DMA_BUFFER *Dma;
+ EFI_PHYSICAL_ADDRESS DmaPhysical;
+ VOID *DmaMapping;
+ BOOLEAN IoReplyEnqueued;
} MPT_SCSI_DEV;
#define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \
CR (PassThruPtr, MPT_SCSI_DEV, PassThru, MPT_SCSI_DEV_SIGNATURE)
+#define MPT_SCSI_DMA_ADDR(Dev, MemberName) \
+ (Dev->DmaPhysical + OFFSET_OF (MPT_SCSI_DMA_BUFFER, MemberName))
+
+#define MPT_SCSI_DMA_ADDR_HIGH(Dev, MemberName) \
+ ((UINT32)(MPT_SCSI_DMA_ADDR (Dev, MemberName) >> 32))
+
+#define MPT_SCSI_DMA_ADDR_LOW(Dev, MemberName) \
+ ((UINT32)MPT_SCSI_DMA_ADDR (Dev, MemberName))
+
//
// Hardware functions
//
@@ -157,6 +189,9 @@ MptScsiInit (
"Req supports 255 targets only (max target is 254)");
Req.MaxDevices = Dev->MaxTarget + 1;
Req.MaxBuses = 1;
+ Req.ReplyFrameSize = sizeof Dev->Dma->IoReply.Data;
+ Req.HostMfaHighAddr = MPT_SCSI_DMA_ADDR_HIGH (Dev, IoRequest);
+ Req.SenseBufferHighAddr = MPT_SCSI_DMA_ADDR_HIGH (Dev, Sense);
//
// Send controller init through doorbell
@@ -218,6 +253,289 @@ MptScsiInit (
return EFI_SUCCESS;
}
+STATIC
+EFI_STATUS
+ReportHostAdapterError (
+ OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ DEBUG ((DEBUG_ERROR, "%a: fatal error in scsi request\n", __FUNCTION__));
+ Packet->InTransferLength = 0;
+ Packet->OutTransferLength = 0;
+ Packet->SenseDataLength = 0;
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_TASK_ABORTED;
+ return EFI_DEVICE_ERROR;
+}
+
+STATIC
+EFI_STATUS
+ReportHostAdapterOverrunError (
+ OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ Packet->SenseDataLength = 0;
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
+ return EFI_BAD_BUFFER_SIZE;
+}
+
+STATIC
+EFI_STATUS
+MptScsiPopulateRequest (
+ IN MPT_SCSI_DEV *Dev,
+ IN UINT8 Target,
+ IN UINT64 Lun,
+ IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ MPT_SCSI_REQUEST_WITH_SG *Request;
+
+ Request = &Dev->Dma->IoRequest.Data;
+
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_BIDIRECTIONAL ||
+ (Packet->InTransferLength > 0 && Packet->OutTransferLength > 0) ||
+ Packet->CdbLength > sizeof (Request->Header.Cdb)) {
+ return EFI_UNSUPPORTED;
+ }
+
+ if (Target > Dev->MaxTarget || Lun > 0 ||
+ Packet->DataDirection > EFI_EXT_SCSI_DATA_DIRECTION_BIDIRECTIONAL ||
+ //
+ // Trying to receive, but destination pointer is NULL, or contradicting
+ // transfer direction
+ //
+ (Packet->InTransferLength > 0 &&
+ (Packet->InDataBuffer == NULL ||
+ Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_WRITE
+ )
+ ) ||
+
+ //
+ // Trying to send, but source pointer is NULL, or contradicting transfer
+ // direction
+ //
+ (Packet->OutTransferLength > 0 &&
+ (Packet->OutDataBuffer == NULL ||
+ Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ
+ )
+ )
+ ) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if (Packet->InTransferLength > sizeof (Dev->Dma->Data)) {
+ Packet->InTransferLength = sizeof (Dev->Dma->Data);
+ return ReportHostAdapterOverrunError (Packet);
+ }
+ if (Packet->OutTransferLength > sizeof (Dev->Dma->Data)) {
+ Packet->OutTransferLength = sizeof (Dev->Dma->Data);
+ return ReportHostAdapterOverrunError (Packet);
+ }
+
+ ZeroMem (Request, sizeof (*Request));
+ Request->Header.TargetId = Target;
+ //
+ // Only LUN 0 is currently supported, hence the cast is safe
+ //
+ Request->Header.Lun[1] = (UINT8)Lun;
+ Request->Header.Function = MPT_MESSAGE_HDR_FUNCTION_SCSI_IO_REQUEST;
+ Request->Header.MessageContext = 1; // We handle one request at a time
+
+ Request->Header.CdbLength = Packet->CdbLength;
+ CopyMem (Request->Header.Cdb, Packet->Cdb, Packet->CdbLength);
+
+ //
+ // SenseDataLength is UINT8, Sense[] is MAX_UINT8, so we can't overflow
+ //
+ ZeroMem (Dev->Dma->Sense, Packet->SenseDataLength);
+ Request->Header.SenseBufferLength = Packet->SenseDataLength;
+ Request->Header.SenseBufferLowAddress = MPT_SCSI_DMA_ADDR_LOW (Dev, Sense);
+
+ Request->Sg.EndOfList = 1;
+ Request->Sg.EndOfBuffer = 1;
+ Request->Sg.LastElement = 1;
+ Request->Sg.ElementType = MPT_SG_ENTRY_TYPE_SIMPLE;
+ Request->Sg.Is64BitAddress = 1;
+ Request->Sg.DataBufferAddress = MPT_SCSI_DMA_ADDR (Dev, Data);
+
+ //
+ // "MPT_SG_ENTRY_SIMPLE.Length" is a 24-bit quantity.
+ //
+ STATIC_ASSERT (
+ sizeof (Dev->Dma->Data) < SIZE_16MB,
+ "MPT_SCSI_DMA_BUFFER.Data must be smaller than 16MB"
+ );
+
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ Request->Header.DataLength = Packet->InTransferLength;
+ Request->Sg.Length = Packet->InTransferLength;
+ Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_READ;
+ } else {
+ Request->Header.DataLength = Packet->OutTransferLength;
+ Request->Sg.Length = Packet->OutTransferLength;
+ Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_WRITE;
+
+ CopyMem (Dev->Dma->Data, Packet->OutDataBuffer, Packet->OutTransferLength);
+ Request->Sg.BufferContainsData = 1;
+ }
+
+ if (Request->Header.DataLength == 0) {
+ Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_NONE;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+MptScsiSendRequest (
+ IN MPT_SCSI_DEV *Dev,
+ IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ EFI_STATUS Status;
+
+ if (!Dev->IoReplyEnqueued) {
+ //
+ // Put one free reply frame on the reply queue, the hardware may use it to
+ // report an error to us.
+ //
+ Status = Out32 (Dev, MPT_REG_REP_Q, MPT_SCSI_DMA_ADDR_LOW (Dev, IoReply));
+ if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
+ }
+ Dev->IoReplyEnqueued = TRUE;
+ }
+
+ Status = Out32 (Dev, MPT_REG_REQ_Q, MPT_SCSI_DMA_ADDR_LOW (Dev, IoRequest));
+ if (EFI_ERROR (Status)) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+MptScsiGetReply (
+ IN MPT_SCSI_DEV *Dev,
+ OUT UINT32 *Reply
+ )
+{
+ EFI_STATUS Status;
+ UINT32 Istatus;
+ UINT32 EmptyReply;
+
+ //
+ // Timeouts are not supported for
+ // EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() in this implementation.
+ //
+ for (;;) {
+ Status = In32 (Dev, MPT_REG_ISTATUS, &Istatus);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Interrupt raised
+ //
+ if (Istatus & MPT_IMASK_REPLY) {
+ break;
+ }
+
+ gBS->Stall (Dev->StallPerPollUsec);
+ }
+
+ Status = In32 (Dev, MPT_REG_REP_Q, Reply);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // The driver is supposed to fetch replies until 0xffffffff is returned, which
+ // will reset the interrupt status. We put only one request, so we expect the
+ // next read reply to be the last.
+ //
+ Status = In32 (Dev, MPT_REG_REP_Q, &EmptyReply);
+ if (EFI_ERROR (Status) || EmptyReply != MAX_UINT32) {
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+MptScsiHandleReply (
+ IN MPT_SCSI_DEV *Dev,
+ IN UINT32 Reply,
+ OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
+ )
+{
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ CopyMem (Packet->InDataBuffer, Dev->Dma->Data, Packet->InTransferLength);
+ }
+
+ if (Reply == Dev->Dma->IoRequest.Data.Header.MessageContext) {
+ //
+ // This is a turbo reply, everything is good
+ //
+ Packet->SenseDataLength = 0;
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
+ Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
+
+ } else if ((Reply & BIT31) != 0) {
+ DEBUG ((DEBUG_INFO, "%a: Full reply returned\n", __FUNCTION__));
+ //
+ // When reply MSB is set, we got a full reply. Since we submitted only one
+ // reply frame, we know it's IoReply.
+ //
+ Dev->IoReplyEnqueued = FALSE;
+
+ Packet->TargetStatus = Dev->Dma->IoReply.Data.ScsiStatus;
+ //
+ // Make sure device only lowers SenseDataLength before copying sense
+ //
+ ASSERT (Dev->Dma->IoReply.Data.SenseCount <= Packet->SenseDataLength);
+ Packet->SenseDataLength =
+ (UINT8)MIN (Dev->Dma->IoReply.Data.SenseCount, Packet->SenseDataLength);
+ CopyMem (Packet->SenseData, Dev->Dma->Sense, Packet->SenseDataLength);
+
+ if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
+ Packet->InTransferLength = Dev->Dma->IoReply.Data.TransferCount;
+ } else {
+ Packet->OutTransferLength = Dev->Dma->IoReply.Data.TransferCount;
+ }
+
+ switch (Dev->Dma->IoReply.Data.IocStatus) {
+ case MPT_SCSI_IOCSTATUS_SUCCESS:
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
+ break;
+ case MPT_SCSI_IOCSTATUS_DEVICE_NOT_THERE:
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_SELECTION_TIMEOUT;
+ return EFI_TIMEOUT;
+ case MPT_SCSI_IOCSTATUS_DATA_UNDERRUN:
+ case MPT_SCSI_IOCSTATUS_DATA_OVERRUN:
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
+ break;
+ default:
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
+ return EFI_DEVICE_ERROR;
+ }
+
+ } else {
+ DEBUG ((DEBUG_ERROR, "%a: unexpected reply (%x)\n", __FUNCTION__, Reply));
+ Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
+ return EFI_DEVICE_ERROR;
+ }
+
+ return EFI_SUCCESS;
+}
+
//
// Ext SCSI Pass Thru
//
@@ -233,7 +551,33 @@ MptScsiPassThru (
IN EFI_EVENT Event OPTIONAL
)
{
- return EFI_UNSUPPORTED;
+ EFI_STATUS Status;
+ MPT_SCSI_DEV *Dev;
+ UINT32 Reply;
+
+ Dev = MPT_SCSI_FROM_PASS_THRU (This);
+ //
+ // We only use first byte of target identifer
+ //
+ Status = MptScsiPopulateRequest (Dev, *Target, Lun, Packet);
+ if (EFI_ERROR (Status)) {
+ //
+ // MptScsiPopulateRequest modified packet according to the error
+ //
+ return Status;
+ }
+
+ Status = MptScsiSendRequest (Dev, Packet);
+ if (EFI_ERROR (Status)) {
+ return ReportHostAdapterError (Packet);
+ }
+
+ Status = MptScsiGetReply (Dev, &Reply);
+ if (EFI_ERROR (Status)) {
+ return ReportHostAdapterError (Packet);
+ }
+
+ return MptScsiHandleReply (Dev, Reply, Packet);
}
STATIC
@@ -488,6 +832,8 @@ MptScsiControllerStart (
{
EFI_STATUS Status;
MPT_SCSI_DEV *Dev;
+ UINTN Pages;
+ UINTN BytesMapped;
Dev = AllocateZeroPool (sizeof (*Dev));
if (Dev == NULL) {
@@ -497,6 +843,7 @@ MptScsiControllerStart (
Dev->Signature = MPT_SCSI_DEV_SIGNATURE;
Dev->MaxTarget = PcdGet8 (PcdMptScsiMaxTargetLimit);
+ Dev->StallPerPollUsec = PcdGet32 (PcdMptScsiStallPerPollUsec);
Status = gBS->OpenProtocol (
ControllerHandle,
@@ -557,11 +904,45 @@ MptScsiControllerStart (
));
}
- Status = MptScsiInit (Dev);
+ //
+ // Create buffers for data transfer
+ //
+ Pages = EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma));
+ Status = Dev->PciIo->AllocateBuffer (
+ Dev->PciIo,
+ AllocateAnyPages,
+ EfiBootServicesData,
+ Pages,
+ (VOID **)&Dev->Dma,
+ EFI_PCI_ATTRIBUTE_MEMORY_CACHED
+ );
if (EFI_ERROR (Status)) {
goto RestoreAttributes;
}
+ BytesMapped = EFI_PAGES_TO_SIZE (Pages);
+ Status = Dev->PciIo->Map (
+ Dev->PciIo,
+ EfiPciIoOperationBusMasterCommonBuffer,
+ Dev->Dma,
+ &BytesMapped,
+ &Dev->DmaPhysical,
+ &Dev->DmaMapping
+ );
+ if (EFI_ERROR (Status)) {
+ goto FreeBuffer;
+ }
+
+ if (BytesMapped != EFI_PAGES_TO_SIZE (Pages)) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Unmap;
+ }
+
+ Status = MptScsiInit (Dev);
+ if (EFI_ERROR (Status)) {
+ goto Unmap;
+ }
+
//
// Host adapter channel, doesn't exist
//
@@ -594,6 +975,19 @@ MptScsiControllerStart (
UninitDev:
MptScsiReset (Dev);
+Unmap:
+ Dev->PciIo->Unmap (
+ Dev->PciIo,
+ Dev->DmaMapping
+ );
+
+FreeBuffer:
+ Dev->PciIo->FreeBuffer (
+ Dev->PciIo,
+ EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma)),
+ Dev->Dma
+ );
+
RestoreAttributes:
Dev->PciIo->Attributes (
Dev->PciIo,
@@ -655,6 +1049,17 @@ MptScsiControllerStop (
MptScsiReset (Dev);
+ Dev->PciIo->Unmap (
+ Dev->PciIo,
+ Dev->DmaMapping
+ );
+
+ Dev->PciIo->FreeBuffer (
+ Dev->PciIo,
+ EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma)),
+ Dev->Dma
+ );
+
Dev->PciIo->Attributes (
Dev->PciIo,
EfiPciIoAttributeOperationSet,
diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
index 4862ff9dd497..bb89e50e08f0 100644
--- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
+++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
@@ -37,3 +37,6 @@ [Protocols]
[FixedPcd]
gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit ## CONSUMES
+
+[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec ## CONSUMES
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 2d09444bbb16..3bf26e8df82d 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -167,6 +167,9 @@ [PcdsFixedAtBuild]
# by ScsiBusDxe.
gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit|7|UINT8|0x39
+ ## Microseconds to stall between polling for MptScsi request result
+ gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec|5|UINT32|0x40
+
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa
--
2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58086): https://edk2.groups.io/g/devel/message/58086
Mute This Topic: https://groups.io/mt/73247273/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 04/24/20 19:59, Nikita Leshenko wrote:
> Machines should be able to boot after this commit. Tested with different
> Linux distributions (Ubuntu, CentOS) and different Windows
> versions (Windows 7, Windows 10, Server 2016).
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2390
> Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
> ---
> .../Include/IndustryStandard/FusionMptScsi.h | 9 +
> OvmfPkg/MptScsiDxe/MptScsi.c | 409 +++++++++++++++++-
> OvmfPkg/MptScsiDxe/MptScsiDxe.inf | 3 +
> OvmfPkg/OvmfPkg.dec | 3 +
> 4 files changed, 422 insertions(+), 2 deletions(-)
>
> diff --git a/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h b/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
> index 655d629d902e..99778d1537da 100644
> --- a/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
> +++ b/OvmfPkg/Include/IndustryStandard/FusionMptScsi.h
> @@ -44,6 +44,15 @@
>
> #define MPT_IOC_WHOINIT_ROM_BIOS 0x02
>
> +#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_NONE (0x00 << 24)
> +#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_WRITE (0x01 << 24)
> +#define MPT_SCSIIO_REQUEST_CONTROL_TXDIR_READ (0x02 << 24)
> +
> +#define MPT_SCSI_IOCSTATUS_SUCCESS 0x0000
> +#define MPT_SCSI_IOCSTATUS_DEVICE_NOT_THERE 0x0043
> +#define MPT_SCSI_IOCSTATUS_DATA_OVERRUN 0x0044
> +#define MPT_SCSI_IOCSTATUS_DATA_UNDERRUN 0x0045
> +
> //
> // Device structures
> //
> diff --git a/OvmfPkg/MptScsiDxe/MptScsi.c b/OvmfPkg/MptScsiDxe/MptScsi.c
> index 15d671b544c2..9cb5088bfbf9 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsi.c
> +++ b/OvmfPkg/MptScsiDxe/MptScsi.c
> @@ -17,6 +17,7 @@
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/UefiLib.h>
> #include <Protocol/PciIo.h>
> +#include <Protocol/PciRootBridgeIo.h>
> #include <Protocol/ScsiPassThruExt.h>
> #include <Uefi/UefiSpec.h>
>
> @@ -30,19 +31,50 @@
> // Runtime Structures
> //
>
> +typedef struct {
> + MPT_SCSI_REQUEST_ALIGNED IoRequest;
> + MPT_SCSI_IO_REPLY_ALIGNED IoReply;
> + //
> + // As EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET.SenseDataLength is defined
> + // as UINT8, defining here SenseData size to MAX_UINT8 will guarantee it
> + // cannot overflow when passed to device.
> + //
> + UINT8 Sense[MAX_UINT8];
> + //
> + // This size of the data is arbitrarily chosen.
> + // It seems to be sufficient for all I/O requests sent through
> + // EFI_SCSI_PASS_THRU_PROTOCOL.PassThru() for common boot scenarios.
> + //
> + UINT8 Data[0x2000];
> +} MPT_SCSI_DMA_BUFFER;
> +
> #define MPT_SCSI_DEV_SIGNATURE SIGNATURE_32 ('M','P','T','S')
> typedef struct {
> UINT32 Signature;
> EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru;
> EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode;
> UINT8 MaxTarget;
> + UINT32 StallPerPollUsec;
> EFI_PCI_IO_PROTOCOL *PciIo;
> UINT64 OriginalPciAttributes;
> + MPT_SCSI_DMA_BUFFER *Dma;
> + EFI_PHYSICAL_ADDRESS DmaPhysical;
> + VOID *DmaMapping;
> + BOOLEAN IoReplyEnqueued;
> } MPT_SCSI_DEV;
>
> #define MPT_SCSI_FROM_PASS_THRU(PassThruPtr) \
> CR (PassThruPtr, MPT_SCSI_DEV, PassThru, MPT_SCSI_DEV_SIGNATURE)
>
> +#define MPT_SCSI_DMA_ADDR(Dev, MemberName) \
> + (Dev->DmaPhysical + OFFSET_OF (MPT_SCSI_DMA_BUFFER, MemberName))
> +
> +#define MPT_SCSI_DMA_ADDR_HIGH(Dev, MemberName) \
> + ((UINT32)(MPT_SCSI_DMA_ADDR (Dev, MemberName) >> 32))
(1) Please use the RShiftU64() from BaseLib.
> +
> +#define MPT_SCSI_DMA_ADDR_LOW(Dev, MemberName) \
> + ((UINT32)MPT_SCSI_DMA_ADDR (Dev, MemberName))
> +
> //
> // Hardware functions
> //
> @@ -157,6 +189,9 @@ MptScsiInit (
> "Req supports 255 targets only (max target is 254)");
> Req.MaxDevices = Dev->MaxTarget + 1;
> Req.MaxBuses = 1;
> + Req.ReplyFrameSize = sizeof Dev->Dma->IoReply.Data;
> + Req.HostMfaHighAddr = MPT_SCSI_DMA_ADDR_HIGH (Dev, IoRequest);
> + Req.SenseBufferHighAddr = MPT_SCSI_DMA_ADDR_HIGH (Dev, Sense);
>
> //
> // Send controller init through doorbell
> @@ -218,6 +253,289 @@ MptScsiInit (
> return EFI_SUCCESS;
> }
>
> +STATIC
> +EFI_STATUS
> +ReportHostAdapterError (
> + OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
> + )
> +{
> + DEBUG ((DEBUG_ERROR, "%a: fatal error in scsi request\n", __FUNCTION__));
> + Packet->InTransferLength = 0;
> + Packet->OutTransferLength = 0;
> + Packet->SenseDataLength = 0;
> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
> + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_TASK_ABORTED;
> + return EFI_DEVICE_ERROR;
> +}
> +
> +STATIC
> +EFI_STATUS
> +ReportHostAdapterOverrunError (
> + OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
> + )
> +{
> + Packet->SenseDataLength = 0;
> + Packet->HostAdapterStatus =
> + EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
> + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
> + return EFI_BAD_BUFFER_SIZE;
> +}
> +
> +STATIC
> +EFI_STATUS
> +MptScsiPopulateRequest (
> + IN MPT_SCSI_DEV *Dev,
> + IN UINT8 Target,
> + IN UINT64 Lun,
> + IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
> + )
> +{
> + MPT_SCSI_REQUEST_WITH_SG *Request;
> +
> + Request = &Dev->Dma->IoRequest.Data;
> +
> + if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_BIDIRECTIONAL ||
> + (Packet->InTransferLength > 0 && Packet->OutTransferLength > 0) ||
> + Packet->CdbLength > sizeof (Request->Header.Cdb)) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + if (Target > Dev->MaxTarget || Lun > 0 ||
> + Packet->DataDirection > EFI_EXT_SCSI_DATA_DIRECTION_BIDIRECTIONAL ||
> + //
> + // Trying to receive, but destination pointer is NULL, or contradicting
> + // transfer direction
> + //
> + (Packet->InTransferLength > 0 &&
> + (Packet->InDataBuffer == NULL ||
> + Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_WRITE
> + )
> + ) ||
> +
> + //
> + // Trying to send, but source pointer is NULL, or contradicting transfer
> + // direction
> + //
> + (Packet->OutTransferLength > 0 &&
> + (Packet->OutDataBuffer == NULL ||
> + Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ
> + )
> + )
> + ) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if (Packet->InTransferLength > sizeof (Dev->Dma->Data)) {
> + Packet->InTransferLength = sizeof (Dev->Dma->Data);
> + return ReportHostAdapterOverrunError (Packet);
> + }
> + if (Packet->OutTransferLength > sizeof (Dev->Dma->Data)) {
> + Packet->OutTransferLength = sizeof (Dev->Dma->Data);
> + return ReportHostAdapterOverrunError (Packet);
> + }
> +
> + ZeroMem (Request, sizeof (*Request));
> + Request->Header.TargetId = Target;
> + //
> + // Only LUN 0 is currently supported, hence the cast is safe
> + //
> + Request->Header.Lun[1] = (UINT8)Lun;
> + Request->Header.Function = MPT_MESSAGE_HDR_FUNCTION_SCSI_IO_REQUEST;
> + Request->Header.MessageContext = 1; // We handle one request at a time
> +
> + Request->Header.CdbLength = Packet->CdbLength;
> + CopyMem (Request->Header.Cdb, Packet->Cdb, Packet->CdbLength);
> +
> + //
> + // SenseDataLength is UINT8, Sense[] is MAX_UINT8, so we can't overflow
> + //
> + ZeroMem (Dev->Dma->Sense, Packet->SenseDataLength);
> + Request->Header.SenseBufferLength = Packet->SenseDataLength;
> + Request->Header.SenseBufferLowAddress = MPT_SCSI_DMA_ADDR_LOW (Dev, Sense);
> +
> + Request->Sg.EndOfList = 1;
> + Request->Sg.EndOfBuffer = 1;
> + Request->Sg.LastElement = 1;
> + Request->Sg.ElementType = MPT_SG_ENTRY_TYPE_SIMPLE;
> + Request->Sg.Is64BitAddress = 1;
> + Request->Sg.DataBufferAddress = MPT_SCSI_DMA_ADDR (Dev, Data);
> +
> + //
> + // "MPT_SG_ENTRY_SIMPLE.Length" is a 24-bit quantity.
> + //
> + STATIC_ASSERT (
> + sizeof (Dev->Dma->Data) < SIZE_16MB,
> + "MPT_SCSI_DMA_BUFFER.Data must be smaller than 16MB"
> + );
> +
> + if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
> + Request->Header.DataLength = Packet->InTransferLength;
> + Request->Sg.Length = Packet->InTransferLength;
> + Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_READ;
> + } else {
> + Request->Header.DataLength = Packet->OutTransferLength;
> + Request->Sg.Length = Packet->OutTransferLength;
> + Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_WRITE;
> +
> + CopyMem (Dev->Dma->Data, Packet->OutDataBuffer, Packet->OutTransferLength);
> + Request->Sg.BufferContainsData = 1;
> + }
> +
> + if (Request->Header.DataLength == 0) {
> + Request->Header.Control = MPT_SCSIIO_REQUEST_CONTROL_TXDIR_NONE;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +MptScsiSendRequest (
> + IN MPT_SCSI_DEV *Dev,
> + IN OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
> + )
> +{
> + EFI_STATUS Status;
> +
> + if (!Dev->IoReplyEnqueued) {
> + //
> + // Put one free reply frame on the reply queue, the hardware may use it to
> + // report an error to us.
> + //
> + Status = Out32 (Dev, MPT_REG_REP_Q, MPT_SCSI_DMA_ADDR_LOW (Dev, IoReply));
> + if (EFI_ERROR (Status)) {
> + return EFI_DEVICE_ERROR;
> + }
> + Dev->IoReplyEnqueued = TRUE;
> + }
> +
> + Status = Out32 (Dev, MPT_REG_REQ_Q, MPT_SCSI_DMA_ADDR_LOW (Dev, IoRequest));
> + if (EFI_ERROR (Status)) {
> + return EFI_DEVICE_ERROR;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +MptScsiGetReply (
> + IN MPT_SCSI_DEV *Dev,
> + OUT UINT32 *Reply
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 Istatus;
> + UINT32 EmptyReply;
> +
> + //
> + // Timeouts are not supported for
> + // EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() in this implementation.
> + //
> + for (;;) {
> + Status = In32 (Dev, MPT_REG_ISTATUS, &Istatus);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + //
> + // Interrupt raised
> + //
> + if (Istatus & MPT_IMASK_REPLY) {
> + break;
> + }
> +
> + gBS->Stall (Dev->StallPerPollUsec);
> + }
> +
> + Status = In32 (Dev, MPT_REG_REP_Q, Reply);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + //
> + // The driver is supposed to fetch replies until 0xffffffff is returned, which
> + // will reset the interrupt status. We put only one request, so we expect the
> + // next read reply to be the last.
> + //
> + Status = In32 (Dev, MPT_REG_REP_Q, &EmptyReply);
> + if (EFI_ERROR (Status) || EmptyReply != MAX_UINT32) {
> + return EFI_DEVICE_ERROR;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +MptScsiHandleReply (
> + IN MPT_SCSI_DEV *Dev,
> + IN UINT32 Reply,
> + OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet
> + )
> +{
> + if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
> + CopyMem (Packet->InDataBuffer, Dev->Dma->Data, Packet->InTransferLength);
> + }
> +
> + if (Reply == Dev->Dma->IoRequest.Data.Header.MessageContext) {
> + //
> + // This is a turbo reply, everything is good
> + //
> + Packet->SenseDataLength = 0;
> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
> + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD;
> +
> + } else if ((Reply & BIT31) != 0) {
> + DEBUG ((DEBUG_INFO, "%a: Full reply returned\n", __FUNCTION__));
(2) Is this a frequent event? If we expect this to happen frequently,
then it should be DEBUG_VERBOSE. Otherwise (= infrequent event),
DEBUG_INFO is fine.
> + //
> + // When reply MSB is set, we got a full reply. Since we submitted only one
> + // reply frame, we know it's IoReply.
> + //
> + Dev->IoReplyEnqueued = FALSE;
> +
> + Packet->TargetStatus = Dev->Dma->IoReply.Data.ScsiStatus;
> + //
> + // Make sure device only lowers SenseDataLength before copying sense
> + //
> + ASSERT (Dev->Dma->IoReply.Data.SenseCount <= Packet->SenseDataLength);
> + Packet->SenseDataLength =
> + (UINT8)MIN (Dev->Dma->IoReply.Data.SenseCount, Packet->SenseDataLength);
> + CopyMem (Packet->SenseData, Dev->Dma->Sense, Packet->SenseDataLength);
> +
> + if (Packet->DataDirection == EFI_EXT_SCSI_DATA_DIRECTION_READ) {
> + Packet->InTransferLength = Dev->Dma->IoReply.Data.TransferCount;
> + } else {
> + Packet->OutTransferLength = Dev->Dma->IoReply.Data.TransferCount;
> + }
> +
> + switch (Dev->Dma->IoReply.Data.IocStatus) {
> + case MPT_SCSI_IOCSTATUS_SUCCESS:
> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK;
> + break;
> + case MPT_SCSI_IOCSTATUS_DEVICE_NOT_THERE:
> + Packet->HostAdapterStatus =
> + EFI_EXT_SCSI_STATUS_HOST_ADAPTER_SELECTION_TIMEOUT;
> + return EFI_TIMEOUT;
> + case MPT_SCSI_IOCSTATUS_DATA_UNDERRUN:
> + case MPT_SCSI_IOCSTATUS_DATA_OVERRUN:
> + Packet->HostAdapterStatus =
> + EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
> + break;
> + default:
> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
> + return EFI_DEVICE_ERROR;
> + }
> +
> + } else {
> + DEBUG ((DEBUG_ERROR, "%a: unexpected reply (%x)\n", __FUNCTION__, Reply));
> + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER;
> + return EFI_DEVICE_ERROR;
> + }
I'm really pleased with the updates to this function. In both the "turbo
reply" and "full reply" cases, we make sure that all 6 of the following
are correctly set on output:
- InTransferLength
- OutTransferLength
- HostAdapterStatus
- TargetStatus
- SenseDataLength
- SenseData
(3) I do have a question about the last branch ("unexpected reply")
though -- would you agree that we should call ReportHostAdapterError()
there, rather than only setting HostAdapterStatus?
Because, per EFI_DEVICE_ERROR, the caller will first consult
HostAdapterStatus, yes, but then (upon seeing "OTHER") it will likely
proceed to TargetStatus and Sense. And we currently leave garbage in
those fields.
(I guess I could have made the same comment under v4, but there I was
hung up on the other output fields in the more common branches. All of
those have been nicely covered in v5, so I've arrived at this last branch.)
> +
> + return EFI_SUCCESS;
> +}
> +
> //
> // Ext SCSI Pass Thru
> //
> @@ -233,7 +551,33 @@ MptScsiPassThru (
> IN EFI_EVENT Event OPTIONAL
> )
> {
> - return EFI_UNSUPPORTED;
> + EFI_STATUS Status;
> + MPT_SCSI_DEV *Dev;
> + UINT32 Reply;
> +
> + Dev = MPT_SCSI_FROM_PASS_THRU (This);
> + //
> + // We only use first byte of target identifer
> + //
> + Status = MptScsiPopulateRequest (Dev, *Target, Lun, Packet);
> + if (EFI_ERROR (Status)) {
> + //
> + // MptScsiPopulateRequest modified packet according to the error
> + //
> + return Status;
> + }
> +
> + Status = MptScsiSendRequest (Dev, Packet);
> + if (EFI_ERROR (Status)) {
> + return ReportHostAdapterError (Packet);
> + }
> +
> + Status = MptScsiGetReply (Dev, &Reply);
> + if (EFI_ERROR (Status)) {
> + return ReportHostAdapterError (Packet);
> + }
> +
> + return MptScsiHandleReply (Dev, Reply, Packet);
> }
>
> STATIC
> @@ -488,6 +832,8 @@ MptScsiControllerStart (
> {
> EFI_STATUS Status;
> MPT_SCSI_DEV *Dev;
> + UINTN Pages;
> + UINTN BytesMapped;
>
> Dev = AllocateZeroPool (sizeof (*Dev));
> if (Dev == NULL) {
> @@ -497,6 +843,7 @@ MptScsiControllerStart (
> Dev->Signature = MPT_SCSI_DEV_SIGNATURE;
>
> Dev->MaxTarget = PcdGet8 (PcdMptScsiMaxTargetLimit);
> + Dev->StallPerPollUsec = PcdGet32 (PcdMptScsiStallPerPollUsec);
>
> Status = gBS->OpenProtocol (
> ControllerHandle,
> @@ -557,11 +904,45 @@ MptScsiControllerStart (
> ));
> }
>
> - Status = MptScsiInit (Dev);
> + //
> + // Create buffers for data transfer
> + //
> + Pages = EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma));
> + Status = Dev->PciIo->AllocateBuffer (
> + Dev->PciIo,
> + AllocateAnyPages,
> + EfiBootServicesData,
> + Pages,
> + (VOID **)&Dev->Dma,
> + EFI_PCI_ATTRIBUTE_MEMORY_CACHED
> + );
> if (EFI_ERROR (Status)) {
> goto RestoreAttributes;
> }
>
> + BytesMapped = EFI_PAGES_TO_SIZE (Pages);
> + Status = Dev->PciIo->Map (
> + Dev->PciIo,
> + EfiPciIoOperationBusMasterCommonBuffer,
> + Dev->Dma,
> + &BytesMapped,
> + &Dev->DmaPhysical,
> + &Dev->DmaMapping
> + );
> + if (EFI_ERROR (Status)) {
> + goto FreeBuffer;
> + }
> +
> + if (BytesMapped != EFI_PAGES_TO_SIZE (Pages)) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto Unmap;
> + }
> +
> + Status = MptScsiInit (Dev);
> + if (EFI_ERROR (Status)) {
> + goto Unmap;
> + }
> +
> //
> // Host adapter channel, doesn't exist
> //
> @@ -594,6 +975,19 @@ MptScsiControllerStart (
> UninitDev:
> MptScsiReset (Dev);
>
> +Unmap:
> + Dev->PciIo->Unmap (
> + Dev->PciIo,
> + Dev->DmaMapping
> + );
> +
> +FreeBuffer:
> + Dev->PciIo->FreeBuffer (
> + Dev->PciIo,
> + EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma)),
(4) Not wrong, but still, please simplify this function call by passing
the local variable "Pages" here.
> + Dev->Dma
> + );
> +
> RestoreAttributes:
> Dev->PciIo->Attributes (
> Dev->PciIo,
> @@ -655,6 +1049,17 @@ MptScsiControllerStop (
>
> MptScsiReset (Dev);
>
> + Dev->PciIo->Unmap (
> + Dev->PciIo,
> + Dev->DmaMapping
> + );
> +
> + Dev->PciIo->FreeBuffer (
> + Dev->PciIo,
> + EFI_SIZE_TO_PAGES (sizeof (*Dev->Dma)),
> + Dev->Dma
> + );
> +
> Dev->PciIo->Attributes (
> Dev->PciIo,
> EfiPciIoAttributeOperationSet,
> diff --git a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
> index 4862ff9dd497..bb89e50e08f0 100644
> --- a/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
> +++ b/OvmfPkg/MptScsiDxe/MptScsiDxe.inf
> @@ -37,3 +37,6 @@ [Protocols]
>
> [FixedPcd]
> gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit ## CONSUMES
> +
> +[Pcd]
> + gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec ## CONSUMES
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 2d09444bbb16..3bf26e8df82d 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -167,6 +167,9 @@ [PcdsFixedAtBuild]
> # by ScsiBusDxe.
> gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiMaxTargetLimit|7|UINT8|0x39
>
> + ## Microseconds to stall between polling for MptScsi request result
> + gUefiOvmfPkgTokenSpaceGuid.PcdMptScsiStallPerPollUsec|5|UINT32|0x40
> +
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa
>
With (1) through (4) addressed:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
(Well, (1) is a must, (2) through (4) are open for discussion, of course.)
Thanks!
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58425): https://edk2.groups.io/g/devel/message/58425
Mute This Topic: https://groups.io/mt/73247273/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
> On 30 Apr 2020, at 12:47, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 04/24/20 19:59, Nikita Leshenko wrote:
>> Machines should be able to boot after this commit. Tested with different
>> Linux distributions (Ubuntu, CentOS) and different Windows
>> versions (Windows 7, Windows 10, Server 2016).
>>
>> Ref: https://urldefense.com/v3/__https://bugzilla.tianocore.org/show_bug.cgi?id=2390__;!!GqivPVa7Brio!NV5JGxsOY-LMe9c_r7p1Ks4Jy755ybGf-hewsLI7texgoPDpsKmcin6UpzFlRxCc3tuWtA$
>> Signed-off-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
>> [..]
>> +
>> + } else if ((Reply & BIT31) != 0) {
>> + DEBUG ((DEBUG_INFO, "%a: Full reply returned\n", __FUNCTION__));
>
> (2) Is this a frequent event? If we expect this to happen frequently,
> then it should be DEBUG_VERBOSE. Otherwise (= infrequent event),
> DEBUG_INFO is fine.
In practice this event happens on IO errors, so I assume it won't be frequent,
that's why I put it in INFO.
>
>> [...]
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogBase|0x0|UINT32|0x8
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageEventLogSize|0x0|UINT32|0x9
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFirmwareFdSize|0x0|UINT32|0xa
>>
>
> With (1) through (4) addressed:
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> (Well, (1) is a must, (2) through (4) are open for discussion, of course.)
Agree with (3) and (4). I think that the DEBUG_INFO is fine, but I don't mind
changing it to VERBOSE.
Nikita
>
> Thanks!
> Laszlo
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58568): https://edk2.groups.io/g/devel/message/58568
Mute This Topic: https://groups.io/mt/73247273/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.