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 F203521E11D0D for ; Tue, 29 Aug 2017 05:36:33 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D2C464E4D3; Tue, 29 Aug 2017 12:39:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D2C464E4D3 Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx09.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-166.phx2.redhat.com [10.3.116.166]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3A6056887A; Tue, 29 Aug 2017 12:39:12 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel References: <1503919610-26185-1-git-send-email-brijesh.singh@amd.com> <1503919610-26185-2-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: Date: Tue, 29 Aug 2017 14:39:11 +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: <1503919610-26185-2-git-send-email-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 29 Aug 2017 12:39:14 +0000 (UTC) Subject: Re: [PATCH 1/3] OvmfPkg/VirtioScsiDxe: map VRING using VirtioRingMap() 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: Tue, 29 Aug 2017 12:36:34 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/28/17 13:26, Brijesh Singh wrote: > When device is behind the IOMMU then driver need to pass the device > address when programing the bus master. The patch uses VirtioRingMap() to > map the VRING system physical address to device address. > > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Tom Lendacky > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh > --- > OvmfPkg/VirtioScsiDxe/VirtioScsi.h | 1 + > OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 47 +++++++++++++++----- > 2 files changed, 38 insertions(+), 10 deletions(-) Reviewed-by: Laszlo Ersek Thanks, Laszlo > > diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h > index 6d00567e8cb8..05a6bf567263 100644 > --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.h > +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.h > @@ -60,6 +60,7 @@ typedef struct { > VRING Ring; // VirtioRingInit 2 > EFI_EXT_SCSI_PASS_THRU_PROTOCOL PassThru; // VirtioScsiInit 1 > EFI_EXT_SCSI_PASS_THRU_MODE PassThruMode; // VirtioScsiInit 1 > + VOID *RingMap; // VirtioRingMap 2 > } VSCSI_DEV; > > #define VIRTIO_SCSI_FROM_PASS_THRU(PassThruPointer) \ > diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > index a983b3df7b9c..5e72b1a24b59 100644 > --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > @@ -707,7 +707,7 @@ VirtioScsiInit ( > { > UINT8 NextDevStat; > EFI_STATUS Status; > - > + UINT64 RingBaseShift; > UINT64 Features; > UINT16 MaxChannel; // for validation only > UINT32 NumQueues; // for validation only > @@ -839,25 +839,42 @@ VirtioScsiInit ( > } > > // > + // If anything fails from here on, we must release the ring resources > + // > + Status = VirtioRingMap ( > + Dev->VirtIo, > + &Dev->Ring, > + &RingBaseShift, > + &Dev->RingMap > + ); > + if (EFI_ERROR (Status)) { > + goto ReleaseQueue; > + } > + > + // > // Additional steps for MMIO: align the queue appropriately, and set the > - // size. If anything fails from here on, we must release the ring resources. > + // size. If anything fails from here on, we must unmap the ring resources. > // > Status = Dev->VirtIo->SetQueueNum (Dev->VirtIo, QueueSize); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > > Status = Dev->VirtIo->SetQueueAlign (Dev->VirtIo, EFI_PAGE_SIZE); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > > // > // step 4c -- Report GPFN (guest-physical frame number) of queue. > // > - Status = Dev->VirtIo->SetQueueAddress (Dev->VirtIo, &Dev->Ring, 0); > + Status = Dev->VirtIo->SetQueueAddress ( > + Dev->VirtIo, > + &Dev->Ring, > + RingBaseShift > + ); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > > // > @@ -867,7 +884,7 @@ VirtioScsiInit ( > Features &= ~(UINT64)VIRTIO_F_VERSION_1; > Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > } > > @@ -877,11 +894,11 @@ VirtioScsiInit ( > // > Status = VIRTIO_CFG_WRITE (Dev, CdbSize, VIRTIO_SCSI_CDB_SIZE); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > Status = VIRTIO_CFG_WRITE (Dev, SenseSize, VIRTIO_SCSI_SENSE_SIZE); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > > // > @@ -890,7 +907,7 @@ VirtioScsiInit ( > NextDevStat |= VSTAT_DRIVER_OK; > Status = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > > // > @@ -926,6 +943,9 @@ VirtioScsiInit ( > > return EFI_SUCCESS; > > +UnmapQueue: > + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > + > ReleaseQueue: > VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > @@ -965,6 +985,7 @@ VirtioScsiUninit ( > Dev->MaxLun = 0; > Dev->MaxSectors = 0; > > + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > SetMem (&Dev->PassThru, sizeof Dev->PassThru, 0x00); > @@ -995,6 +1016,12 @@ VirtioScsiExitBoot ( > // > Dev = Context; > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > + > + // > + // Unmap the ring buffer so that hypervisor will not be able to get > + // readable data after device reset. > + // > + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > } > > >