Re: [edk2-devel] [PATCH v2 15/17] OvmfPkg/PvScsiDxe: Support sending SCSI request and receive response

["Laszlo Ersek" <lersek@redhat.com>]

On 03/27/20 23:17, Liran Alon wrote:

> 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.
> 

(1) The commit that added the "ScsiIoDevice->ExtScsiSupport" branch to ScsiExecuteSCSICommand() was 70c94b3b6ddf ("Porting R8's PI-enabled ScsiBus driver", 2007-07-02).

$ git show -U100 70c94b3b6ddf

At that time, the ScsiExecuteSCSICommand() function had the following documentation:

     EFI_SUCCESS           - The SCSI Request Packet was sent by the host 
                             successfully, and TransferLength bytes were 
                             transferred to/from DataBuffer.See 
                             HostAdapterStatus, TargetStatus, 
                             SenseDataLength, and SenseData in that order
                             for additional status information.
     EFI_WARN_BUFFER_TOO_SMALL - 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.

No "EFI_BAD_BUFFER_SIZE".

So commit 70c94b3b6ddf was not entirely correct, because after it, the ScsiExecuteSCSICommand() could return EFI_BAD_BUFFER_SIZE (propagating it from ScsiIoDevice->ExtScsiPassThru->PassThru()), but the function's documentation was not updated.

(At the same commit, "MdePkg/Include/Protocol/ScsiPassThruExt.h" already specified EFI_BAD_BUFFER_SIZE as "The SCSI Request Packet was not executed" -- that had come from commit d1f950002362, "Checked in the Protocols introduced in UEFI/PI.", 2007-06-19.)


(2) In commit f36d6e669c97 (2007-09-20), the leading comment block on ScsiExecuteSCSICommand() was updated. The EFI_WARN_BUFFER_TOO_SMALL retval disappeared, and the incorrect EFI_BAD_BUFFER_SIZE language ("The SCSI Request Packet was executed") appeared. (Incorrect for the "EXT" passthru, anyway.)

Interestingly, in this commit message, we see:

    3. Correctify some return status to sync with newest Uefi Spec 2.1

However, in the UEFI 2.0 spec <http://www.uefi.org/sites/default/files/resources/UEFI_Specification_2_and_Errata_Sept16_08.pdf>, EFI_BAD_BUFFER_SIZE is already defined as "The SCSI Request Packet was not executed".

So I think it's a bug in edk2 (at least a documentation bug).

Thanks
Laszlo