[edk2-devel] [PATCH v2 15/17] OvmfPkg/PvScsiDxe: Support sending SCSI request and receive response
Liran Alon
liran.alon at oracle.com
Fri Mar 27 22:04:52 UTC 2020
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56516): https://edk2.groups.io/g/devel/message/56516
Mute This Topic: https://groups.io/mt/72544127/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list