Re: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table

["Guo Dong" <guo.dong@intel.com>]

Sure. I updated the patch per comments.

Thanks,
Guo

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, September 27, 2021 7:11 PM
To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table

I prefer we still let caller to provide the HOB pointer.
This also eliminates a global variable "mAcpiBoardInfo".

You could change the BuildHobFromAcpi() to return the HOB pointer.
So that the pointer can be directly passed to ParseMemoryInfo().

Thanks,
Ray

> -----Original Message-----
> From: Dong, Guo <guo.dong@intel.com>
> Sent: Monday, September 27, 2021 2:32 AM
> To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> Hi Ray,
> 
> In this patch, we added a shared file AcpiTable.c for both universal payload and non-universal payload.
> The exposed API from this file is:   EFI_STATUS  BuildHobFromAcpi ( IN   UINT64 AcpiTableBase);
> This function will build an ACPI board HOB based on the information from ACPI table.
> 
> For universal payload, it calls this function to build a hob for other modules. The main function is very simple and clear.
> 
> For non-universal payload, ACPI board HOB is used in the ParseMemoryInfo() callback for PCIE base info.
> So we could get this HOB from the caller, or get this HOB inside the callback. I select to do it inside the callback.
> 
> Thanks,
> Guo
> 
> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Saturday, September 25, 2021 7:48 PM
> To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; You, Benjamin <benjamin.you@intel.com>
> Subject: RE: [`edk2-devel][PATCH] UefiPayloadPkg: Build a HOB from bootloader ACPI table
> 
> 
> -  Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo);
> 
> +  Status = ParseMemoryInfo (MemInfoCallbackMmio, NULL);
> 
> Guo,
> I am curious why you changed this part.
> Without this change, MemInfoCallbackMmio() can get the AcpiBoardInfo from the parameter.
> With the change, it has to locate the Guided HOB itself.
> 
> 
> Other parts look good to me.
> 
> Thanks,
> Ray