On 28/03/2020 1:04, Liran Alon wrote:
On 28/03/2020 0:05, Laszlo Ersek wrote:
On 03/27/20 14:04, Liran Alon wrote:
On 27/03/2020 14:26, Laszlo Ersek wrote:
On 03/25/20 17:10, Liran Alon wrote:
+ Packet->HostAdapterStatus =
+ EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN;
+ return EFI_BAD_BUFFER_SIZE;
I think EFI_BAD_BUFFER_SIZE is invalid here. According to the UEFI
spec,
EFI_BAD_BUFFER_SIZE means "The SCSI Request Packet was not executed".
But that's not the case here -- we do have a partially completed
transfer.
Hmm... According to the documentation above EFI_SCSI_PASS_THRU_PASSTHRU
in MdePkg/Include/Protocol/ScsiPassThru.h:
@retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was
executed, but the
entire DataBuffer could not be
transferred.
The actual number of bytes
transferred is returned
in TransferLength. See
HostAdapterStatus,
TargetStatus, SenseDataLength, and
SenseData in
that order for additional status
information.
So I don't know who to believe... It does seem to me that this
documentation in the code makes more sense
and then my current code is correct. What do you think?
You are looking at the wrong protocol header file.
Oops. You are right.
The top of this
header file bears the comment
SCSI Pass Through protocol as defined in EFI 1.1.
and the UEFI-2.8 spec does not define EFI_SCSI_PASS_THRU_PROTOCOL; it
only refers to Mantis ticket 845
<https://urldefense.com/v3/__https://mantis.uefi.org/mantis/view.php?id=845__;!!GqivPVa7Brio!M5o2BC3kQvh7gEgQh98Kb7YJeFCM_ajknF8iRXjDer3Ir8hUy6dHOReS12oCRvE$
> with subject
"EFI_SCSI_PASS_THRU_PROTOCOL replacement".
Instead, please consult EFI_EXT_SCSI_PASS_THRU_PASSTHRU in
"MdePkg/Include/Protocol/ScsiPassThruExt.h". There, the
EFI_BAD_BUFFER_SIZE return value conforms to the UEFI 2.8 spec ("The
SCSI Request Packet was not executed").
Honestly, I'm not sure if this is not a bug in UEFI-2.8 spec and
ScsiPassThruExt.h header file. It doesn't make sense.
In the new header file, there is no way for PassThru() to indicate to
caller that a partial data-transfer have occurred.
Which seems to be a quite standard functionality. Almost all SCSI
devices are able to return information on partial data-transfer
completions.
Looking at QEMU's virtio-scsi.c virtio_scsi_command_complete()
implementation, we can see how it handles partial data-transfer
completions.
It sets Response->Response to VIRTIO_SCSI_S_OK and Response->Residual
accordingly.
Looking at EDK2 VirtioScsi driver, function ParseResponse(), this will
be handled by updating Packet->InTransferLength and
Packet->OutTransferLength based on Response->Residual,
but then setting Packet->HostAdapterStatus to
EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK and returning EFI_SUCCESS.
(In contrast to your suggestion for me to return EFI_DEVICE_ERROR in
this case).
The documentation of PassThru in ScsiPassThruExt.h for EFI_SUCCESS
specifies:
@retval EFI_SUCCESS The SCSI Request Packet was sent by
the host. For bi-directional
commands, InTransferLength bytes were
transferred from
InDataBuffer. For write and
bi-directional commands,
OutTransferLength bytes were
transferred by
OutDataBuffer.
But it seems to reference the InTransferLength and OutTransferLength
*before* the call to PassThru().
In contrast to old header file, ScsiPassThru.h, which for
EFI_BAD_BUFFER_SIZE specifies explicitly:
@retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was
executed, but the
entire DataBuffer could not be
transferred.
The actual number of bytes
transferred is returned
in TransferLength. See
HostAdapterStatus,
TargetStatus, SenseDataLength, and
SenseData in
that order for additional status
information.
In addition, looking at EDK2 iSCSI ScsiPassThru driver
(NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c), one can see it use
EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET
(Which matches the new header file ScsiPassThruExt.h and not the old
ScsiPassThru.h).
If you would have a look at how IScsiOnScsiRspRcvd() handles
SCSI_RSP_PDU_FLAG_BI_READ_OVERFLOW and SCSI_RSP_PDU_FLAG_OVERFLOW
bits, it seems to always
set Status to EFI_BAD_BUFFER_SIZE as I have done. After updating
Packet->InTransferLength / Packet->OutTransferLength.
What do you think? Do you have any further insights? This is quite
confusing to me.
-Liran
Furthermore, I have later found ScsiExecuteSCSICommand() in
MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c to be the one that calls the
PassThru() method.
Looking at it's "if (ScsiIoDevice->ExtScsiSupport)" branch (Which is
relevant to us), one can see it just simply executes the PassThru()
device and returns.
Examining ScsiExecuteSCSICommand() documentation specifies for
EFI_BAD_BUFFER_SIZE:
@retval EFI_BAD_BUFFER_SIZE The SCSI Request Packet was executed,
but the entire DataBuffer could not be
transferred.
The actual number of bytes transferred is
returned
in TransferLength. See HostAdapterStatus,
TargetStatus, SenseDataLength, and
SenseData in
that order for additional status information.
Which again seems to align to what I have done in my PvScsi driver...
-Liran
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56517): https://edk2.groups.io/g/devel/message/56517
Mute This Topic: https://groups.io/mt/72544127/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
