Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob

["Andrew Fish" <afish@apple.com>]

[-- Attachment #1: Type: text/plain, Size: 4917 bytes --]

Ray,

The other option would be to define a Library Class for the AcpiTableDxe driver to consume and have a NULL version of it in the MdeModulePkg. That would allow more flexibility? It kind of depends if you want to make the implementation depend on a HOB or a LibraryClass? It at least gives the non pure PI implementations more potential options? I’m not sure with the problem you are trying to solve that helps or hurts, but I though I would throw out another option. The other advantage about the lib is could let you define the HOB in another package, if that makes more sense. 

Not trying to slow down the solution, but just giving some other options in case another partitioning of this problem would be helpful? 

Thanks,

Andrew Fish 

> On Mar 24, 2021, at 6:10 PM, Ni, Ray <ray.ni@intel.com> wrote:
> 
> Ben,
> I understand your point.
> The purpose of changing the AcpiTableDxe to directly consume the HOB is to reduce the overall system complexity.
> The complexity I see with your option is:
> 1. platform needs to include that driver to consume the ACPI table from bootloader
> 2. that driver (consume HOB and install table through AcpiTable protocol) needs to register a protocol notification on AcpiTable protocol and do the table installation in the callback.
> 
> Laszlo,
> The change here is to meet the requirement that bootloader provides the ACPI table.
> In OVMF case, it's not the bootloader but the virtual hardware who provides the ACPI table.
> I can see the similarity between the two: the ACPI table is produced before the UEFI Payload runs.
> Can you review the new option below and see whether it can meet the OVMF needs as well?
> 1. Define a new GUID gPldHobAcpiTable in MdeModulePkg.dec and a structure as below in MdeModulePkg/Include/Guid/Acpi.h.
> typedef struct { 
>   UINT64                  TableAddress;
> }  PLD_HOB_ACPI_TABLE;
> 2. Change AcpiTableDxe driver to consume the HOB
> 3. Remove SYSTEM_TABLE_INFO. AcpiTableBase/AcpiTableSize.
> 4. Remove the BlSupportDxe code that consume the ACPI table in SYSTEM_TABLE_INFO.
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
>> Sent: Thursday, March 25, 2021 2:33 AM
>> To: Benjamin Doron <benjamin.doron00@gmail.com>; devel@edk2.groups.io
>> Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob
>> 
>> On 03/24/21 17:55, Benjamin Doron wrote:
>>>> 
>>>> 
>>>>> Hi all,
>>>>> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe (in
>>>>> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSDP
>>>>> and then install the tables? It's a solution that uses the regular
>>>>> UefiLib, so it avoids platform-specific quirks (and as I see it, if RSDP
>>>>> is in the configuration table, we probably always want those tables).
>>>> 
>>>> I'm sorry, I don't understand how this would help.
>>> 
>>> As I understand it, the issue is that this patchset changes MdeModulePkg to do platform-specific parsing.
>>> 
>>> Perhaps it would be an acceptable solution for platforms to retrieve the tables, then add
>>> RSDP/them to the configuration table to be installed by AcpiTableDxe/AcpiPlatformDxe.
>>> This allows MdeModulePkg to abstract away the parsing, only installing tables
>>> available to it.
>> 
>> But this is already the best approach, and already what's happening --
>> when you call EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() from the
>> platform's AcpiPlatformDxe, that's *how* you tell AcpiTableDxe in
>> MdeModulePkg to pick up the table and hook it into the RSDT / XSDT /
>> wherever, and also to manage RSD PTR as a UEFI configuration table.
>> 
>> Are you (more or less) proposing a new EFI_ACPI_TABLE_PROTOCOL member
>> function for taking a forest of ACPI tables, passed in by RSD PTR?
>> 
>> Sorry about being dense. :)
>> 
>>> (Currently, UefiPayloadPkg's BlSupportDxe retrieves the data from a HOB and calls
>>> `gBS->InstallConfigurationTable` with the address of RSDP).
>>> 
>>> I understand that this may not work for OVMF if tables are located differently in memory.
>>> 
>>>> 
>>>> 
>>>>> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed in
>>>>> DSC) but not added to a FV (not listed in FDF). So, how has this been
>>>>> tested?
>>>> 
>>>> I assume through an out-of-tree platform. Many such core modules exist
>>>> in edk2 that are not consumed by any of the virtual platforms in the
>>>> edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg).
>>> 
>>> This makes sense, but AcpiTableDxe must be added to UefiPayloadPkg's FDF
>>> if patch 2/2 is merged. Otherwise, ACPI tables will not be advertised.
>>> 
>> 
>> Good point; I missed that. (I'm not familiar with the UefiPayloadPkg FDF
>> at all.)
>> 
>> Laszlo
>> 
>> 
>> 
>> 
>> 
> 
> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 22332 bytes --]