From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from userp2120.oracle.com (userp2120.oracle.com [156.151.31.85]) by mx.groups.io with SMTP id smtpd.web12.93.1585346700438014016 for ; Fri, 27 Mar 2020 15:05:00 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@oracle.com header.s=corp-2020-01-29 header.b=y2a/qdBR; spf=pass (domain: oracle.com, ip: 156.151.31.85, mailfrom: liran.alon@oracle.com) Received: from pps.filterd (userp2120.oracle.com [127.0.0.1]) by userp2120.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02RM4uwi141356; Fri, 27 Mar 2020 22:04:59 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=vOgEkNQYlWyG04USdIErg52vzBl56M4MHZs6KVUl2sI=; b=y2a/qdBRiNuZsaCsigjGKgySXwwBsr8MyDImNBIKRB95qUuLUnw7LxRqq1vrMAt2K8+s 7eEqIwSRDsFIDdJDBmwCHxK5/nY7cLzdhfajZW/y32HgZtWuEwOYhFtX40IQ2wvPiIy0 BrZI6fGVOgYt0ZvIaddQwVaePHsSr0N2zivilwkTNbmKG+pqGYmIZal1G4fiD4ijgtaO bH6jcVl9emDR2s9242LQIK7RXmu8q6gtE+SFjQjHfp/pHkgbb1g75adh1Y4JFULUBu4A 5+c/yLT3cX+ioL/Yajj/YluXAkL/oEuc3J5uCep5A447fuATxp1eX29NOb4f3b+0uF1w aQ== Received: from userp3030.oracle.com (userp3030.oracle.com [156.151.31.80]) by userp2120.oracle.com with ESMTP id 3019vecm6c-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 27 Mar 2020 22:04:59 +0000 Received: from pps.filterd (userp3030.oracle.com [127.0.0.1]) by userp3030.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 02RM2rZ0149241; Fri, 27 Mar 2020 22:04:59 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userp3030.oracle.com with ESMTP id 2yxw4wphju-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 27 Mar 2020 22:04:59 +0000 Received: from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id 02RM4vJO025793; Fri, 27 Mar 2020 22:04:57 GMT Received: from [192.168.14.112] (/79.180.216.197) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 27 Mar 2020 15:04:57 -0700 Subject: Re: [edk2-devel] [PATCH v2 15/17] OvmfPkg/PvScsiDxe: Support sending SCSI request and receive response To: Laszlo Ersek , devel@edk2.groups.io Cc: nikita.leshchenko@oracle.com, aaron.young@oracle.com, jordan.l.justen@intel.com, ard.biesheuvel@linaro.org References: <20200325161005.16743-1-liran.alon@oracle.com> <20200325161005.16743-16-liran.alon@oracle.com> <6f32838a-d136-050e-7a05-f817da7954a8@oracle.com> From: "Liran Alon" Message-ID: Date: Sat, 28 Mar 2020 01:04:52 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9573 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 suspectscore=0 spamscore=0 mlxlogscore=999 adultscore=0 phishscore=0 mlxscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2003270181 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9573 signatures=668685 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxscore=0 impostorscore=0 phishscore=0 clxscore=1015 adultscore=0 spamscore=0 malwarescore=0 mlxlogscore=999 lowpriorityscore=0 bulkscore=0 priorityscore=1501 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2003020000 definitions=main-2003270181 X-MIME-Autoconverted: from 8bit to quoted-printable by userp2120.oracle.com id 02RM4uwi141356 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable 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: >>>> +=A0=A0=A0=A0=A0 Packet->HostAdapterStatus =3D >>>> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 EFI_EXT_SCSI_STATUS_H= OST_ADAPTER_DATA_OVERRUN_UNDERRUN; >>>> +=A0=A0=A0=A0=A0 return EFI_BAD_BUFFER_SIZE; >>> I think EFI_BAD_BUFFER_SIZE is invalid here. According to the UEFI sp= ec, >>> 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_PASSTHR= U >> in MdePkg/Include/Protocol/ScsiPassThru.h: >> >> =A0 @retval EFI_BAD_BUFFER_SIZE=A0=A0=A0=A0=A0=A0 The SCSI Request Pa= cket was >> executed, but the >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 entire DataBuffer could not be >> transferred. >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 The actual number of bytes >> transferred is returned >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 in TransferLength. See >> HostAdapterStatus, >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 TargetStatus, SenseDataLength, and >> SenseData in >> =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 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 > 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=20 ScsiPassThruExt.h header file. It doesn't make sense. In the new header file, there is no way for PassThru() to indicate to=20 caller that a partial data-transfer have occurred. Which seems to be a quite standard functionality. Almost all SCSI=20 devices are able to return information on partial data-transfer completio= ns. Looking at QEMU's virtio-scsi.c virtio_scsi_command_complete()=20 implementation, we can see how it handles partial data-transfer completio= ns. It sets Response->Response to VIRTIO_SCSI_S_OK and Response->Residual=20 accordingly. Looking at EDK2 VirtioScsi driver, function ParseResponse(), this will=20 be handled by updating Packet->InTransferLength and=20 Packet->OutTransferLength based on Response->Residual, but then setting Packet->HostAdapterStatus to=20 EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OK and returning EFI_SUCCESS. (In contrast to your suggestion for me to return EFI_DEVICE_ERROR in=20 this case). The documentation of PassThru in ScsiPassThruExt.h for EFI_SUCCESS=20 specifies: =A0 @retval EFI_SUCCESS=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 The SCSI Request P= acket was sent by the=20 host. For bi-directional =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 commands, InTransferLength bytes were=20 transferred from =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 InDataBuffer. For write and=20 bi-directional commands, =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 OutTransferLength bytes were transferred by =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0 OutDataBuffer. But it seems to reference the InTransferLength and OutTransferLength=20 *before* the call to PassThru(). In contrast to old header file, ScsiPassThru.h, which for=20 EFI_BAD_BUFFER_SIZE specifies explicitly: =A0@retval EFI_BAD_BUFFER_SIZE=A0=A0=A0=A0=A0=A0 The SCSI Request Packet= was=20 executed, but the =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 entire DataBuffer could not be=20 transferred. =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 The actual number of bytes=20 transferred is returned =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 in TransferLength. See=20 HostAdapterStatus, =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 TargetStatus, SenseDataLength, and=20 SenseData in =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 that order for additional status=20 information. In addition, looking at EDK2 iSCSI ScsiPassThru driver=20 (NetworkPkg/IScsiDxe/IScsiExtScsiPassThru.c), one can see it use=20 EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET (Which matches the new header file ScsiPassThruExt.h and not the old=20 ScsiPassThru.h). If you would have a look at how IScsiOnScsiRspRcvd() handles=20 SCSI_RSP_PDU_FLAG_BI_READ_OVERFLOW and SCSI_RSP_PDU_FLAG_OVERFLOW bits,=20 it seems to always set Status to EFI_BAD_BUFFER_SIZE as I have done. After updating=20 Packet->InTransferLength / Packet->OutTransferLength. What do you think? Do you have any further insights? This is quite=20 confusing to me. -Liran