On 08/07/17 13:58, Brijesh Singh wrote:
> The vring buffer is communication block between guest and hypervisor,
> allocate the vring buffer using AllocateSharedPages() so that it can be
> Map() with BusMasterCommonBufer 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.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/OvmfPkg/Library/VirtioLib/VirtioLib.c
> b/OvmfPkg/Library/VirtioLib/VirtioLib.c
> index 09a3f9b7f2e5..9ba64204326f 100644
> --- a/OvmfPkg/Library/VirtioLib/VirtioLib.c
> +++ b/OvmfPkg/Library/VirtioLib/VirtioLib.c
> @@ -61,6 +61,7 @@ VirtioRingInit (
> OUT VRING *Ring
> )
> {
> + EFI_STATUS Status;
> UINTN RingSize;
> volatile UINT8 *RingPagesPtr;
>
> @@ -79,9 +80,12 @@ 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) {
> + Status = VirtioAllocateSharedPages (VirtIo, Ring->NumPages, &Ring->Base);
> + if (EFI_ERROR (Status)) {
> return EFI_OUT_OF_RESOURCES;
> }
> SetMem (Ring->Base, RingSize, 0x00);
> @@ -143,7 +147,7 @@ VirtioRingUninit (
> IN OUT VRING *Ring
> )
> {
> - FreePages (Ring->Base, Ring->NumPages);
> + VirtioFreeSharedPages (VirtIo, Ring->NumPages, Ring->Base);
> SetMem (Ring, sizeof *Ring, 0x00);
> }
>
>
The logic looks good, I have some superficial comments:
(1) Please update the subject, the commit message, and the code to refer
to VirtIo->AllocateSharedPages() (or even VIRTIO_ALLOCATE_SHARED, as you
see fit -- just not the helper function, because we won't have that).
(2) Please update the documentation of "@retval EFI_OUT_OF_RESOURCES" --
replace it with
@return Status codes propagated from VirtIo->AllocateSharedPages().
(3) Accordingly, please forward the Status received, on the error branch.
(4) Please remove MemoryAllocationLib from the #includes and also from
[LibraryClasses].
Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
