From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 5ADBB803D3 for ; Sun, 19 Mar 2017 19:15:17 -0700 (PDT) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP; 19 Mar 2017 19:15:17 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,191,1486454400"; d="scan'208";a="77186748" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga005.jf.intel.com with ESMTP; 19 Mar 2017 19:15:16 -0700 Received: from fmsmsx121.amr.corp.intel.com (10.18.125.36) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 19 Mar 2017 19:15:16 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx121.amr.corp.intel.com (10.18.125.36) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 19 Mar 2017 19:15:16 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.132]) with mapi id 14.03.0248.002; Mon, 20 Mar 2017 10:15:06 +0800 From: "Zeng, Star" To: Ard Biesheuvel , "edk2-devel@lists.01.org" , "lersek@redhat.com" , "leif.lindholm@linaro.org" CC: "Tian, Feng" , "Yao, Jiewen" , "Zeng, Star" Thread-Topic: [RFC PATCH] MdeModulePkg/AcpiTableDxe: inhibit table publication until we have a FADT Thread-Index: AQHSoCptpYufOO/UZ0meFXZ2BZKQDqGc/E7w Date: Mon, 20 Mar 2017 02:15:05 +0000 Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B835AFC@shsmsx102.ccr.corp.intel.com> References: <20170318205824.13326-1-ard.biesheuvel@linaro.org> In-Reply-To: <20170318205824.13326-1-ard.biesheuvel@linaro.org> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [RFC PATCH] MdeModulePkg/AcpiTableDxe: inhibit table publication until we have a FADT X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Mar 2017 02:15:17 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable In fact, I prefer the Laszlo's proposal to prevent the production of EFI_AC= PI_TABLE_PROTOCOL by NULL library class, but not the special handling in co= re module AcpiTableDxe. Then AcpiTableDxe could be not run(by adding DEPEX in NULL library class) o= r be unloaded(by returning unsuccessful in constructor of NULL library clas= s), and the modules or codes depend on EFI_ACPI_TABLE_PROTOCOL could be not= run. Thanks, Star -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]=20 Sent: Sunday, March 19, 2017 4:58 AM To: edk2-devel@lists.01.org; lersek@redhat.com; leif.lindholm@linaro.org; Z= eng, Star Cc: Tian, Feng ; Ard Biesheuvel Subject: [RFC PATCH] MdeModulePkg/AcpiTableDxe: inhibit table publication u= ntil we have a FADT Since commit 78c41ff519b1 ("ArmVirtPkg/FdtClientDxe: make DT table installa= tion !ACPI dependent"), we inhibit the installation of the core ACPI tables= when booting in DT mode, to relieve the OS from having to decide which har= dware description to use. However, as it turns out, in presence of the Ramd= iskDxe or BGRT drivers, some ACPI tables are still registered with the prot= ocol, and published under the ACPI entry point configuration tables, ignori= ng the fact that there is no way the OS can boot with only NFIT and BGRT ta= bles present. So let's only publish the ACPI tables if we can reasonably assume that the = tables that the OS requires have been registered with the protocol, by chec= king that we have either FADT1 or FADT3 table (or both) before installing t= he configuration tables. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- This is a counterproposal for Laszlo's series [0], which is technically sou= nd, but a bit unwieldy. Instead of making ARM the odd one out, and tweaking= universal modules via NULL library class resolution, we can prevent AcpiTa= bleDxe from publishing the entry points when only NFIT or BGRT tables are p= resent (which sounds like a strange thing to do in any case) I am aware that this still leaves some loose ends when it comes to ordering= and the ReadyToBoot callback, which I agree we should solve regardless, bu= t I hope we can do so without putting the burden on the ARM platforms to al= ways carry a set of NULL class libraries to keep sane behavior. [0] https://lists.01.org/pipermail/edk2-devel/2017-March/008684.html MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 15 ++++++++= +++++++ 1 file changed, 15 insertions(+) diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c b= /MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c index 4bb848df5203..4c9921f69a8a 100644 --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c @@ -130,6 +130,21 @@ PublishTables ( UINT64 Buffer64; =20 // + // Don't publish anything yet if we don't have a FADT table. This=20 + table is // mandatory for all ACPI compatible OSes, and installing=20 + the ACPI entry // point configuration table without a full set of=20 + ACPI tables may confuse // some OSes (Linux/arm64). (This may happen=20 + when the BGRT or ramdisk drivers // publish their respective ACPI=20 + tables without regard for whether ACPI boot // is currently enabled.) =20 + // if (AcpiTableInstance->Fadt1 =3D=3D NULL && AcpiTableInstance->Fadt3= =20 + =3D=3D NULL) { + DEBUG ((DEBUG_INFO, + "%a: not publishing ACPI tables [yet], no FADT table installed\n", + __FUNCTION__)); + return EFI_SUCCESS; + } + + // // Reorder tables as some operating systems don't seem to find the // FADT correctly if it is not in the first few entries // -- 2.9.3