From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by mx.groups.io with SMTP id smtpd.web12.47696.1585567405116405114 for ; Mon, 30 Mar 2020 04:23:25 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=E+RnLVxf; spf=pass (domain: redhat.com, ip: 216.205.24.74, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585567404; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6uab/eGSMGX7oasQnIaFvyEBiKW+5kw9EXlHMEUx7F0=; b=E+RnLVxfcRf6uiObsrPPLJ6nL7uvohbUxUFX12BcsUEh9KWaSGiYEQNijbH4IZzSCQyIeS J/zYaxvH3sVyzKrQY4yqvVpcOAbJNIAFpksJ93/ZiKk4xqx0CRkrD4Gd8gXIVGehrRDSMl hPEXYszg1dnAjzEPfMenlKfZSRi80Kg= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-475-GmECilvuN7iIRzd69PuQ8Q-1; Mon, 30 Mar 2020 07:23:18 -0400 X-MC-Unique: GmECilvuN7iIRzd69PuQ8Q-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 05F4913F9; Mon, 30 Mar 2020 11:23:17 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-191.ams2.redhat.com [10.36.112.191]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5A5B748; Mon, 30 Mar 2020 11:23:15 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 15/17] OvmfPkg/PvScsiDxe: Support sending SCSI request and receive response To: Liran Alon , 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> <65eb2511-e2f5-d2b5-4470-278b06a0e077@oracle.com> <9bbdb66f-9fdb-bd5a-74b8-6dc31015dc8a@oracle.com> From: "Laszlo Ersek" Message-ID: <4cc0d542-59cf-aad1-c317-7d9f6a43d342@redhat.com> Date: Mon, 30 Mar 2020 13:23:14 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <9bbdb66f-9fdb-bd5a-74b8-6dc31015dc8a@oracle.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 03/28/20 20:18, Liran Alon wrote: > Sorry for the spam but I think I eventually figured it out. It's not spam; thank you for the thorough investigation! > The call-chain in EDK2 looks like this: > ScsiDiskWriteSectors() (MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c) > ScsiDiskWrite16() (MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c) > ScsiWrite16Command() (MdePkg/Library/UefiScsiLib/UefiScsiLib.c) > ScsiExecuteSCSICommand() (MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBus.c) > ExtScsiPassThru->PassThru() >=20 > It can be seen that: > * ScsiExecuteSCSICommand() just passes Packet to > ExtScsiPassThru->PassThru() and returns it's return value. > * ScsiWrite16Command() always updates DataLength with > Packet->OutTransferLength and returns ScsiExecuteSCSICommand() return > value. > * In ScsiDiskWrite16(): > =A0 ** If EFI_BAD_BUFFER_SIZE is returned, NeedRetry is set to TRUE and > EFI_DEVICE_ERROR is returned. > =A0 ** Otherwise, if EFI_SUCCESS is returned but HostAdapterStatus is set > to EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN, then again > NeedRetry is set to TRUE and EFI_DEVICE_ERROR is returned. > =A0 ** Otherwise, just return EFI_SUCCESS to caller. > * In ScsiDiskWriteSectors() we finally see the logic we were looking for: > =A0 ** If ScsiDiskWrite16() returned EFI_SUCCESS, then we break out of > retry loop and indeed take into account ByteCount. (Which was updated > from Packet->OutTransferLength). > =A0 ** Otherwise, (e.g. EFI_BAD_BUFFER_SIZE was returned), if NeedRetry i= s > set to TRUE, then next request retry is updated to be done with > ByteCount (Which was updated from Packet->OutTransferLength). >=20 > So it seems the UEFI-2.8 spec is right and a lot of EDK2 function > documentation is out-of-date. On a tangent... Since you mention ScsiDiskWriteSectors(), let me mention co= mmit 5abc2a70da4f ("MdeModulePkg: ScsiDiskDxe: adapt SectorCount when short= ening transfers", 2015-09-10), and the related mailing list discussion (thi= s is what I referred to before, when I mentioned that Paolo had looked at t= he VirtioScsiDxe code twice -- this is the second occasion, from September = 2015): http://mid.mail-archive.com/1441390936-27763-1-git-send-email-lersek@redhat= .com > Therefore, I will update my code accordingly. i.e.: > 1) Change PvScsi.c PopulateRequest() such that in case TransferLength is > too big for DMA communication buffer Data field, we update > TransferLength and return EFI_BAD_BUFFER_SIZE. Sounds OK. > =A0=A0=A0 But in addition, for completion, we should also set > HostAdapterStatus to > EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN and TargetStatus > to EFI_EXT_SCSI_STATUS_TARGET_GOOD > =A0=A0=A0 and SenseDataLength to 0 as done in VirtioScsi.c PopulateReques= t() > in case it returns EFI_BAD_BUFFER_SIZE. OK, thanks. > 2) Change PvScsi.c HandleResponse() such that: > =A0 ** If device returned PvScsiBtStatDataUnderrun: Update TransferLength > with Response->DataLen, set HostAdapterStatus to > EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN and return > EFI_SUCCESS. OK. > =A0 ** If device returned PvScsiBtStatDatarun: Just set HostAdapterStatus > to EFI_EXT_SCSI_STATUS_HOST_ADAPTER_DATA_OVERRUN_UNDERRUN and return > EFI_SUCCESS. Again I'm not familiar with PvScsiBtStatDataUnderrun / PvScsiBtStatDatarun = (let alone their differences); I'm OK with this too. Thanks! Laszlo