On 08/23/17 14:22, Brijesh Singh wrote:
> The VRING buffer is a communication area between guest and hypervisor.
> Allocate it using VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() so that
> it can be mapped later with VirtioRingMap() for bi-directional access.
>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.si...@amd.com>
> ---
> OvmfPkg/Library/VirtioLib/VirtioLib.inf | 1 -
> OvmfPkg/Include/Library/VirtioLib.h | 5 ++---
> OvmfPkg/Library/VirtioLib/VirtioLib.c | 22 +++++++++++++-------
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/OvmfPkg/Library/VirtioLib/VirtioLib.inf
> b/OvmfPkg/Library/VirtioLib/VirtioLib.inf
> index fb5897a88ecf..e33856de38c4 100644
> --- a/OvmfPkg/Library/VirtioLib/VirtioLib.inf
> +++ b/OvmfPkg/Library/VirtioLib/VirtioLib.inf
> @@ -32,5 +32,4 @@ [LibraryClasses]
> BaseLib
> BaseMemoryLib
> DebugLib
> - MemoryAllocationLib
> UefiBootServicesTableLib
> diff --git a/OvmfPkg/Include/Library/VirtioLib.h
> b/OvmfPkg/Include/Library/VirtioLib.h
> index 3d9314b3acaf..c3e56ea23b89 100644
> --- a/OvmfPkg/Include/Library/VirtioLib.h
> +++ b/OvmfPkg/Include/Library/VirtioLib.h
> @@ -42,9 +42,8 @@
>
> @param[out] Ring The virtio ring to set up.
>
> - @retval EFI_OUT_OF_RESOURCES AllocatePages() failed to allocate contiguous
> - pages for the requested QueueSize. Fields of
> - Ring have indeterminate value.
> + @retval Status codes propagated from
> + VirtIo->AllocateSharedPages().
(1) The documentation has been updated (thanks for that), but I also
requested "@return", not "@retval". Can be fixed up on push.
>
> @retval EFI_SUCCESS Allocation and setup successful. Ring->Base
> (and nothing else) is responsible for
> diff --git a/OvmfPkg/Library/VirtioLib/VirtioLib.c
> b/OvmfPkg/Library/VirtioLib/VirtioLib.c
> index 535635ac0ba8..e5366e385f5d 100644
> --- a/OvmfPkg/Library/VirtioLib/VirtioLib.c
> +++ b/OvmfPkg/Library/VirtioLib/VirtioLib.c
> @@ -19,7 +19,6 @@
> #include <Library/BaseLib.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> -#include <Library/MemoryAllocationLib.h>
> #include <Library/UefiBootServicesTableLib.h>
>
> #include <Library/VirtioLib.h>
> @@ -44,9 +43,8 @@
>
> @param[out] Ring The virtio ring to set up.
>
> - @retval EFI_OUT_OF_RESOURCES AllocatePages() failed to allocate contiguous
> - pages for the requested QueueSize. Fields of
> - Ring have indeterminate value.
> + @retval Status codes propagated from
> + VirtIo->AllocateSharedPages().
(2) Same as (1), should be @return.
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks!
Laszlo
>
> @retval EFI_SUCCESS Allocation and setup successful. Ring->Base
> (and nothing else) is responsible for
> @@ -61,6 +59,7 @@ VirtioRingInit (
> OUT VRING *Ring
> )
> {
> + EFI_STATUS Status;
> UINTN RingSize;
> volatile UINT8 *RingPagesPtr;
>
> @@ -79,10 +78,17 @@ VirtioRingInit (
> sizeof *Ring->Used.AvailEvent,
> EFI_PAGE_SIZE);
>
> + //
> + // Allocate a shared ring buffer
> + //
> Ring->NumPages = EFI_SIZE_TO_PAGES (RingSize);
> - Ring->Base = AllocatePages (Ring->NumPages);
> - if (Ring->Base == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> + Status = VirtIo->AllocateSharedPages (
> + VirtIo,
> + Ring->NumPages,
> + &Ring->Base
> + );
> + if (EFI_ERROR (Status)) {
> + return Status;
> }
> SetMem (Ring->Base, RingSize, 0x00);
> RingPagesPtr = Ring->Base;
> @@ -143,7 +149,7 @@ VirtioRingUninit (
> IN OUT VRING *Ring
> )
> {
> - FreePages (Ring->Base, Ring->NumPages);
> + VirtIo->FreeSharedPages (VirtIo, Ring->NumPages, Ring->Base);
> SetMem (Ring, sizeof *Ring, 0x00);
> }
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
