From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by mx.groups.io with SMTP id smtpd.web12.1856.1616563821380301776 for ; Tue, 23 Mar 2021 22:30:22 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@intel.onmicrosoft.com header.s=selector2-intel-onmicrosoft-com header.b=YrAXy2jV; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: ray.ni@intel.com) IronPort-SDR: Ualbbf7j2oQtFrDs8NU60ML1gp4jqOKrqrLTyg+zrOTc+8WspY8foIWqg76iW1s2jWf52sO1SY buq2kkQn+7ng== X-IronPort-AV: E=McAfee;i="6000,8403,9932"; a="169970674" X-IronPort-AV: E=Sophos;i="5.81,272,1610438400"; d="scan'208,217";a="169970674" Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Mar 2021 22:30:19 -0700 IronPort-SDR: GB84EmUoIIZ7l15Dd7kNZJuZqqBdBDVxZY71pDm/xS8E+aodSZO09/P1CS8qnjiSIWfOnX2qoD R18K0OO2aaYw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.81,272,1610438400"; d="scan'208,217";a="442053596" Received: from fmsmsx603.amr.corp.intel.com ([10.18.126.83]) by fmsmga002.fm.intel.com with ESMTP; 23 Mar 2021 22:30:19 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx603.amr.corp.intel.com (10.18.126.83) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Tue, 23 Mar 2021 22:30:19 -0700 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx611.amr.corp.intel.com (10.18.126.91) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Tue, 23 Mar 2021 22:30:18 -0700 Received: from fmsedg601.ED.cps.intel.com (10.1.192.135) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2 via Frontend Transport; Tue, 23 Mar 2021 22:30:18 -0700 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (104.47.55.102) by edgegateway.intel.com (192.55.55.70) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2106.2; Tue, 23 Mar 2021 22:30:18 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ICUYWr2dL5nuoa4MHByAMrC5uqBcc3uW9CMbxkkoDMNyfb7WIxotg6hd1JCWy0We7+bghZdCa1OyUabfha1pPWA2P7N7HeMM1abjxxw2aOlP1IvC9naaqGjiG2kdMPsVaq5qTBceRLFBCOMS0JDcH3ET35i1vvY4u/rlprBAkfylS63oWjPCrPdJrbe5ARRFUYuRV+YStAL/VCsDTmbL4YqiPk2lxJMOTVXGnpqeLIe2seI7EwZs/ltXaVZZAIJ2AmdzK5kwpy3dEhUkPjlURbDO4pl/knIblIdrrJKPAIInY0O0T5Ff5qxiKEp+EnNJB/GYV3hksecZmF91rk3Opg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mO5JKJpGFKMHtcGtg+JLIb6+ZaLjsTrV25HkSNPO1sI=; b=hJmXseyqAan2ybaKYNyc1jsBBhRmZUffGZXHde2cK1Eafjj9ylldWJJyRrcd5VTbA+V32fxyNJ2R0UkLfFrZnTXdKjxfzo0pXK0PAc8b+8W/mWXLHTJnjIfnbZE8zQomsgbcw/0w0nfAgT0pv0/wOU2LQsAZ8xmcGL0E67VfofkGZc2xotvB/VBEdSRzRiAZxtZ7apQgQ90PxOGf+P9JQFlEpW1N2emDkW0ntCIFDtW5pSHta/3qKAcTh5vM8Iakptxrb910uJqoj2y48srbfwLFWI4crMbIVJ0gGe4bVvx+8MHLY9xWhmo5+GlZVoQ5kT0H4goV2ClRXbO1q+gB9Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=mO5JKJpGFKMHtcGtg+JLIb6+ZaLjsTrV25HkSNPO1sI=; b=YrAXy2jVpbbGHwxkJHM+mdUhWfmsuhMJL813dVUCTsaiiR6QEH4E7JB8b+cTB4ZfTEr44xr4cSHuAiEMgEjtuHLT4wioK8BLaQNtjv//KkqA0OLVY1Dw7WdqxiZA9ibR6IWatk9lDwEo7YEeJxWnNFJHTguoZcWdXfkaycFBcOE= Received: from CO1PR11MB4930.namprd11.prod.outlook.com (2603:10b6:303:9b::11) by MWHPR11MB1328.namprd11.prod.outlook.com (2603:10b6:300:2b::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3977.25; Wed, 24 Mar 2021 05:30:16 +0000 Received: from CO1PR11MB4930.namprd11.prod.outlook.com ([fe80::8d64:91ed:c259:e95]) by CO1PR11MB4930.namprd11.prod.outlook.com ([fe80::8d64:91ed:c259:e95%7]) with mapi id 15.20.3955.027; Wed, 24 Mar 2021 05:30:16 +0000 From: "Ni, Ray" To: Andrew Fish , edk2-devel-groups-io , "Dong, Guo" CC: "lersek@redhat.com" , "Liu, Zhiguang" , "Wang, Jian J" , "Wu, Hao A" , "Bi, Dandan" , Liming Gao Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob Thread-Topic: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install Acpi table from Hob Thread-Index: AQHXH6MuKmDiE1g/vES5qIAg1KwXHaqRpRSQgAAaUACAAN4VsA== Date: Wed, 24 Mar 2021 05:30:16 +0000 Message-ID: References: <20210323032438.950-1-zhiguang.liu@intel.com> <20210323032438.950-3-zhiguang.liu@intel.com> <41831DD2-0EBC-4B29-B6A0-BE56F322F478@apple.com> In-Reply-To: <41831DD2-0EBC-4B29-B6A0-BE56F322F478@apple.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: apple.com; dkim=none (message not signed) header.d=none;apple.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.147.194] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: e4f89e8f-e431-4468-43fe-08d8ee85e7f2 x-ms-traffictypediagnostic: MWHPR11MB1328: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 1xQlghY/lYJrsC97iJD06sq7112DoLqTZ4KpPhtjoCBPhvLu3LnWuvQLt76ps2ixdUzxm/2b6ZbXZDGF9MnnfdRpJP6I8WwnvjJlW8N6LIw8/SoqO3+lm6UXUrsr0TurXErD0gx6QVqSjJKOeX0uwdaLkhDEf8hKb7QQU+aeDMYQUn/bbmiAu377nJzcS2r2Tdz8CRIDauBg8vWfSylJNuZlMyQ7/TIKEQlX3HiBnxtW7EkwJX/OXqWWYQg9twKV6ru5N4F43+uCP4QwOQQYyFAJEGUaLghrVoze/+98rtmd6lYDvX/AWsEPiRk8wRZBgu+WeXcgy2s7CVihehpZS0ShVwHNritzpzBbeTTa0Z3QY1hfc2bvZ0Dz6sAZl6hEVKJ9aA60N2v16jAt97CT/ioKFsXj8IifexOScPXxrHUbvQs2TRd9Da8EZY/ftPxfsWiYIBjbteCnswogeLGTtzRzYqE4UCOhyHCAbjkTcdR1spJ8n2i7ntmmUttAjZ5u2D8D3t+Hmr6dXV/Tw9JFIA3EN2WpsUCVT8VkCvXQQ39GS1qelhN7Y53aPAM/ORxo0NGNQUxEog4/GNbAtN5TI8z6KxBhg9JgZJ1ckO42H5aJqnmLrnx17P2a6lwdvcEqqXvCofE0CTCL6rsmobn0KIzboUM5NiS4kD6ZO5j8O7TcOUYGy/dyu6x8KY9DmO2oGr3qZErEGe36TYPXX/GcPu+RQ7oi5jiGvwvPsY5fzgw9AaIcgxGO2G7Lnk+IrR0b x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CO1PR11MB4930.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(376002)(346002)(366004)(136003)(396003)(39860400002)(7696005)(478600001)(966005)(53546011)(6506007)(26005)(110136005)(54906003)(186003)(4326008)(316002)(2906002)(55016002)(45080400002)(71200400001)(9686003)(6636002)(33656002)(76116006)(86362001)(66446008)(64756008)(66556008)(66476007)(52536014)(66946007)(5660300002)(166002)(8676002)(83380400001)(38100700001)(8936002);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?4xK/vJTLXPszPseIZHwimld2MUS6VzHyy7WrGy/NtJX5MTyBLD9gbTvLze9+?= =?us-ascii?Q?JhuvPWOHa7p41ABPqkj13wWYyxe2zO5GlIDaQcko6DUKa7EobnsPOGFQ9xjc?= =?us-ascii?Q?MeEeZvqiocXO96ID3BZH/Ht79sOX+yQ+CL7Zxk4czDWxE550K6/dfyL7T6gj?= =?us-ascii?Q?V9r+0TWeXhDPPoOO3q2ZjUI5LNBMdHN3VCkSXJHFijDSIFcSMA56gY9JpRyD?= =?us-ascii?Q?PzC8KxAlnBKyKXmtdBW9KGxuDyAe6EbCg8bkNqJ+PLq4rkYW+2qa785u0CYU?= =?us-ascii?Q?C4dCYcea6eDpD6+NqDvI6f+UomhAUsA0j66Lr4MlK6FzcTZ/uB2jNqrRfiif?= =?us-ascii?Q?J/Vll1SamSTp5AnjJhKwddDEqcS0Jjum8BowgsCaEcW91tIZNAsFB8ulOGaU?= =?us-ascii?Q?tsmk9m8Gm60nbTudUptMA+U4CAi9r8Gforh58+x05FdoO5t0GdPlzoZARVsc?= =?us-ascii?Q?s8GoMU1jJOvlBJLR5g67F204TuyDgmR278ABzMJKR6S1OCZ9DANGuh9wzvSj?= =?us-ascii?Q?DzxfaPB6vkIj2f/k0K8ais2EjYDkJvJ8TWdPl2M6y8NUovNPf8LXkqIP+DvT?= =?us-ascii?Q?ScwZar8b+RYPAOwiPb35Wck0zSbXOqHaCqckVcHM0JXVUTiZCAG8JcZT86Vf?= =?us-ascii?Q?/kVsg94kamQ7TkyKh4WdxAwvw4DF4ehiOXjvgaPboTY7WzEmGyLO6FtClCh5?= =?us-ascii?Q?ErOmCCtdkyc+tj8WcTRoImWktDBH91+C7Fs/Ou7Hlv1DB/jvNUkFJbVh04zy?= =?us-ascii?Q?IcisSbK7A0oXDGr1mJJ+osh70CIvEUCy4oWmjxMU7ha/pcni6n///P9SDL4I?= =?us-ascii?Q?ntFB+gbJU1w560G6m5wEkHYJSLWtz1pnB4OQZoScTTztxLYicjrZFClbhBZA?= =?us-ascii?Q?XbcRCTQ3k922CbvqdghZOI1YYbR25iptA5qrA3y1HLms1ganUs5CJR+6HPOC?= =?us-ascii?Q?H/Ig+5GhlS1Ft1PFDUpOXDhL9BwKEPR1P6iJCJxw3RFYd9pRfh+7RNkKIrvw?= =?us-ascii?Q?lJdqEEsma/BsXYJAMkB/SGnvjGBbPUiPJkf/lmpc4+zWwaiOEHxebxiFfJoO?= =?us-ascii?Q?xuEzCpRjHAgSZciDxuveVFKzzmd6iXWaC3dqKfN58r8/0o5oYNH/9xfNAu3Y?= =?us-ascii?Q?Mn5t8dx3pLkSuuRhlc6J8xkD0d/fZz4Xf6p7bwLWwxXv92JxB/aTHeH2LHjW?= =?us-ascii?Q?tP2hKWR9JoHcGMZR6BVySByS5QWIfMQsv2EIiHB+fBwZ+nvRNjrDLKDRs4hs?= =?us-ascii?Q?mKmkxswk/JIHPVsqsE7ssSD3n0zDcgR72HhdqOC2H4vXKI90UxYH13fIpW5F?= =?us-ascii?Q?Z0VDETYRG15tdePxrwWqhtAI?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4930.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: e4f89e8f-e431-4468-43fe-08d8ee85e7f2 X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2021 05:30:16.1484 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: ts5nGDP0SC++TTxo5S6BNgmkO9RqR059o10hmZeRr77WoAkNmJgqqRlqaSWX3ojXBIZE3idPOA721nkVULr15Q== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1328 Return-Path: ray.ni@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CO1PR11MB4930E9FB94CD132B9F11A2C68C639CO1PR11MB4930namp_" --_000_CO1PR11MB4930E9FB94CD132B9F11A2C68C639CO1PR11MB4930namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Andrew, If the change is to use the gEfiAcpiTableGuid to identify another entry in= the EFI configuration table, I agree it's a violation. We position this as a pure implementation that reuses a spec defined GUID. We didn't realize that it's a violation to the spec. We could define a new GUID for the HOB data. But using the same GUID avoid= s introducing new GUID for the similar purpose. Thanks, Ray From: Andrew Fish Sent: Wednesday, March 24, 2021 12:13 AM To: edk2-devel-groups-io ; Dong, Guo Cc: lersek@redhat.com; Liu, Zhiguang ; Ni, Ray ; Wang, Jian J ; Wu, Hao A ; Bi, Dandan ; Liming Gao Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install A= cpi table from Hob My concern is gEfiAcpiTableGuid is owned by the UEFI Spec and any off labe= l usage should probably be defined by the PI Spec. Is this a code 1st proposal or just an implementation? Thanks, Andrew Fish On Mar 23, 2021, at 8:45 AM, Guo Dong > wrote: Add my comments on the ideas behind. UefiPayloadPkg is not a platform specific package, it tries to provide a g= eneric payload using platform independent Modules. This allows to reuse the= payload for different boot firmware solutions (Slim Bootloader, Coreboot, = EDK2 bootloader) on different platforms. Just like other DXE modules (e.g. variable DXE driver, firmware performan= ce DXE driver), standardizing and modularizing platform independent modules= through a HOB as the AcpiTableDxe patch did to support potential data from= PEI/bootloader is a nature way for EDKII modules reuse. Understood the con= cerns to keep AcpiTableDxe as simple as possible. I think it deserve for co= de reuse by adding PEI/bootloader HOB support. Thanks, Guo -----Original Message----- From: devel@edk2.groups.io > On Behalf Of Laszlo Ersek Sent: Tuesday, March 23, 2021 5:40 AM To: Liu, Zhiguang >;= Ni, Ray >; Dong, Guo > Cc: devel@edk2.groups.io; Wang, Jian J >; Wu, Hao A >; Bi, Dandan >; Liming Gao > Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install A= cpi table from Hob On 03/23/21 04:24, Zhiguang Liu wrote: If HOB contains APCI table information, entry point of AcpiTableDxe.inf should parse the APCI table from HOB, and install these tables. We assume the whole ACPI table (starting with EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER) is contained by a single gEfiAcpiTableGuid HOB. This way, for UefiPayloadPkg, there is no need to specially hanle acpi tab= le. Zhiguang Liu (2): MdeModulePkg/ACPI: Install ACPI table from HOB. UefiPayloadPkg: Remove code that installs APCI MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf | 3 ++- MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- ---- UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 13 ++-----= ------ UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h | 5 +---- UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 5 ++--- 5 files changed, 133 insertions(+), 27 deletions(-) Where does this idea come from? (1) There is no bugzilla for this, apparently (not linked in the commit messages anyway). (2) Also, I'm not sure if reusing an existing (and standardized) GUID for this purpose is a good idea. I think an edk2-only (MdeModulePkg-defined), brand new GUID, for the HOB, would be better. (3) I'm also not convinced at all that this *whole approach* is sound. The fact that UefiPayloadPkg has the ACPI content available to it in a particular data representation (as a HOB, for example) is just a platform trait. Why should that platform trait leak into the central implementation of the ACPI table protocol? UefiPayloadPkg can call the ACPI table protocol interfaces to install its tables. OVMF does the same -- OVMF also does not construct its own ACPI tables, but downloads them in a quirky representation from QEMU. We didn't hack the central AcpiTableDxe driver for that use case; instead, we dissected the blob (in OvmfPkg) into individual tables, and called the proper ACPI table protocol method (InstallAcpiTable()) with the individual tables. I disagree with the code complexity / platform quirk in AcpiTableDxe. At the bare minimum, this feature should be possible to compile out altogether. I don't necessarily mean a FeaturePCD; there could be a new INF file too, that shares most of the functionality with the current core driver, but also includes (from a different source file) the new logi= c. But I'd really like if this mess were kept out of MdeModulePkg altogether. It's the job of the platform ACPI driver to use the ACPI table protocol. Of course if you can show that this HOB usage is standard / publicly specified, that's different. Please do not merge this series. Laszlo --_000_CO1PR11MB4930E9FB94CD132B9F11A2C68C639CO1PR11MB4930namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Andrew,

If the change is to use the gEfiAcpiTableGuid to id= entify another entry in the EFI configuration table, I agree it’s a v= iolation.

 

We position this as a pure implementation that reus= es a spec defined GUID.

We didn’t realize that it’s a violation= to the spec.

 

We could define a new GUID for the HOB data. But us= ing the same GUID avoids introducing new GUID for the similar purpose.=

 

 

Thanks,

Ray

 

From: Andrew Fish <afish@apple.com> Sent: Wednesday, March 24, 2021 12:13 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Dong, Guo &l= t;guo.dong@intel.com>
Cc: lersek@redhat.com; Liu, Zhiguang <zhiguang.liu@intel.com>= ; Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com&= gt;; Wu, Hao A <hao.a.wu@intel.com>; Bi, Dandan <dandan.bi@intel.c= om>; Liming Gao <gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver in= stall Acpi table from Hob

 

My concern is gEfiAcpiTableGuid is owned by th= e UEFI Spec and any off label usage should probably be defined by the PI Sp= ec. 

 

Is this a code 1st proposal or just an implementati= on? 

 

Thanks,

 

Andrew Fish



On Mar 23, 2021, at 8:45 AM, Guo Dong <guo.dong@intel.com> wrote:

 


Add my comments on the ideas behind.
UefiPayloadPkg is not a platform specific package, it tries to provide a g= eneric payload using platform independent Modules. This allows to reuse the= payload for different boot firmware solutions (Slim Bootloader, Coreboot, = EDK2 bootloader) on different platforms.

Just like other DXE modules (e.g. variable DXE driver,  firmware perf= ormance DXE driver), standardizing and modularizing platform independent mo= dules through a HOB as the AcpiTableDxe patch did to support potential data= from PEI/bootloader is a nature way for EDKII modules reuse. Understood the concerns to keep AcpiTableDxe as simp= le as possible. I think it deserve for code reuse by adding PEI/bootloader = HOB support.

Thanks,
Guo


-----Original Message-----
From: devel@edk2.groups.io <devel@edk= 2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Tuesday, March 23, 2021 5:40 AM
To: Liu, Zhiguang <zhiguang.l= iu@intel.com>; Ni, Ray <ray.n= i@intel.com>; Dong,
Guo <guo.dong@intel.com> Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A
<hao.a.wu@intel.com>; Bi, = Dandan <dandan.bi@intel.com&g= t;; Liming Gao
<gaoliming@byosoft.com.cn>
Subject: Re: [edk2-devel] [Patch V2 0/2] Let AcpiTableDxe driver install A= cpi
table from Hob

On 03/23/21 04:24, Zhiguang Liu wrote:

If HOB contains APCI table information, entry poi= nt of AcpiTableDxe.inf
should parse the APCI table from HOB, and install these tables.
We assume the whole ACPI table (starting with

EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER)

is contained by a single gEfiAcpiTableGuid HOB. This way, for UefiPayloadPkg, there is no need to specially hanle acpi tab= le.

Zhiguang Liu (2):
 MdeModulePkg/ACPI: Install ACPI table from HOB.
 UefiPayloadPkg: Remove code that installs APCI

MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf   &nbs= p;|   3 ++-
MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 134

+++++++++++++++++++++++++++++++++++++++++++++++++= +++++++++++++++
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
----

UefiPayloadPkg/BlS= upportDxe/BlSupportDxe.c         &n= bsp;         |  13 ++----= -------
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h      &= nbsp;           &nbs= p;|   5 +----
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf      = ;           |  =  5 ++---
5 files changed, 133 insertions(+), 27 deletions(-)


Where does this idea come from?

(1) There is no bugzilla for this, apparently (not linked in the commit messages anyway).

(2) Also, I'm not sure if reusing an existing (and standardized) GUID
for this purpose is a good idea. I think an edk2-only
(MdeModulePkg-defined), brand new GUID, for the HOB, would be better.

(3) I'm also not convinced at all that this *whole approach* is sound.

The fact that UefiPayloadPkg has the ACPI content available to it in a
particular data representation (as a HOB, for example) is just a
platform trait. Why should that platform trait leak into the central
implementation of the ACPI table protocol?

UefiPayloadPkg can call the ACPI table protocol interfaces to install
its tables. OVMF does the same -- OVMF also does not construct its own
ACPI tables, but downloads them in a quirky representation from QEMU. We didn't hack the central AcpiTableDxe driver for that use case; instead, we dissected the blob (in OvmfPkg) into individual tables, and called
the proper ACPI table protocol method (InstallAcpiTable()) with the
individual tables.

I disagree with the code complexity / platform quirk in AcpiTableDxe. At the bare minimum, this feature should be possible to compile out
altogether. I don't necessarily mean a FeaturePCD; there could be a new INF file too, that shares most of the functionality with the current
core driver, but also includes (from a different source file) the new logi= c.

But I'd really like if this mess were kept out of MdeModulePkg
altogether. It's the job of the platform ACPI driver to use the ACPI
table protocol.

Of course if you can show that this HOB usage is standard / publicly
specified, that's different.

Please do not merge this series.

Laszlo







 

--_000_CO1PR11MB4930E9FB94CD132B9F11A2C68C639CO1PR11MB4930namp_--