From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (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 E421F8040E for ; Thu, 16 Mar 2017 17:59:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=intel.com; i=@intel.com; q=dns/txt; s=intel; t=1489712378; x=1521248378; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=bh+cj4HygDPskDoZ4yVd2cT0zJ4eiCMxvRIvJdtx+Oo=; b=hg0x3aut/mfjoLXV5MsI1jmaipktGTJv8lmCv80vQZPczK6D2blNzOGo +h0b2pFMO+Y4E1v61vNoGM2tsEnjZQ==; Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Mar 2017 17:59:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,174,1486454400"; d="scan'208";a="945269449" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga003.jf.intel.com with ESMTP; 16 Mar 2017 17:59:38 -0700 Received: from fmsmsx119.amr.corp.intel.com (10.18.124.207) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 16 Mar 2017 17:59:38 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX119.amr.corp.intel.com (10.18.124.207) with Microsoft SMTP Server (TLS) id 14.3.248.2; Thu, 16 Mar 2017 17:59:37 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.204]) with mapi id 14.03.0248.002; Fri, 17 Mar 2017 08:59:34 +0800 From: "Fan, Jeff" To: Laszlo Ersek , "Zeng, Star" , "edk2-devel@lists.01.org" CC: "Yao, Jiewen" Thread-Topic: [PATCH] MdeModulePkg/AcpiTableDxe: Not make FADT.{DSDT,X_DSDT} mutual exclusion Thread-Index: AQHSni3Q9AI5uXPvMkiHt9T95ZG1MqGW1v2AgAFgBRA= Date: Fri, 17 Mar 2017 00:59:33 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4C562B18@shsmsx102.ccr.corp.intel.com> References: <1489652229-8940-1-git-send-email-star.zeng@intel.com> <1002c38c-8bdf-d51a-3bc4-b604178d1295@redhat.com> In-Reply-To: <1002c38c-8bdf-d51a-3bc4-b604178d1295@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDE1NmRmODEtZjJmOC00MGU3LTg3ZjgtZWIxMDhlMWVmMTg4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ilc1YTRQNmlQb255UHptVWJZUXJma2ViNzIrVVFzT3JMa2Jmb3BWQVgrYWs9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/AcpiTableDxe: Not make FADT.{DSDT, X_DSDT} mutual exclusion 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: Fri, 17 Mar 2017 00:59:39 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Tested-by: Jeff Fan -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Thursday, March 16, 2017 7:59 PM To: Zeng, Star; edk2-devel@lists.01.org Cc: Fan, Jeff; Yao, Jiewen Subject: Re: [PATCH] MdeModulePkg/AcpiTableDxe: Not make FADT.{DSDT,X_DSDT}= mutual exclusion On 03/16/17 09:17, Star Zeng wrote: > 198a46d768fb90d2f9b16e26451b4814e7469eaf improved the DSDT and X_DSDT=20 > fields mutual exclusion by checking FADT revision, but that breaks=20 > some OS that has assumption to only consume X_DSDT field even the DSDT=20 > address is < 4G. >=20 > To have better compatibility, this patch is to update the code to not=20 > make FADT.{DSDT,X_DSDT} mutual exclusion, but always set both DSDT and=20 > X_DSDT fields in the FADT when the DSDT address is < 4G. >=20 > Cc: Laszlo Ersek > Cc: Jeff Fan > Cc: Jiewen Yao > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng >=20 > NOTE: This patch comes out from the discussion at=20 > https://lists.01.org/pipermail/edk2-devel/2017-March/008580.html. > --- > .../Acpi/AcpiTableDxe/AcpiTableProtocol.c | 88 +++++++++-------= ------ > 1 file changed, 34 insertions(+), 54 deletions(-) Reviewed-by: Laszlo Ersek Thank you! Laszlo > diff --git=20 > a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c=20 > b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c > index 4bb848df5203..a4fd9aff845d 100644 > --- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c > +++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c > @@ -432,50 +432,6 @@ ReallocateAcpiTableBuffer ( } > =20 > /** > - Determine whether the FADT table passed in as parameter requires=20 > mutual > - exclusion between the DSDT and X_DSDT fields. (That is, whether=20 > there exists > - an explicit requirement that at most one of those fields is=20 > permitted to be > - nonzero.) > - > - @param[in] Fadt The EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE object = to > - check. > - > - @retval TRUE Fadt requires mutual exclusion between DSDT and X_DSD= T. > - @retval FALSE Otherwise. > -**/ > -BOOLEAN > -RequireDsdtXDsdtExclusion ( > - IN EFI_ACPI_3_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt > - ) > -{ > - // > - // Mantis ticket #1393 was addressed in ACPI 5.1 Errata B.=20 > Unfortunately, we > - // can't tell apart 5.1 Errata A and 5.1 Errata B just from looking=20 > at the > - // FADT table. Therefore let's require exclusion for table versions >= =3D 5.1. > - // > - // While this needlessly covers 5.1 and 5.1A too, it is safer to=20 > require > - // DSDT<->X_DSDT exclusion for lax (5.1, 5.1A) versions of the spec=20 > than to > - // permit DSDT<->X_DSDT duplication for strict (5.1B) versions of the = spec. > - // > - // The same applies to 6.0 vs. 6.0A. While 6.0 does not require the > - // exclusion, 6.0A and 6.1 do. Since we cannot distinguish 6.0 from=20 > 6.0A > - // based on just the FADT, we lump 6.0 in with the rest of >=3D 5.1. > - // > - if ((Fadt->Header.Revision < 5) || > - ((Fadt->Header.Revision =3D=3D 5) && > - (((EFI_ACPI_5_1_FIXED_ACPI_DESCRIPTION_TABLE *)Fadt)->MinorVersio= n =3D=3D 0))) { > - // > - // version <=3D 5.0 > - // > - return FALSE; > - } > - // > - // version >=3D 5.1 > - // > - return TRUE; > -} > - > -/** > This function adds an ACPI table to the table list. It will detect FA= CS and > allocate the correct type of memory and properly align the table. > =20 > @@ -692,11 +648,23 @@ AddTableToList ( > } > if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) { > AcpiTableInstance->Fadt3->Dsdt =3D (UINT32) (UINTN) AcpiTableIns= tance->Dsdt3; > - if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) { > - Buffer64 =3D 0; > - } else { > - Buffer64 =3D AcpiTableInstance->Fadt3->Dsdt; > - } > + // > + // Comment block "the caller installs the tables in "DSDT, FADT"= order" > + // The below comments are also in "the caller installs the table= s in "FADT, DSDT" order" comment block. > + // > + // The ACPI specification, up to and including revision 5.1 Erra= ta A, > + // allows the DSDT and X_DSDT fields to be both set in the FADT. > + // (Obviously, this only makes sense if the DSDT address is repr= esentable in 4 bytes.) > + // Starting with 5.1 Errata B, specifically for Mantis 1393 , > + // the spec requires at most one of DSDT and X_DSDT fields to be= set to a nonzero value, > + // but strangely an exception is 6.0 that has no this requiremen= t. > + // > + // Here we do not make the DSDT and X_DSDT fields mutual exclusi= on conditionally > + // by checking FADT revision, but always set both DSDT and X_DSD= T fields in the FADT > + // to have better compatibility as some OS may have assumption t= o only consume X_DSDT > + // field even the DSDT address is < 4G. > + // > + Buffer64 =3D AcpiTableInstance->Fadt3->Dsdt; > } else { > AcpiTableInstance->Fadt3->Dsdt =3D 0; > Buffer64 =3D (UINT64) (UINTN) AcpiTableInstance->Dsdt3; @@=20 > -896,11 +864,23 @@ AddTableToList ( > if (AcpiTableInstance->Fadt3 !=3D NULL) { > if ((UINT64)(UINTN)AcpiTableInstance->Dsdt3 < BASE_4GB) { > AcpiTableInstance->Fadt3->Dsdt =3D (UINT32) (UINTN) AcpiTableI= nstance->Dsdt3; > - if (RequireDsdtXDsdtExclusion (AcpiTableInstance->Fadt3)) { > - Buffer64 =3D 0; > - } else { > - Buffer64 =3D AcpiTableInstance->Fadt3->Dsdt; > - } > + // > + // Comment block "the caller installs the tables in "FADT, DSD= T" order" > + // The below comments are also in "the caller installs the tab= les in "DSDT, FADT" order" comment block. > + // > + // The ACPI specification, up to and including revision 5.1 Er= rata A, > + // allows the DSDT and X_DSDT fields to be both set in the FAD= T. > + // (Obviously, this only makes sense if the DSDT address is re= presentable in 4 bytes.) > + // Starting with 5.1 Errata B, specifically for Mantis 1393 , > + // the spec requires at most one of DSDT and X_DSDT fields to = be set to a nonzero value, > + // but strangely an exception is 6.0 that has no this requirem= ent. > + // > + // Here we do not make the DSDT and X_DSDT fields mutual exclu= sion conditionally > + // by checking FADT revision, but always set both DSDT and X_D= SDT fields in the FADT > + // to have better compatibility as some OS may have assumption= to only consume X_DSDT > + // field even the DSDT address is < 4G. > + // > + Buffer64 =3D AcpiTableInstance->Fadt3->Dsdt; > } else { > AcpiTableInstance->Fadt3->Dsdt =3D 0; > Buffer64 =3D (UINT64) (UINTN) AcpiTableInstance->Dsdt3; >=20