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 E288A21E2BE59 for ; Sun, 27 Aug 2017 11:55:40 -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 ED9065F7AE; Sun, 27 Aug 2017 18:58:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com ED9065F7AE Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-92.phx2.redhat.com [10.3.116.92]) by smtp.corp.redhat.com (Postfix) with ESMTP id CC3CA600C2; Sun, 27 Aug 2017 18:58:14 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel References: <1503697414-6830-1-git-send-email-brijesh.singh@amd.com> <1503697414-6830-2-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: <10d6bd88-6320-cbbb-826a-2316e95ac804@redhat.com> Date: Sun, 27 Aug 2017 20:58:13 +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: <1503697414-6830-2-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.39]); Sun, 27 Aug 2017 18:58:18 +0000 (UTC) Subject: Re: [PATCH 1/3] OvmfPkg/VirtioBlkDxe: 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: Sun, 27 Aug 2017 18:55:41 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/25/17 23:43, 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/VirtioBlkDxe/VirtioBlk.h | 1 + > OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 45 ++++++++++++++++---- > 2 files changed, 38 insertions(+), 8 deletions(-) > > diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h > index 6c402ca88ea4..9ec0b956b818 100644 > --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.h > +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.h > @@ -41,6 +41,7 @@ typedef struct { > VRING Ring; // VirtioRingInit 2 > EFI_BLOCK_IO_PROTOCOL BlockIo; // VirtioBlkInit 1 > EFI_BLOCK_IO_MEDIA BlockIoMedia; // VirtioBlkInit 1 > + VOID *RingMap; // VirtioRingMap 2 > } VBLK_DEV; > > #define VIRTIO_BLK_FROM_BLOCK_IO(BlockIoPointer) \ > diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c > index bff15fe3add1..663ba281ab73 100644 > --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c > +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c > @@ -580,7 +580,8 @@ VirtioBlkDriverBindingSupported ( > virtio-blk attributes the host provides. > > @return Error codes from VirtioRingInit() or > - VIRTIO_CFG_READ() / VIRTIO_CFG_WRITE(). > + VIRTIO_CFG_READ() / VIRTIO_CFG_WRITE or > + VirtioRingMap(). > > **/ > > @@ -601,6 +602,7 @@ VirtioBlkInit ( > UINT8 AlignmentOffset; > UINT32 OptIoSize; > UINT16 QueueSize; > + UINT64 RingBaseShift; > > PhysicalBlockExp = 0; > AlignmentOffset = 0; > @@ -729,25 +731,42 @@ VirtioBlkInit ( > } > > // > + // 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; > } > > > @@ -758,7 +777,7 @@ VirtioBlkInit ( > Features &= ~(UINT64)VIRTIO_F_VERSION_1; > Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > } > > @@ -768,7 +787,7 @@ VirtioBlkInit ( > NextDevStat |= VSTAT_DRIVER_OK; > Status = Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, NextDevStat); > if (EFI_ERROR (Status)) { > - goto ReleaseQueue; > + goto UnmapQueue; > } > > // > @@ -811,6 +830,9 @@ VirtioBlkInit ( > } > return EFI_SUCCESS; > > +UnmapQueue: > + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > + > ReleaseQueue: > VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > @@ -849,6 +871,7 @@ VirtioBlkUninit ( > // > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > > + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > SetMem (&Dev->BlockIo, sizeof Dev->BlockIo, 0x00); > @@ -885,6 +908,12 @@ VirtioBlkExitBoot ( > // > 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 is reset. > + // > + Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > } > > /** > Reviewed-by: Laszlo Ersek