Re: [Xen-devel] [edk2] [PATCH RFC v2 4/7] OvmfPkg: extract OVMF info passed by Xen hvmloader

[Jordan Justen <jljusten@xxxxxxxxx>]

On Tue, Nov 19, 2013 at 12:38 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  OvmfPkg/PlatformPei/Xen.c |   31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c
> index a720b91..fbb2619 100644
> --- a/OvmfPkg/PlatformPei/Xen.c
> +++ b/OvmfPkg/PlatformPei/Xen.c
> @@ -26,12 +26,15 @@
>  #include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <IndustryStandard/E820.h>
>  #include <Guid/XenInfo.h>
>
>  #include "Platform.h"
>
>  EFI_XEN_INFO mXenInfo;
>
> +#define OVMF_INFO_PHYSICAL_ADDRESS 0x00001000
>
>  /**
>    Connects to the Hypervisor.
> @@ -50,6 +53,9 @@ XenConnect (
>    UINT32 TransferReg;
>    UINT32 TransferPages;
>    UINT32 XenVersion;
> +  EFI_XEN_OVMF_INFO *Info = (EFI_XEN_OVMF_INFO *) OVMF_INFO_PHYSICAL_ADDRESS;
> +
> +  ZeroMem (&mXenInfo, sizeof(mXenInfo));
>
>    AsmCpuid (XenLeaf + 2, &TransferPages, &TransferReg, NULL, NULL);
>    mXenInfo.HyperPages = AllocatePages (TransferPages);
> @@ -72,6 +78,31 @@ XenConnect (
>    /* TBD: Locate hvm_info and reserve it away. */
>    mXenInfo.HvmInfo = NULL;
>
> +  if (!AsciiStrCmp ((CHAR8 *) Info->Signature, "XenHVMOVMF")) {

Would AsciiStrnCmp be a good idea here? Like I mentioned in the other
patch, we normally use an integer based signature.

> +    EFI_E820_ENTRY *E820Map;
> +    UINTN Loop, EntryCount, Base;
> +
> +    /* E820 map */
> +    EntryCount = Info->E820Nr;
> +    Base = Info->E820;
> +
> +    E820Map = AllocateZeroPool (sizeof(EFI_E820_ENTRY) * EntryCount);
> +
> +    if (!E820Map) {
> +      FreePages (mXenInfo.HyperPages, TransferPages);
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    for (Loop = 0; Loop < EntryCount; Loop++) {
> +      EFI_E820_ENTRY *src = (EFI_E820_ENTRY *)Base + Loop;
> +      EFI_E820_ENTRY *dst = (EFI_E820_ENTRY *)E820Map + Loop;
> +      CopyMem (dst, src, sizeof(EFI_E820_ENTRY));
> +    }

How about AllocateCopyPool and just copy the entire array in one shot?

-Jordan

> +
> +    mXenInfo.E820 = E820Map;
> +    mXenInfo.E820EntryCount = EntryCount;
> +  }
> +
>    BuildGuidDataHob (
>      &gEfiXenInfoGuid,
>      &mXenInfo,
> --
> 1.7.10.4
>
>
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing
> conversations that shape the rapidly evolving mobile landscape. Sign up now.
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel