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 67D752095AE4C for ; Thu, 31 Aug 2017 15:07:59 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BA938552FE; Thu, 31 Aug 2017 22:10:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BA938552FE Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.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 21AC3619D4; Thu, 31 Aug 2017 22:10:40 +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> From: Laszlo Ersek Message-ID: Date: Fri, 1 Sep 2017 00:10:40 +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-1-git-send-email-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Thu, 31 Aug 2017 22:10:42 +0000 (UTC) Subject: Re: [PATCH v3 0/4] OvmfPkg/VirtioScsiDxe: map host address to device address 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 22:07:59 -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: > The patch updates VirtioScsiDxe to use IOMMU-like member functions to map > the system physical address to device address for buffers (including vring, > device specific request and response pointed by vring descriptor, and any > furter memory reference by those request and response). > > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Tom Lendacky > Cc: Laszlo Ersek > > Repo: https://github.com/codomania/edk2 > Branch: virtio-scsi-3 > > Changes since v2: > * changes to address v2 feedback > > Brijesh Singh (4): > OvmfPkg/VirtioScsiDxe: map VRING using VirtioRingMap() > OvmfPkg/VirtioScsiDxe: add helper to create a fake host adapter error > OvmfPkg/VirtioScsiDxe: map virtio-scsi request and response buffers > OvmfPkg/VirtioScsiDxe: negotiate VIRTIO_F_IOMMU_PLATFORM > > OvmfPkg/VirtioScsiDxe/VirtioScsi.h | 1 + > OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 306 +++++++++++++++++--- > 2 files changed, 273 insertions(+), 34 deletions(-) > Okay, so these are the updates (cumulatively) that I implemented here: > diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > index 313b4f66c7f8..7b8c3d22c8de 100644 > --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > @@ -387,19 +387,23 @@ 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(). > - */ > + > + 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 construct 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(). > + > +**/ > STATIC > EFI_STATUS > ReportHostAdapterError ( > @@ -490,14 +494,15 @@ VirtioScsiPassThru ( > // * we perform the request fine > // * but we fail to unmap the "InDataMapping" > // > - // In that case simply returing the EFI_DEVICE_ERROR is not sufficient. > - // In addition to the error code we also need to update Packet fields > - // accordingly so that we report the full loss of the incoming transfer. > - // We allocate a temporary buffer and map it with BusMasterCommonBuffer. > - // If the Virtio request is successful then we copy the data from > - // temporary buffer into Packet->InDataBuffer. > + // In that case simply returing the EFI_DEVICE_ERROR is not sufficient. In > + // addition to the error code we also need to update Packet fields > + // accordingly so that we report the full loss of the incoming transfer. > // > - InDataNumPages = (UINTN)EFI_SIZE_TO_PAGES (Packet->InTransferLength); > + // We allocate a temporary buffer and map it with BusMasterCommonBuffer. If > + // the Virtio request is successful then we copy the data from temporary > + // buffer into Packet->InDataBuffer. > + // > + InDataNumPages = EFI_SIZE_TO_PAGES ((UINTN)Packet->InTransferLength); > Status = Dev->VirtIo->AllocateSharedPages ( > Dev->VirtIo, > InDataNumPages, To patch #2, I added the following editing remark: [lersek@redhat.com: fix style & typo in ReportHostAdapterError() comment] To patch #3, I added the following editing remarks: [lersek@redhat.com: restore lost sentence/paragraph in commit message] [lersek@redhat.com: reindent/reflow "InDataBuffer" comment block] [lersek@redhat.com: cast arg, not result, of EFI_SIZE_TO_PAGES() to UINTN] With the above, the series is now fully Reviewed-by me. I tested this series as follows: virtio-mmio legacy PCI modern PCI ----------- ----------------- -------------------------- test scenario AARCH64 IA32 X64 IA32 X64/SEV- X64/SEV+ ---------------- ----------- -------- -------- -------- -------- -------- R/W text editor PASS PASS PASS PASS PASS PASS shell RECONNECT PASS PASS PASS PASS PASS PASS OS boot from it PASS PASS PASS PASS PASS PASS Therefore, for the series: Regression-tested-by: Laszlo Ersek Tested-by: Laszlo Ersek Pushed as commit range fefeb416e63b..17cbf7359f04. Thanks, Laszlo