From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 40AEB20958BC8 for ; Thu, 31 Aug 2017 12:04:41 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 795B8C047B95; Thu, 31 Aug 2017 19:07:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 795B8C047B95 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-89.phx2.redhat.com [10.3.116.89]) by smtp.corp.redhat.com (Postfix) with ESMTP id 07A6460317; Thu, 31 Aug 2017 19:07:22 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel References: <1504191674-3949-1-git-send-email-brijesh.singh@amd.com> <1504191674-3949-3-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: <8a6efa48-273b-e843-30dd-779ff9a2a436@redhat.com> Date: Thu, 31 Aug 2017 21:07:21 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <1504191674-3949-3-git-send-email-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 31 Aug 2017 19:07:24 +0000 (UTC) Subject: Re: [PATCH v3 2/4] OvmfPkg/VirtioScsiDxe: add helper to create a fake host adapter error X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 31 Aug 2017 19:04:41 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/31/17 17:01, Brijesh Singh wrote: > When virtio request fails we return EFI_DEVICE_ERROR, as per the spec > EFI_EXT_SCSI_PASS_THRU_PROTOCOL.PassThru() member function is required > to implement elaborated error reporting. > > The patch refactors out entire block of the code that creates the host > adapter error into a separate helper function (ReportHostAdapterError). > > Suggested-by: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Tom Lendacky > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Reviewed-by: Laszlo Ersek > Signed-off-by: Brijesh Singh > --- > OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 36 ++++++++++++++++---- > 1 file changed, 29 insertions(+), 7 deletions(-) > > diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > index 5e72b1a24b59..5e977c636a0a 100644 > --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > @@ -97,7 +97,7 @@ > // set some fields in the EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET in/out > // parameter on return. The following is a full list of those fields, for > // easier validation of PopulateRequest(), ParseResponse(), and > -// VirtioScsiPassThru() below. > +// ReportHostAdapterError() below. > // > // - InTransferLength > // - OutTransferLength > @@ -387,6 +387,33 @@ ParseResponse ( > return EFI_DEVICE_ERROR; > } > > +/** > + * The function can be used to create a fake host adapter error. > + * > + * When VirtioScsiPassThru is failed due to some reasons then this function > + * can be called to contstruct a host adapter error. > + * > + * @param[out] Packet The Extended SCSI Pass Thru Protocol packet that > + * the host adapter error shall be placed in. > + * > + * @retval EFI_DEVICE_ERROR The function returns this status code > + * unconditionally, to be propagated by > + * VirtioScsiPassThru(). > + */ (1) I hoped that you'd just copy & paste the comment block from my v2 review -- in v3, the bar of "*" characters to the left has not been removed (it should have been, it is not edk2 coding style), plus the "contstruct" typo has not been fixed, etc. I'll update the comment block before pushing. Thanks, Laszlo > +STATIC > +EFI_STATUS > +ReportHostAdapterError ( > + OUT EFI_EXT_SCSI_PASS_THRU_SCSI_REQUEST_PACKET *Packet > + ) > +{ > + Packet->InTransferLength = 0; > + Packet->OutTransferLength = 0; > + Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER; > + Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD; > + Packet->SenseDataLength = 0; > + return EFI_DEVICE_ERROR; > +} > + > > // > // The next seven functions implement EFI_EXT_SCSI_PASS_THRU_PROTOCOL > @@ -472,12 +499,7 @@ VirtioScsiPassThru ( > // > if (VirtioFlush (Dev->VirtIo, VIRTIO_SCSI_REQUEST_QUEUE, &Dev->Ring, > &Indices, NULL) != EFI_SUCCESS) { > - Packet->InTransferLength = 0; > - Packet->OutTransferLength = 0; > - Packet->HostAdapterStatus = EFI_EXT_SCSI_STATUS_HOST_ADAPTER_OTHER; > - Packet->TargetStatus = EFI_EXT_SCSI_STATUS_TARGET_GOOD; > - Packet->SenseDataLength = 0; > - return EFI_DEVICE_ERROR; > + return ReportHostAdapterError (Packet); > } > > return ParseResponse (Packet, &Response); >