Re: [PATCH v3 16/35] OvmfPkg/XenHypercallLib: Enable it in PEIM

["Laszlo Ersek" <lersek@redhat.com>]

On 07/04/19 16:42, Anthony PERARD wrote:
> Allow to use Xen hypercalls earlier, during the PEIM stage, but
> XenHypercallLibInit() must be called once the XenInfo HOB is created
> with the HyperPage setup.
> 
> Change the return value of XenHypercallLibInit so failure can be
> detected when the call shouldn't fail, but still have the constructor
> always succeed.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> 
> Notes:
>     v3:
>     - only modify XenHypercallLib, and to the modification of XenPlatformPei
>       in a separated patch.
>     - Allow XenHypercallLibInit to be called outside the library instead of
>       creating a new function, but also return failure on failure,
>       and have a new constructor that never fail.
> 
>  .../Library/XenHypercallLib/XenHypercallLib.inf  |  4 ++--
>  OvmfPkg/Include/Library/XenHypercallLib.h        | 12 ++++++++++++
>  .../Library/XenHypercallLib/X86XenHypercall.c    |  8 +-------
>  OvmfPkg/Library/XenHypercallLib/XenHypercall.c   | 16 ++++++++++++++++
>  4 files changed, 31 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf b/OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> index 1208f0057a..21ce5b4434 100644
> --- a/OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> +++ b/OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
> @@ -12,10 +12,10 @@ [Defines]
>    FILE_GUID                      = B5EE9A32-CA5A-49A8-82E3-ADA4CCB77C7C
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
> -  CONSTRUCTOR                    = XenHypercallLibInit
> +  CONSTRUCTOR                    = XenHypercallLibConstruct
>  
>  [Defines.IA32, Defines.X64]
> -  LIBRARY_CLASS                  = XenHypercallLib|DXE_DRIVER UEFI_DRIVER
> +  LIBRARY_CLASS                  = XenHypercallLib|PEIM DXE_DRIVER UEFI_DRIVER
>  
>  [Defines.ARM, Defines.AARCH64]
>    LIBRARY_CLASS                  = XenHypercallLib
> diff --git a/OvmfPkg/Include/Library/XenHypercallLib.h b/OvmfPkg/Include/Library/XenHypercallLib.h
> index c43822782b..c1491dd652 100644
> --- a/OvmfPkg/Include/Library/XenHypercallLib.h
> +++ b/OvmfPkg/Include/Library/XenHypercallLib.h
> @@ -10,6 +10,18 @@
>  #ifndef __XEN_HYPERCALL_LIB_H__
>  #define __XEN_HYPERCALL_LIB_H__
>  
> +/**
> +  To call when the gEfiXenInfoGuid HOB became available after the library init
> +  function has already been executed.
> +
> +  This allow to make hypercall in the PEIM stage.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +XenHypercallLibInit (
> +  VOID
> +  );
> +
>  /**
>    Check if the Xen Hypercall library is able to make calls to the Xen
>    hypervisor.
> diff --git a/OvmfPkg/Library/XenHypercallLib/X86XenHypercall.c b/OvmfPkg/Library/XenHypercallLib/X86XenHypercall.c
> index 27083f924f..f779e46470 100644
> --- a/OvmfPkg/Library/XenHypercallLib/X86XenHypercall.c
> +++ b/OvmfPkg/Library/XenHypercallLib/X86XenHypercall.c
> @@ -59,13 +59,7 @@ XenHypercallLibInit (
>  
>    GuidHob = GetFirstGuidHob (&gEfiXenInfoGuid);
>    if (GuidHob == NULL) {
> -    //
> -    // We don't fail library construction, since that has catastrophic
> -    // consequences for client modules (whereas those modules may easily be
> -    // running on a non-Xen platform). Instead, XenHypercallIsAvailable() above
> -    // will return FALSE.
> -    //
> -    return RETURN_SUCCESS;
> +    return RETURN_NOT_FOUND;
>    }
>    XenInfo = (EFI_XEN_INFO *) GET_GUID_HOB_DATA (GuidHob);
>    HyperPage = XenInfo->HyperPages;
> diff --git a/OvmfPkg/Library/XenHypercallLib/XenHypercall.c b/OvmfPkg/Library/XenHypercallLib/XenHypercall.c
> index a2c41a2a69..d4fa802743 100644
> --- a/OvmfPkg/Library/XenHypercallLib/XenHypercall.c
> +++ b/OvmfPkg/Library/XenHypercallLib/XenHypercall.c
> @@ -15,6 +15,22 @@
>  #include <Library/DebugLib.h>
>  #include <Library/XenHypercallLib.h>
>  
> +RETURN_STATUS
> +EFIAPI
> +XenHypercallLibConstruct (
> +  VOID
> +  )
> +{
> +  XenHypercallLibInit ();
> +  //
> +  // We don't fail library construction, since that has catastrophic
> +  // consequences for client modules (whereas those modules may easily be
> +  // running on a non-Xen platform). Instead, XenHypercallIsAvailable()
> +  // will return FALSE.
> +  //
> +  return RETURN_SUCCESS;
> +}
> +
>  UINT64
>  EFIAPI
>  XenHypercallHvmGetParam (
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>