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 12484209589D1 for ; Wed, 9 Aug 2017 14:11:09 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 07F10102DFE; Wed, 9 Aug 2017 21:13:27 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 07F10102DFE Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-102.phx2.redhat.com [10.3.116.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id D5BB65E277; Wed, 9 Aug 2017 21:13:24 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel References: <1502107139-412-1-git-send-email-brijesh.singh@amd.com> <1502107139-412-6-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: <33f9b0d5-6450-6f87-78e4-6950fab59dbd@redhat.com> Date: Wed, 9 Aug 2017 23:13:23 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1502107139-412-6-git-send-email-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 09 Aug 2017 21:13:27 +0000 (UTC) Subject: Re: [PATCH v1 05/14] OvmfPkg/VirtioLib: Pass VirtIo instance in VringInit/Uinit() 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: Wed, 09 Aug 2017 21:11:09 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit (1) the subject has a typo ("Uinit()") so I suggest: OvmfPkg/VirtioLib: take VirtIo instance in VirtioRingInit/VirtioRingUninit (74 characters). On 08/07/17 13:58, Brijesh Singh wrote: > Passing the VirtIo protocol instance will allow the vring to use > VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages () to allocate vring buffer. > > 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/Include/Library/VirtioLib.h | 14 ++++++++++---- > OvmfPkg/Library/VirtioLib/VirtioLib.c | 14 ++++++++++---- > OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 7 ++++--- > OvmfPkg/VirtioGpuDxe/Commands.c | 7 ++++--- > OvmfPkg/VirtioNetDxe/SnpInitialize.c | 9 +++++---- > OvmfPkg/VirtioNetDxe/SnpShutdown.c | 5 +++-- > OvmfPkg/VirtioRngDxe/VirtioRng.c | 7 ++++--- > OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 7 ++++--- > 8 files changed, 44 insertions(+), 26 deletions(-) > > diff --git a/OvmfPkg/Include/Library/VirtioLib.h b/OvmfPkg/Include/Library/VirtioLib.h > index fa82734f1852..610654225de7 100644 > --- a/OvmfPkg/Include/Library/VirtioLib.h > +++ b/OvmfPkg/Include/Library/VirtioLib.h > @@ -35,6 +35,8 @@ > - 1.1 Virtqueues, > - 2.3 Virtqueue Configuration. > > + @param[in] VirtIo The virtio device which will use the ring. > + > @param[in] The number of descriptors to allocate for the > virtio ring, as requested by the host. > > @@ -52,8 +54,9 @@ > EFI_STATUS > EFIAPI > VirtioRingInit ( > - IN UINT16 QueueSize, > - OUT VRING *Ring > + IN VIRTIO_DEVICE_PROTOCOL *VirtIo, > + IN UINT16 QueueSize, > + OUT VRING *Ring > ); > > > @@ -65,13 +68,16 @@ VirtioRingInit ( > invoking this function: the VSTAT_DRIVER_OK bit must be clear in > VhdrDeviceStatus. > > - @param[out] Ring The virtio ring to clean up. > + @param[in] VirtIo The virtio device which will was using the ring. (2) s/will was/was/ > + > + @param[out] Ring The virtio ring to clean up. > > **/ > VOID > EFIAPI > VirtioRingUninit ( > - IN OUT VRING *Ring > + IN VIRTIO_DEVICE_PROTOCOL *VirtIo, > + IN OUT VRING *Ring > ); > > > diff --git a/OvmfPkg/Library/VirtioLib/VirtioLib.c b/OvmfPkg/Library/VirtioLib/VirtioLib.c > index f6b464b6cdf8..50e5ec28ea1b 100644 > --- a/OvmfPkg/Library/VirtioLib/VirtioLib.c > +++ b/OvmfPkg/Library/VirtioLib/VirtioLib.c > @@ -37,6 +37,8 @@ > - 1.1 Virtqueues, > - 2.3 Virtqueue Configuration. > > + @param[in] VirtIo The virtio device which will use the ring. > + > @param[in] The number of descriptors to allocate for the > virtio ring, as requested by the host. > > @@ -54,8 +56,9 @@ > EFI_STATUS > EFIAPI > VirtioRingInit ( > - IN UINT16 QueueSize, > - OUT VRING *Ring > + IN VIRTIO_DEVICE_PROTOCOL *VirtIo, > + IN UINT16 QueueSize, > + OUT VRING *Ring > ) > { > UINTN RingSize; > @@ -128,13 +131,16 @@ VirtioRingInit ( > invoking this function: the VSTAT_DRIVER_OK bit must be clear in > VhdrDeviceStatus. > > - @param[out] Ring The virtio ring to clean up. > + @param[in] VirtIo The virtio device which will was using the ring. (3) Same as (2). With these fixed: Reviewed-by: Laszlo Ersek Thanks Laszlo > + > + @param[out] Ring The virtio ring to clean up. > > **/ > VOID > EFIAPI > VirtioRingUninit ( > - IN OUT VRING *Ring > + IN VIRTIO_DEVICE_PROTOCOL *VirtIo, > + IN OUT VRING *Ring > ) > { > FreePages (Ring->Base, Ring->NumPages); > diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c > index 3ce72281c204..61b9cab4ff02 100644 > --- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c > +++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c > @@ -12,6 +12,7 @@ > > Copyright (C) 2012, Red Hat, Inc. > Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
> + Copyright (c) 2017, AMD Inc, All rights reserved.
> > This program and the accompanying materials are licensed and made available > under the terms and conditions of the BSD License which accompanies this > @@ -722,7 +723,7 @@ VirtioBlkInit ( > goto Failed; > } > > - Status = VirtioRingInit (QueueSize, &Dev->Ring); > + Status = VirtioRingInit (Dev->VirtIo, QueueSize, &Dev->Ring); > if (EFI_ERROR (Status)) { > goto Failed; > } > @@ -811,7 +812,7 @@ VirtioBlkInit ( > return EFI_SUCCESS; > > ReleaseQueue: > - VirtioRingUninit (&Dev->Ring); > + VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > Failed: > // > @@ -848,7 +849,7 @@ VirtioBlkUninit ( > // > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > > - VirtioRingUninit (&Dev->Ring); > + VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > SetMem (&Dev->BlockIo, sizeof Dev->BlockIo, 0x00); > SetMem (&Dev->BlockIoMedia, sizeof Dev->BlockIoMedia, 0x00); > diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c > index 962087cfec97..c2e4d72feb67 100644 > --- a/OvmfPkg/VirtioGpuDxe/Commands.c > +++ b/OvmfPkg/VirtioGpuDxe/Commands.c > @@ -3,6 +3,7 @@ > VirtIo GPU initialization, and commands (primitives) for the GPU device. > > Copyright (C) 2016, Red Hat, Inc. > + Copyright (c) 2017, AMD Inc, All rights reserved.
> > This program and the accompanying materials are licensed and made available > under the terms and conditions of the BSD License which accompanies this > @@ -127,7 +128,7 @@ VirtioGpuInit ( > // > // [...] population of virtqueues [...] > // > - Status = VirtioRingInit (QueueSize, &VgpuDev->Ring); > + Status = VirtioRingInit (VgpuDev->VirtIo, QueueSize, &VgpuDev->Ring); > if (EFI_ERROR (Status)) { > goto Failed; > } > @@ -148,7 +149,7 @@ VirtioGpuInit ( > return EFI_SUCCESS; > > ReleaseQueue: > - VirtioRingUninit (&VgpuDev->Ring); > + VirtioRingUninit (VgpuDev->VirtIo, &VgpuDev->Ring); > > Failed: > // > @@ -183,7 +184,7 @@ VirtioGpuUninit ( > // configuration. > // > VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0); > - VirtioRingUninit (&VgpuDev->Ring); > + VirtioRingUninit (VgpuDev->VirtIo, &VgpuDev->Ring); > } > > /** > diff --git a/OvmfPkg/VirtioNetDxe/SnpInitialize.c b/OvmfPkg/VirtioNetDxe/SnpInitialize.c > index 430670a980f2..6d9b81a9f939 100644 > --- a/OvmfPkg/VirtioNetDxe/SnpInitialize.c > +++ b/OvmfPkg/VirtioNetDxe/SnpInitialize.c > @@ -5,6 +5,7 @@ > > Copyright (C) 2013, Red Hat, Inc. > Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
> + Copyright (c) 2017, AMD Inc, All rights reserved.
> > This program and the accompanying materials are licensed and made available > under the terms and conditions of the BSD License which accompanies this > @@ -73,7 +74,7 @@ VirtioNetInitRing ( > if (QueueSize < 2) { > return EFI_UNSUPPORTED; > } > - Status = VirtioRingInit (QueueSize, Ring); > + Status = VirtioRingInit (Dev->VirtIo, QueueSize, Ring); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -103,7 +104,7 @@ VirtioNetInitRing ( > return EFI_SUCCESS; > > ReleaseQueue: > - VirtioRingUninit (Ring); > + VirtioRingUninit (Dev->VirtIo, Ring); > > return Status; > } > @@ -509,10 +510,10 @@ AbortDevice: > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > > ReleaseTxRing: > - VirtioRingUninit (&Dev->TxRing); > + VirtioRingUninit (Dev->VirtIo, &Dev->TxRing); > > ReleaseRxRing: > - VirtioRingUninit (&Dev->RxRing); > + VirtioRingUninit (Dev->VirtIo, &Dev->RxRing); > > DeviceFailed: > // > diff --git a/OvmfPkg/VirtioNetDxe/SnpShutdown.c b/OvmfPkg/VirtioNetDxe/SnpShutdown.c > index 01409c0ce714..5e84191fbbdd 100644 > --- a/OvmfPkg/VirtioNetDxe/SnpShutdown.c > +++ b/OvmfPkg/VirtioNetDxe/SnpShutdown.c > @@ -4,6 +4,7 @@ > > Copyright (C) 2013, Red Hat, Inc. > Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
> + Copyright (c) 2017, AMD Inc, All rights reserved.
> > This program and the accompanying materials are licensed and made available > under the terms and conditions of the BSD License which accompanies this > @@ -66,8 +67,8 @@ VirtioNetShutdown ( > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > VirtioNetShutdownRx (Dev); > VirtioNetShutdownTx (Dev); > - VirtioRingUninit (&Dev->TxRing); > - VirtioRingUninit (&Dev->RxRing); > + VirtioRingUninit (Dev->VirtIo, &Dev->TxRing); > + VirtioRingUninit (Dev->VirtIo, &Dev->RxRing); > > Dev->Snm.State = EfiSimpleNetworkStarted; > Status = EFI_SUCCESS; > diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.c b/OvmfPkg/VirtioRngDxe/VirtioRng.c > index 1a186d04082a..e20602ac7225 100644 > --- a/OvmfPkg/VirtioRngDxe/VirtioRng.c > +++ b/OvmfPkg/VirtioRngDxe/VirtioRng.c > @@ -6,6 +6,7 @@ > > Copyright (C) 2012, Red Hat, Inc. > Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.
> + Copyright (c) 2017, AMD Inc, All rights reserved.
> > This driver: > > @@ -275,7 +276,7 @@ VirtioRngInit ( > goto Failed; > } > > - Status = VirtioRingInit (QueueSize, &Dev->Ring); > + Status = VirtioRingInit (Dev->VirtIo, QueueSize, &Dev->Ring); > if (EFI_ERROR (Status)) { > goto Failed; > } > @@ -331,7 +332,7 @@ VirtioRngInit ( > return EFI_SUCCESS; > > ReleaseQueue: > - VirtioRingUninit (&Dev->Ring); > + VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > Failed: > // > @@ -358,7 +359,7 @@ VirtioRngUninit ( > // the old comms area. > // > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > - VirtioRingUninit (&Dev->Ring); > + VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > } > > // > diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > index c080404330e5..c2f6f412ff40 100644 > --- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > +++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c > @@ -27,6 +27,7 @@ > > Copyright (C) 2012, Red Hat, Inc. > Copyright (c) 2012 - 2014, Intel Corporation. All rights reserved.
> + Copyright (c) 2017, AMD Inc, All rights reserved.
> > This program and the accompanying materials are licensed and made available > under the terms and conditions of the BSD License which accompanies this > @@ -832,7 +833,7 @@ VirtioScsiInit ( > goto Failed; > } > > - Status = VirtioRingInit (QueueSize, &Dev->Ring); > + Status = VirtioRingInit (Dev->VirtIo, QueueSize, &Dev->Ring); > if (EFI_ERROR (Status)) { > goto Failed; > } > @@ -926,7 +927,7 @@ VirtioScsiInit ( > return EFI_SUCCESS; > > ReleaseQueue: > - VirtioRingUninit (&Dev->Ring); > + VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > Failed: > // > @@ -964,7 +965,7 @@ VirtioScsiUninit ( > Dev->MaxLun = 0; > Dev->MaxSectors = 0; > > - VirtioRingUninit (&Dev->Ring); > + VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > > SetMem (&Dev->PassThru, sizeof Dev->PassThru, 0x00); > SetMem (&Dev->PassThruMode, sizeof Dev->PassThruMode, 0x00); >