[v3,27/35] OvmfPkg/XenPlatformLib: Cache result for XenDetected

Commit Message

Anthony PERARD July 4, 2019, 2:42 p.m. UTC

We are going to replace XenDetected() implementation in
PlatformBootManagerLib by the one in XenPlatformLib.
PlatformBootManagerLib's implementation does cache the result of
GetFirstGuidHob(), so we do something similar in XenPlatformLib.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
Signed-off-by: Anthony PERARD <[e-mail]>
---

Notes:
    v3:
    - new patch

 .../Library/XenPlatformLib/XenPlatformLib.c    | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

Comments

Laszlo Ersek July 10, 2019, 9:31 a.m. UTC | #1

On 07/04/19 16:42, Anthony PERARD wrote:
> We are going to replace XenDetected() implementation in
> PlatformBootManagerLib by the one in XenPlatformLib.
> PlatformBootManagerLib's implementation does cache the result of
> GetFirstGuidHob(), so we do something similar in XenPlatformLib.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689
> Signed-off-by: Anthony PERARD <[e-mail]>
> ---
> 
> Notes:
>     v3:
>     - new patch
> 
>  .../Library/XenPlatformLib/XenPlatformLib.c    | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c b/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
> index 6f27cbffa8..b5257b0c97 100644
> --- a/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
> +++ b/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
> @@ -26,13 +26,25 @@ XenGetInfoHOB (
>    )
>  {
>    EFI_HOB_GUID_TYPE  *GuidHob;
> +  STATIC BOOLEAN     Cached = FALSE;
> +  STATIC EFI_XEN_INFO *XenInfo;

(1) The alignment of the variable names is weird. The above is neither
condensed nor precisely aligned. Please pick one:

  EFI_HOB_GUID_TYPE *GuidHob;
  STATIC BOOLEAN Cached = FALSE;
  STATIC EFI_XEN_INFO *XenInfo;

or

  EFI_HOB_GUID_TYPE   *GuidHob;
  STATIC BOOLEAN      Cached = FALSE;
  STATIC EFI_XEN_INFO *XenInfo;

(The 2nd form is preferred in edk2.)

> +
> +  //
> +  // Return the cached result for the benefit of XenDetected that can be
> +  // called many times.
> +  //
> +  if (Cached) {
> +    return XenInfo;
> +  }
>  
>    GuidHob = GetFirstGuidHob (&gEfiXenInfoGuid);
>    if (GuidHob == NULL) {
> -    return NULL;
> +    XenInfo = NULL;
> +  } else {
> +    XenInfo = (EFI_XEN_INFO *) GET_GUID_HOB_DATA (GuidHob);
>    }
> -
> -  return (EFI_XEN_INFO *) GET_GUID_HOB_DATA (GuidHob);
> +  Cached = TRUE;
> +  return XenInfo;
>  }
>  
>  /**
> 

This will work fine in DXE modules (and by the end of the series, only
DXE modules use XenPlatformLib -- AcpiPlatformDxe, XenIoPvhDxe, and
PlatformBootManagerLib, which is only linked into DXE modules).

With (1) fixed:

Reviewed-by: Laszlo Ersek <[e-mail]>

Thanks
Laszlo

Patch

diff --git a/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c b/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
index 6f27cbffa8..b5257b0c97 100644
--- a/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
+++ b/OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
@@ -26,13 +26,25 @@  XenGetInfoHOB (
   )
 {
   EFI_HOB_GUID_TYPE  *GuidHob;
+  STATIC BOOLEAN     Cached = FALSE;
+  STATIC EFI_XEN_INFO *XenInfo;
+
+  //
+  // Return the cached result for the benefit of XenDetected that can be
+  // called many times.
+  //
+  if (Cached) {
+    return XenInfo;
+  }
 
   GuidHob = GetFirstGuidHob (&gEfiXenInfoGuid);
   if (GuidHob == NULL) {
-    return NULL;
+    XenInfo = NULL;
+  } else {
+    XenInfo = (EFI_XEN_INFO *) GET_GUID_HOB_DATA (GuidHob);
   }
-
-  return (EFI_XEN_INFO *) GET_GUID_HOB_DATA (GuidHob);
+  Cached = TRUE;
+  return XenInfo;
 }
 
 /**