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 DC6F221CFA5F7 for ; Wed, 16 Aug 2017 08:30:33 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8239AB2C1; Wed, 16 Aug 2017 15:32:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8239AB2C1 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-59.phx2.redhat.com [10.3.116.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id DDB186BF79; Wed, 16 Aug 2017 15:32:57 +0000 (UTC) To: Brijesh Singh , edk2-devel@lists.01.org Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel References: <1502710605-8058-1-git-send-email-brijesh.singh@amd.com> <1502710605-8058-9-git-send-email-brijesh.singh@amd.com> From: Laszlo Ersek Message-ID: Date: Wed, 16 Aug 2017 17:32:56 +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: <1502710605-8058-9-git-send-email-brijesh.singh@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 16 Aug 2017 15:32:59 +0000 (UTC) Subject: Re: [PATCH v2 08/23] OvmfPkg/VirtioPciDeviceDxe: implement IOMMU-like member functions 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, 16 Aug 2017 15:30:34 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 08/14/17 13:36, Brijesh Singh wrote: > The patch implements the newly added IOMMU-like member functions by > respectively delegating the job to: > > - VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages () -> > MemoryAllocationLib.AllocatePages() > > - VIRTIO_DEVICE_PROTOCOL.FreeSharedPages () -> > MemoryAllocationLib.FreePages () > > - VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer () -> no-op > > - VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer () -> no-op > > Suggested-by: Laszlo Ersek > 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/VirtioPciDeviceDxe/VirtioPciDevice.h | 34 ++++++++++++ > OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c | 7 ++- > OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c | 58 ++++++++++++++++++++ > 3 files changed, 98 insertions(+), 1 deletion(-) > > diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h > index 6f51f816ef0f..41df5a98e560 100644 > --- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h > +++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h > @@ -3,6 +3,7 @@ > Internal definitions for the VirtIo PCI Device driver > > Copyright (C) 2013, ARM Ltd > + 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 > @@ -156,4 +157,37 @@ VirtioPciSetDeviceStatus ( > IN UINT8 DeviceStatus > ); > > +EFI_STATUS > +EFIAPI > +VirtioPciAllocateSharedPages ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN UINTN NumPages, > + OUT VOID **HostAddress > + ); > + > +VOID > +EFIAPI > +VirtioPciFreeSharedPages ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN UINTN NumPages, > + IN VOID *HostAddress > + ); > + > +EFI_STATUS > +EFIAPI > +VirtioPciMapSharedBuffer ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN VIRTIO_MAP_OPERATION Operation, > + IN VOID *HostAddress, > + IN OUT UINTN *NumberOfBytes, > + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, > + OUT VOID **Mapping > + ); > + > +EFI_STATUS > +EFIAPI > +VirtioPciUnmapSharedBuffer ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN VOID *Mapping > + ); > #endif // _VIRTIO_PCI_DEVICE_DXE_H_ > diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c > index e41730456471..b847f3c02528 100644 > --- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c > +++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c > @@ -5,6 +5,7 @@ > Copyright (C) 2012, Red Hat, Inc. > Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
> Copyright (C) 2013, ARM Ltd. > + 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 > @@ -40,7 +41,11 @@ STATIC VIRTIO_DEVICE_PROTOCOL mDeviceProtocolTemplate = { > VirtioPciGetDeviceStatus, // GetDeviceStatus > VirtioPciSetDeviceStatus, // SetDeviceStatus > VirtioPciDeviceWrite, // WriteDevice > - VirtioPciDeviceRead // ReadDevice > + VirtioPciDeviceRead, // ReadDevice > + VirtioPciAllocateSharedPages, // AllocateSharedPages > + VirtioPciFreeSharedPages, // FreeSharedPages > + VirtioPciMapSharedBuffer, // MapSharedBuffer > + VirtioPciUnmapSharedBuffer, // UnmapSharedBuffer > }; > > /** > diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c > index 5f86914265ea..4597095deb78 100644 > --- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c > +++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c > @@ -5,6 +5,7 @@ > Copyright (C) 2012, Red Hat, Inc. > Copyright (c) 2012, Intel Corporation. All rights reserved.
> Copyright (C) 2013, ARM Ltd. > + 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 > @@ -271,3 +272,60 @@ VirtioPciSetDeviceStatus ( > return VirtioPciIoWrite (Dev, VIRTIO_PCI_OFFSET_QUEUE_DEVICE_STATUS, > sizeof (UINT8), DeviceStatus); > } > + > +EFI_STATUS > +EFIAPI > +VirtioPciAllocateSharedPages ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN UINTN NumPages, > + OUT VOID **HostAddress > + ) > +{ > + VOID *Buffer; > + > + Buffer = AllocatePages (NumPages); > + if (Buffer == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + *HostAddress = Buffer; > + return EFI_SUCCESS; > +} > + > +VOID > +EFIAPI > +VirtioPciFreeSharedPages ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN UINTN NumPages, > + IN VOID *HostAddress > + ) > +{ > + FreePages (HostAddress, NumPages); > +} > + > +EFI_STATUS > +EFIAPI > +VirtioPciMapSharedBuffer ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN VIRTIO_MAP_OPERATION Operation, > + IN VOID *HostAddress, > + IN OUT UINTN *NumberOfBytes, > + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, > + OUT VOID **Mapping > + ) > +{ > + *DeviceAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress; > + *Mapping = NULL; > + > + return EFI_SUCCESS; > +} > + > +EFI_STATUS > +EFIAPI > +VirtioPciUnmapSharedBuffer ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + OUT VOID *Mapping > + ) (1) Is this an Easter egg for your reviewer? :) "Mapping" should be IN, not OUT. This oversight is surprising -- or, well, an Easter egg :) -- because the function declaration, in "OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h" above, is correct. The patch is good otherwise. Thanks! Laszlo > +{ > + return EFI_SUCCESS; > +} >