From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from rn-mailsvcp-ppex-lapp15.apple.com (rn-mailsvcp-ppex-lapp15.apple.com [17.179.253.34]) by mx.groups.io with SMTP id smtpd.web12.3308.1616644528911675920 for ; Wed, 24 Mar 2021 20:55:28 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@apple.com header.s=20180706 header.b=YDLoROTK; spf=pass (domain: apple.com, ip: 17.179.253.34, mailfrom: afish@apple.com) Received: from pps.filterd (rn-mailsvcp-ppex-lapp15.rno.apple.com [127.0.0.1]) by rn-mailsvcp-ppex-lapp15.rno.apple.com (8.16.0.43/8.16.0.43) with SMTP id 12P3pm7B002033; Wed, 24 Mar 2021 20:55:28 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=from : message-id : content-type : mime-version : subject : date : in-reply-to : cc : to : references; s=20180706; bh=0xy3PafguqaL46DIzSWfVGZW/eauo3vFOsbhYdGSsX0=; b=YDLoROTKGxgsIvrLEv0x8OZOPhRF4wymfXN4Ir7SoDPTyhpIh1ZyTKBgqpJ+N57as6lA 32tDK/IP5HuNXRVBjMA0TWwBzwz9l11duRTKrcdFtS34Pv6TOx3iH/oqbOVht0Qh7vdF WrzAWw7W+b7W6hIoSjI5Vzge9EaRqkpDfvb56+u+Em7Rsp6NKO1g13EOsiGyhs9pZkHn je5OzW7yXs8gsikYMQQhHob9enBGrUCVq4RLIl6Uyamj4jMFVu0xeVGNgF5ONOOcPSxo OBQFMlulNoCWoGRZkrOgr5DQEcz0VlCV8L8/Eh9SLDSIH5P1oncJOxNhaWiaV2TAU2kA kw== Received: from rn-mailsvcp-mta-lapp02.rno.apple.com (rn-mailsvcp-mta-lapp02.rno.apple.com [10.225.203.150]) by rn-mailsvcp-ppex-lapp15.rno.apple.com with ESMTP id 37dembd4nj-3 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 24 Mar 2021 20:55:28 -0700 Received: from rn-mailsvcp-mmp-lapp03.rno.apple.com (rn-mailsvcp-mmp-lapp03.rno.apple.com [17.179.253.16]) by rn-mailsvcp-mta-lapp02.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) with ESMTPS id <0QQI002L3AWGC870@rn-mailsvcp-mta-lapp02.rno.apple.com>; Wed, 24 Mar 2021 20:55:28 -0700 (PDT) Received: from process_milters-daemon.rn-mailsvcp-mmp-lapp03.rno.apple.com by rn-mailsvcp-mmp-lapp03.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) id <0QQI00Q00AP7PO00@rn-mailsvcp-mmp-lapp03.rno.apple.com>; Wed, 24 Mar 2021 20:55:28 -0700 (PDT) X-Va-A: X-Va-T-CD: 04619f74f9efdf6774a6b133346de527 X-Va-E-CD: b5f30b9d65bd1c0f3fb8f4c9dc4cd7af X-Va-R-CD: 15345fbd519b77a3bf708ef7fea543cf X-Va-CD: 0 X-Va-ID: 6f11a57b-93e6-4d42-af00-733d17d5355d X-V-A: X-V-T-CD: 04619f74f9efdf6774a6b133346de527 X-V-E-CD: b5f30b9d65bd1c0f3fb8f4c9dc4cd7af X-V-R-CD: 15345fbd519b77a3bf708ef7fea543cf X-V-CD: 0 X-V-ID: 16572977-1604-43b3-99a0-f59ddc2f37a2 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369,18.0.761 definitions=2021-03-24_14:2021-03-24,2021-03-24 signatures=0 Received: from [17.235.46.127] (unknown [17.235.46.127]) by rn-mailsvcp-mmp-lapp03.rno.apple.com (Oracle Communications Messaging Server 8.1.0.7.20201203 64bit (built Dec 3 2020)) with ESMTPSA id <0QQI00DI6AWC1L00@rn-mailsvcp-mmp-lapp03.rno.apple.com>; Wed, 24 Mar 2021 20:55:25 -0700 (PDT) From: "Andrew Fish" Message-id: MIME-version: 1.0 (Mac OS X Mail 14.0 \(3654.20.0.2.1\)) Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob Date: Wed, 24 Mar 2021 20:55:24 -0700 In-reply-to: Cc: "lersek@redhat.com" , Benjamin Doron To: edk2-devel-groups-io , ray.ni@intel.com References: <8ecc6ec1-f7a0-7878-ef57-b58f99bc3af2@redhat.com> <8402.1616604928827766426@groups.io> <2e602df8-b1fc-193b-f61a-86802ace9108@redhat.com> X-Mailer: Apple Mail (2.3654.20.0.2.1) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.369,18.0.761 definitions=2021-03-24_14:2021-03-24,2021-03-24 signatures=0 Content-type: multipart/alternative; boundary="Apple-Mail=_51EBC5C4-530B-4658-8526-193A1B18A808" --Apple-Mail=_51EBC5C4-530B-4658-8526-193A1B18A808 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Ray, The other option would be to define a Library Class for the AcpiTableDxe d= river to consume and have a NULL version of it in the MdeModulePkg. That wo= uld allow more flexibility? It kind of depends if you want to make the impl= ementation depend on a HOB or a LibraryClass? It at least gives the non pur= e PI implementations more potential options? I=E2=80=99m not sure with the = problem you are trying to solve that helps or hurts, but I though I would t= hrow out another option. The other advantage about the lib is could let you= define the HOB in another package, if that makes more sense.=20 Not trying to slow down the solution, but just giving some other options i= n case another partitioning of this problem would be helpful?=20 Thanks, Andrew Fish=20 > On Mar 24, 2021, at 6:10 PM, Ni, Ray wrote: >=20 > 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 th= e table installation in the callback. >=20 > 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 provi= des the ACPI table. > I can see the similarity between the two: the ACPI table is produced bef= ore 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 structur= e as below in MdeModulePkg/Include/Guid/Acpi.h. > typedef struct {=20 > 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_TA= BLE_INFO. >=20 >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Laszlo E= rsek >> Sent: Thursday, March 25, 2021 2:33 AM >> To: Benjamin Doron ; devel@edk2.groups.io >> Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver instal= l Acpi table from Hob >>=20 >> On 03/24/21 17:55, Benjamin Doron wrote: >>>>=20 >>>>=20 >>>>> Hi all, >>>>> Would it be acceptable/feasible for AcpiTableDxe or AcpiPlatformDxe = (in >>>>> MdeModulePkg) to use `EfiGetSystemConfigurationTable` to get the RSD= P >>>>> 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)= . >>>>=20 >>>> I'm sorry, I don't understand how this would help. >>>=20 >>> As I understand it, the issue is that this patchset changes MdeModuleP= kg to do platform-specific parsing. >>>=20 >>> Perhaps it would be an acceptable solution for platforms to retrieve t= he tables, then add >>> RSDP/them to the configuration table to be installed by AcpiTableDxe/A= cpiPlatformDxe. >>> This allows MdeModulePkg to abstract away the parsing, only installing= tables >>> available to it. >>=20 >> 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. >>=20 >> 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? >>=20 >> Sorry about being dense. :) >>=20 >>> (Currently, UefiPayloadPkg's BlSupportDxe retrieves the data from a HO= B and calls >>> `gBS->InstallConfigurationTable` with the address of RSDP). >>>=20 >>> I understand that this may not work for OVMF if tables are located dif= ferently in memory. >>>=20 >>>>=20 >>>>=20 >>>>> Regarding UefiPayloadPkg: AcpiTableDxe is currently compiled (listed= in >>>>> DSC) but not added to a FV (not listed in FDF). So, how has this bee= n >>>>> tested? >>>>=20 >>>> I assume through an out-of-tree platform. Many such core modules exis= t >>>> in edk2 that are not consumed by any of the virtual platforms in the >>>> edk2 repo itself (EmulatorPkg, ArmVirtPkg, OvmfPkg). >>>=20 >>> This makes sense, but AcpiTableDxe must be added to UefiPayloadPkg's F= DF >>> if patch 2/2 is merged. Otherwise, ACPI tables will not be advertised. >>>=20 >>=20 >> Good point; I missed that. (I'm not familiar with the UefiPayloadPkg FD= F >> at all.) >>=20 >> Laszlo >>=20 >>=20 >>=20 >>=20 >>=20 >=20 >=20 >=20 >=20 --Apple-Mail=_51EBC5C4-530B-4658-8526-193A1B18A808 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8 Ray,

The other option would be to define a Libra= ry 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 depen= ds 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=E2=80=99m not sure with the problem you are trying to solve that helps o= r 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 opt= ions 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 understan= d your point.
The purpo= se of changing the AcpiTableDxe to directly consume the HOB is to reduce th= e overall system complexity.
<= span style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size:= 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; = letter-spacing: normal; text-align: start; text-indent: 0px; text-transform= : none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: = 0px; text-decoration: none; float: none; display: inline !important;" class= = =3D"">The complexity I see with your option is:
1. platform needs to include that driver to consu= me the ACPI table from bootloader
2. that driver (consume HOB and install table through AcpiTab= le protocol) needs to register a protocol notification on AcpiTable protoco= l and do the table installation in the callback.

Laszlo,
The change here is to mee= t 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: t= he ACPI table is produced before the UEFI Payload runs.
Can you review the new option below and se= e whether it can meet the OVMF needs as well?
1. Define a new GUID gPldHobAcpiTable in MdeModulePk= g.dec and a structure as below in MdeModulePkg/Include/Guid/Acpi.h.<= br style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 1= 2px; font-style: normal; font-variant-caps: normal; font-weight: normal; le= tter-spacing: normal; text-align: start; text-indent: 0px; text-transform: = none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0p= x; text-decoration: none;" class=3D"">typedef struct { 
  UINT64        = ;          TableAddress;<= /span>
}  PLD_HOB_ACPI_TA= BLE;
2. Change AcpiTabl= eDxe 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@edk= 2.groups.io <deve= l@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Thur= sday, March 25, 2021 2:33 AM
To: Benjamin Doron <benjamin.doron00@gmail.c= om>; devel@edk2.g= roups.io
Subject: Re: [edk2-devel] [Patch V2 0/2] Let Acp= iTableDxe driver install Acpi table from Hob

O= n 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 RSDPand then install the tables? It's a solution that uses the regu= lar
UefiLib, so it avoids platform-specific quirks (and as I = see it, if RSDP
is in the configuration table, we probably al= ways want those tables).

I'm sorr= y, I don't understand how this would help.

As I understand it, the issue is that this patchset changes MdeMo= dulePkg to do platform-specific parsing.

Perha= ps it would be an acceptable solution for platforms to retrieve the tables,= then add
RSDP/them to the configuration table to be installe= d by AcpiTableDxe/AcpiPlatformDxe.
This allows MdeModulePkg t= o abstract away the parsing, only installing tables
available= to it.

But this is already the b= est approach, and already what's happening --
when you call E= FI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() from the
platform's= AcpiPlatformDxe, that's *how* you tell AcpiTableDxe in
MdeMo= dulePkg 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_TA= BLE_PROTOCOL member
function for taking a forest of ACPI tabl= es, passed in by RSD PTR?

Sorry about being de= nse. :)

(= 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 fo= r OVMF if tables are located differently in memory.



<= blockquote type=3D"cite" class=3D"">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. Ma= ny such core modules exist
in edk2 that are not consumed by a= ny of the virtual platforms in the
edk2 repo itself (Emulator= Pkg, ArmVirtPkg, OvmfPkg).

This m= akes sense, but AcpiTableDxe must be added to UefiPayloadPkg's FDF
if patch 2/2 is merged. Otherwise, ACPI tables will not be advertise= d.


Good point; I m= issed that. (I'm not familiar with the UefiPayloadPkg FDF
at = all.)

Laszlo







=
--Apple-Mail=_51EBC5C4-530B-4658-8526-193A1B18A808--