From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mx.groups.io with SMTP id smtpd.web11.18023.1684231579362901569 for ; Tue, 16 May 2023 03:06:19 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="unable to parse pub key" header.i=@intel.com header.s=intel header.b=LlB+jfk+; spf=pass (domain: intel.com, ip: 134.134.136.31, mailfrom: vincentx.ke@intel.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684231579; x=1715767579; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=4vShIYtOan7JuKkx6DclI9gGfZyrRltlgsqQvjF9C4g=; b=LlB+jfk+2sNFFPiQSmkoUnZcw3XdLC/KDI6VJj0Vk68VCNIRBRWspg2z utpLHq0UPd1U9IkvPS7pyIrkQm26YPLBUEzRH+MGZje05wq9e1H/LBl/F IqhXa2tTpQpUCyW2Uaeus3Vd+WGWaPy3yaLSjZXFztPLoTTqo92Dh1EgZ o8X8VIjRJvg3HEyt6hkzr818tKRRRjewc4wk9Xnf12JA6zGcIGmO5qXcS yflJWss3o6kKb+Fj2WRftN8aXtuRWJAZhtoU91JWewp/Ah+7a8gRAxpnk JA75Ir85eFA6dugXYGSiIEwXrkR1R0Z+pwKvTlCzIKdRxlxshqPXza8zk Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10711"; a="414844939" X-IronPort-AV: E=Sophos;i="5.99,278,1677571200"; d="scan'208";a="414844939" Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 May 2023 03:06:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10711"; a="947785205" X-IronPort-AV: E=Sophos;i="5.99,278,1677571200"; d="scan'208";a="947785205" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga006.fm.intel.com with ESMTP; 16 May 2023 03:06:18 -0700 Received: from fmsmsx611.amr.corp.intel.com (10.18.126.91) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Tue, 16 May 2023 03:06:18 -0700 Received: from fmsmsx601.amr.corp.intel.com (10.18.126.81) 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.2507.23; Tue, 16 May 2023 03:06:17 -0700 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23 via Frontend Transport; Tue, 16 May 2023 03:06:17 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.176) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.23; Tue, 16 May 2023 03:06:17 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KrJefe8V5rOtHnslc4Cb7uRhpOxPywwhl+ckskenPCBdTvRux4BNFKHcAXp00joQGp1i6AoCi9dSEpzF3Riym4KmOWFnl/rHadfJ0NZ9VlD6I1AnZHNOynMcwnS/g/1ZRd0eYMhjDKX+49EHKmQb4gvxuu7ZR2jwfLPTlcji9TP6FmfVNPX9hViL1BmLeXVv2mAKjFnDJJruTlIyLafNTdT1s66RFL1nV0u2f6x8tb8SM8YrLgMiAsXqwYlfc4K6U9prlV2Aups60x5ioZkNgvguCdL/qgkvqaQEP28t9vDEs09hgnn2dmo0mbZtY1LEe1RTvWkHhEZ9tNGvIxBVXA== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=C7VJtEoAqNI++eKmzGW+pesf/nBGZs5dtM7BUyQulzQ=; b=oVqRwaWctQ6yWjzw7Bztfq9d7PlfDBBJgfksJKofWxbxgUoAzIjlOohM7JYWcNhP1F5Uq1OUzqjqv6Rj1zfEJUmOMAih8not7gEG7W8Nw2wUPMzmId2iV/maLslibsUVo0y0boXlpnobJ9ntHGjhJbqffm989eIHnI0McjhJn395EowrkwppYQW7MeGzyK+Kuj91xE1h9VSaJnd3Yt8QjRob1wzvuNX7oZfbvNr8VCjDuE5rxzFsSKQC+6C6La3qG2GK1hpnUwReItJPujy3pXj94S5YekSaO0ouj21sAX4wtZFe71pXy+SAnuaI5nubeo/LRuMwpAVA6jjyNlwcZw== 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 Received: from DM4PR11MB5487.namprd11.prod.outlook.com (2603:10b6:5:39f::22) by CO1PR11MB5155.namprd11.prod.outlook.com (2603:10b6:303:91::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6387.32; Tue, 16 May 2023 10:06:09 +0000 Received: from DM4PR11MB5487.namprd11.prod.outlook.com ([fe80::5b26:2660:6aaf:37f9]) by DM4PR11MB5487.namprd11.prod.outlook.com ([fe80::5b26:2660:6aaf:37f9%3]) with mapi id 15.20.6387.030; Tue, 16 May 2023 10:06:09 +0000 From: "VincentX Ke" To: "Ni, Ray" , "devel@edk2.groups.io" CC: "Chiu, Chasel" , "Desimone, Nathaniel L" , "Oram, Isaac W" , "Gao, Liming" , "Dong, Eric" , "Sinha, Ankit" Subject: Re: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature filed in FACS Thread-Topic: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature filed in FACS Thread-Index: AQHZg+9qMoyfP8iEjUi/H3/NG1SRnK9WYYuAgAZNNmA= Date: Tue, 16 May 2023 10:06:09 +0000 Message-ID: References: <20230511095959.562-1-vincentx.ke@intel.com> In-Reply-To: Accept-Language: zh-TW, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DM4PR11MB5487:EE_|CO1PR11MB5155:EE_ x-ms-office365-filtering-correlation-id: 4ed8e7b7-521f-448f-37f5-08db55f52bd7 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: UpvXFTp07Amp8SH+Rv6A2RIk9MGHQr3t7lhgWqJZ/XXKNc7XeDKhefXgDdhcmtKvdmbJVgNIlatU5kIK+s8Nx7v945Jck3n2Z45xOKsmweThGpkZJqc+tadm++pBzPFWJa8AtA6Od0MLrtNLfCwvnat0n7zVYwtajWKThe/TFMvhJEBTJyY0ewsy6y/8zK+wtVib4rWY20S4dbeu0WrAjbrVnsqR3btVAx9NFsIWq09txJEuJ+rKtJ7ZfrnLrnxUVVgH03pXFu8CfF0In4+RGGpEeDp7nHS7O6VAD5UnBOHqmSqhlUzhePLhHdhfsq8eJwW9c+W2wg/3qSLL8nC+TYnJ9G6+uj9xRTZNpXvQaST/n90OkIqxk3Lwdbuq8OyBVp4LmiAl33a7NUiR1ub7S8Qv0yhsUWWXpN+UZ/92p3EScpkO6me0QVMKNPZIC5hgpNInsNYC7bs9DW6goQcAgx5v2hYzFG3LqAlQMa7VstmT+Jm+WDRKN6DVcU/QLhps2+09ofHNo51BeXlJRoI2r4bWSAbJnB1vsYk18DkkSf+ZasqSxmK228T7vJrqEO2MbiykTmjgHszIx3wc7+tXLo+BSpYHSduHjxj1dtxttnArcZDg6PyPHwwOzHBx9fJ1dnUXZr2WGTqnqdlc+aahrA== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM4PR11MB5487.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(136003)(346002)(396003)(39860400002)(376002)(366004)(451199021)(30864003)(5660300002)(76116006)(64756008)(66476007)(66446008)(52536014)(66556008)(2906002)(66946007)(15650500001)(38070700005)(41300700001)(316002)(86362001)(4326008)(110136005)(54906003)(8676002)(8936002)(66899021)(478600001)(71200400001)(38100700002)(122000001)(82960400001)(7696005)(966005)(186003)(33656002)(55016003)(107886003)(26005)(6506007)(53546011)(9686003)(83380400001);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?lkfy009dDcYWKk27KGK5skmnEAGNeIhr9bcGQUglzC9hpeAQb+tX0TCQwg?= =?iso-8859-1?Q?HNk3bd6B1aQEGtz/4QKb6g8B6sOcMZdYvkh2ejwZVJzLbrJrzrmZPdsub4?= =?iso-8859-1?Q?41voDi+f40fFMh5efvmafS1wdDvGeG0gUNWtIJiBrHOPYUb73fOZdrNVIG?= =?iso-8859-1?Q?1LItaP2BYgs1SOwPXYHLI+ONai3vONIfXfrWT9vrPJe/jT03/OoSLkNTlG?= =?iso-8859-1?Q?5yy0u3+iP2BFO9IXJW0XWfn3ReSUaBzzLjQOBgWKhneykB5R04W9Y+HmOW?= =?iso-8859-1?Q?oC0z8aYv1/AuvZjseT6lXAUgxiKIHp/yOpUBFXmyQGBpzoEAspI3utacg9?= =?iso-8859-1?Q?6qZNBJDsnNb3EGAANNvHcGlCdroq0NHkBrKBXFbOnra3pKLYiX/CnXo60H?= =?iso-8859-1?Q?WLrXUN9RC+mN2U0CtNOXPP93pVIgQsyx95WOhmh3DrFc+9bFT2vWa4fkHM?= =?iso-8859-1?Q?j69i1TohK+i8LgJwteUQ33x7tDzMsetYgkz3Z4XUYweJscfUF2T31CzB1V?= =?iso-8859-1?Q?3/HEp2FpNuPX5iE5gxfXagtLpIF+C0keYESoVlOnRUDwcwZiAFkxm2jhh1?= =?iso-8859-1?Q?U64zcKWSwPnPdhoBj0tPkPiiUTr4LP3rvGc4QNwPc47QDacZUa9KKZnwmX?= =?iso-8859-1?Q?dN0BQJ3+zY7OdBVkNN8aIm1B+EHi884T7VJIeCoL9QrxXr6qUrdr0WCQ3S?= =?iso-8859-1?Q?WulhPSGI94yFc+Np1Yl49N7KPhh4dD162mWihrWX0J5Sl2ydGctlTdI8/b?= =?iso-8859-1?Q?CRJml/OUOi6FgnwgX9tK/JAX4NFZFJPsGQe6cs6IR673OLHlfqXYCQd3xs?= =?iso-8859-1?Q?GgRZxchcxU1W3Dshccq0DKjKeEqOTmtmiZJ5JhqOf0WB2M7uChQ9YOHURV?= =?iso-8859-1?Q?vKdGGlQ7DdflokNvfBuEmPwF5a14oLBrCkDS85fxtqJY3iBBl3YhWuZcXr?= =?iso-8859-1?Q?YO0c2dEc+66bt5C540myiTRtJotJ8yLCE3hIOFX3fJOIBkWBFj4YipHjRi?= =?iso-8859-1?Q?FByk4gvTELebkSTpORK2heMOyNVPMSNCZVheHB7rsH3+iWDbGiqHzMxuNa?= =?iso-8859-1?Q?+VEhU7/0PhgGpQHdOe3vhup29USLxKVPfTBusc5vSV98tlrVVSKYuP55U4?= =?iso-8859-1?Q?QYOVf4ZIJfcN+SRee4ImAV/6yfQ7dCuXvhqiobHvet+uh7ysWQQ6iJgQvb?= =?iso-8859-1?Q?J0YlUDaPTUOk36uj3BkeAVwnZi/QWrzIygYLI4qp8yxvIXI4GPqYkAmjXC?= =?iso-8859-1?Q?IPtMj16tsaOjFaq6x05fzvgWRuAri2iUFFqxXDl4YpGXPdVGedDt7pD45b?= =?iso-8859-1?Q?rh/MijGPCiSWanvvoMGylF8yjMO4Jl92D0V3NpBlOHEXzoIMC2AMWncUDj?= =?iso-8859-1?Q?drF4nWeUIW52lqRmdq3MsLw0DYSWIYz3rUGvgiwsfNv83dGUc3/DKF9we7?= =?iso-8859-1?Q?sB8qsP0fSGMV7q+7urfS46HNRgrZ5FydYlPRHPrGHg7bPg0zATmAo+4r9a?= =?iso-8859-1?Q?Gcn2N2OqcC9idxCU0EQVFMo/XOUnAUi0t6D2KzQ1LVyhDpf2F8C1aNPF9Z?= =?iso-8859-1?Q?CvYRqKZN2NL5OXwSkuKt6iSciv8fUaiVm39wX7vZG6r1oMu+Z4UkXBx204?= =?iso-8859-1?Q?AD8fDNPiGRijbNtmiFOhGK9gxf0zetcwbQ?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB5487.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4ed8e7b7-521f-448f-37f5-08db55f52bd7 X-MS-Exchange-CrossTenant-originalarrivaltime: 16 May 2023 10:06:09.3674 (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: 1Hy3Dy+JCdgsrRze0bYuy2Kv4edjIJfzTA3NiT/vAGdcTktudAEXpsVHHukPdoy6y5llcU8PqVvKj7rY3r5SRQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB5155 Return-Path: vincentx.ke@intel.com X-OriginatorOrg: intel.com Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Hi, Ray Please see below replies. And thanks for the code review. #2. Different versions of FADT have different sizes. This CopyMem is danger= ous. And why do you CopyMem()? Reply: Based on https://edk2.groups.io/g/devel/topic/98821097#104899,=A0The= extern variable "Facs" will only keep the unique one in AcpiPlatform.c, "e= xtern EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE Facs" should be follow u= p if version changed. And since the purpose is calculating CRC, we only nee= d data of FACS rather than physical pointer to FACS. #5. You don't need to check the Signature. You can check if Xsdt is not 0, = use XSDT, otherwise use RSDT. Reply: To avoid the case that XSDT is garbage in RSDT only platform. Only c= hecking with "Xsdt is not 0" cannot meet the requirement. =A0#7. Why not return earlier but here? Reply: The reason to return here is AllocateZeroPool() failed or XSDT/RSDT = doesn't find out any pointer entry. This return should be kept. Sincerely, Vincent -----Original Message----- From: Ni, Ray =20 Sent: Friday, May 12, 2023 5:33 PM To: devel@edk2.groups.io; Ke, VincentX Cc: Chiu, Chasel ; Desimone, Nathaniel L ; Oram, Isaac W ; Gao, Liming = ; Dong, Eric ; Sinha, Ankit = Subject: RE: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature fil= ed in FACS > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of=20 > VincentX Ke > Sent: Thursday, May 11, 2023 6:00 PM > To: devel@edk2.groups.io > Cc: Ke, VincentX ; Chiu, Chasel=20 > ; Desimone, Nathaniel L=20 > ; Oram, Isaac W=20 > ; Gao, Liming ;=20 > Dong, Eric ; Sinha, Ankit > Subject: [edk2-devel] [PATCH v7] MinPlatformPkg: Update HWSignature=20 > filed in FACS >=20 > From: VincentX Ke >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D4428 >=20 > Calculating CRC based on each ACPI table. > Update HWSignature filed in FACS based on CRC while ACPI table changed. >=20 > Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c > Signed-off-by: VincentX Ke > Cc: Chasel Chiu > Cc: Nate DeSimone > Cc: Isaac Oram > Cc: Liming Gao > Cc: Eric Dong > Cc: Ankit Sinha > Signed-off-by: VincentX Ke > --- > Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 287 > +++++++++++++++----- > Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 + > 2 files changed, 223 insertions(+), 65 deletions(-) >=20 > diff --git=20 > a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > index e967031a3b..3dca6f99f7 100644 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c > @@ -1191,98 +1191,255 @@ PlatformUpdateTables ( } >=20 >=20 >=20 > /** >=20 > - This function calculates RCR based on PCI Device ID and Vendor ID=20 > from the devices >=20 > - available on the platform. >=20 > - It also includes other instances of BIOS change to calculate CRC=20 > and provides as >=20 > - HWSignature filed in FADT table. >=20 > + This function calculates CRC based on each offset in the ACPI table. >=20 > + >=20 > + @param[in] Table The ACPI table required to calculate CRC. >=20 > + >=20 > + @retval CRC A pointer to allocate UINT32 that >=20 > + contains the CRC32 data. >=20 > +**/ >=20 > +UINT32 >=20 > +AcpiTableCrcCalculator ( >=20 > + IN EFI_ACPI_COMMON_HEADER *Table >=20 > + ) >=20 > +{ >=20 > + EFI_STATUS Status; >=20 > + UINT32 CRC; >=20 > + >=20 > + Status =3D EFI_SUCCESS; >=20 > + CRC =3D 0; >=20 > + >=20 > + // >=20 > + // Calculate CRC value. >=20 > + // >=20 > + if (Table->Signature =3D=3D > EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) { >=20 > + // >=20 > + // Zero HardwareSignature field before Calculating FACS CRC >=20 > + // >=20 > + ((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)- > >HardwareSignature =3D 0; >=20 > + } >=20 > + >=20 > + Status =3D gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length,= =20 > + &CRC); >=20 > + return CRC; >=20 > +} >=20 > + >=20 > +/** >=20 > + This function count ACPI tables in RSDT/XSDT and return the result. >=20 > + >=20 > + @param[in] Sdt ACPI XSDT/RSDT. >=20 > + @param[in] TablePointerSize Size of table pointer: >=20 > + 4(RSDT) or 8(XSDT). >=20 > + >=20 > + @retval TableCount The total number of ACPI tables in >=20 > + RSDT or XSDT. >=20 > +**/ >=20 > +UINTN >=20 > +CountTableInSDT ( >=20 > + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt, >=20 > + IN UINTN TablePointerSize >=20 > + ) >=20 > +{ >=20 > + UINTN Index; >=20 > + UINTN TableCount; >=20 > + UINTN EntryCount; >=20 > + UINT64 EntryPtr; >=20 > + UINTN BasePtr; >=20 > + EFI_ACPI_COMMON_HEADER *Table; >=20 > + >=20 > + EntryCount =3D (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / > TablePointerSize; >=20 > + BasePtr =3D (UINTN)(Sdt + 1); >=20 > + >=20 > + for (Index =3D 0, TableCount =3D 0; Index < EntryCount; Index++) { >=20 > + EntryPtr =3D 0; >=20 > + Table =3D NULL; >=20 > + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize), > TablePointerSize); >=20 > + Table =3D (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr)); >=20 > + if (Table) { 1. if (Table !=3D NULL) >=20 > + TableCount++; >=20 > + } >=20 > + >=20 > + if (Table->Signature =3D=3D > EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) { >=20 > + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof > (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE)); 2. Different versions of FADT have different sizes. This CopyMem is dangero= us. And why do you CopyMem()? >=20 > + if (Fadt.FirmwareCtrl || Fadt.XFirmwareCtrl) { 3. if (Fadt.FirmwareCtrl !=3D 0 || ...) >=20 > + TableCount++; >=20 > + } >=20 > + >=20 > + if (Fadt.Dsdt || Fadt.XDsdt) { 4. if (Fadt. Dsdt !=3D 0 || ...) >=20 > + TableCount++; >=20 > + } >=20 > + } >=20 > + } >=20 > + >=20 > + return TableCount; >=20 > +} >=20 > + >=20 > +/** >=20 > + This function calculates CRC based on each ACPI table. >=20 > + It also calculates CRC and provides as HWSignature filed in FACS. >=20 > **/ >=20 > VOID >=20 > -IsHardwareChange ( >=20 > +IsAcpiTableChange ( >=20 > VOID >=20 > ) >=20 > { >=20 > - EFI_STATUS Status; >=20 > - UINTN Index; >=20 > - UINTN HandleCount; >=20 > - EFI_HANDLE *HandleBuffer; >=20 > - EFI_PCI_IO_PROTOCOL *PciIo; >=20 > - UINT32 CRC; >=20 > - UINT32 *HWChange; >=20 > - UINTN HWChangeSize; >=20 > - UINT32 PciId; >=20 > - UINTN Handle; >=20 > - EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr; >=20 > - EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE *pFADT; >=20 > - >=20 > - HandleCount =3D 0; >=20 > - HandleBuffer =3D NULL; >=20 > - >=20 > - Status =3D gBS->LocateHandleBuffer ( >=20 > - ByProtocol, >=20 > - &gEfiPciIoProtocolGuid, >=20 > - NULL, >=20 > - &HandleCount, >=20 > - &HandleBuffer >=20 > - ); >=20 > - if (EFI_ERROR (Status)) { >=20 > - return; // PciIO protocol not installed yet! >=20 > + EFI_STATUS Status; >=20 > + UINTN Index; >=20 > + UINTN AcpiTableCount; >=20 > + UINTN EntryCount; >=20 > + UINTN BasePtr; >=20 > + UINT64 EntryPtr; >=20 > + UINT32 *TableCrcRecord; >=20 > + UINT32 HWSignature; >=20 > + EFI_ACPI_COMMON_HEADER *Table; >=20 > + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp; >=20 > + EFI_ACPI_DESCRIPTION_HEADER *Rsdt; >=20 > + EFI_ACPI_DESCRIPTION_HEADER *Xsdt; >=20 > + EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr; >=20 > + >=20 > + Index =3D 0; >=20 > + AcpiTableCount =3D 0; >=20 > + EntryCount =3D 0; >=20 > + BasePtr =3D 0; >=20 > + EntryPtr =3D 0; >=20 > + HWSignature =3D 0; >=20 > + TableCrcRecord =3D NULL; >=20 > + Rsdp =3D NULL; >=20 > + Rsdt =3D NULL; >=20 > + Xsdt =3D NULL; >=20 > + FacsPtr =3D NULL; >=20 > + >=20 > + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__)); >=20 > + >=20 > + Status =3D EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID > **)&Rsdp); >=20 > + if (EFI_ERROR (Status) || (Rsdp =3D=3D NULL)) { >=20 > + return; >=20 > + } >=20 > + >=20 > + Rsdt =3D (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress; >=20 > + Xsdt =3D (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress; >=20 > + >=20 > + if (Xsdt->Signature !=3D > EFI_ACPI_6_5_EXTENDED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) { >=20 > + if (Rsdt->Signature !=3D > EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) { >=20 > + return; >=20 > + } >=20 > } 5. You doesn't need to check the Signature. You can check if Xsdt is not 0,= use XSDT, otherwise use RSDT. >=20 >=20 >=20 > // >=20 > - // Allocate memory for HWChange and add additional entrie for >=20 > - // pFADT->XDsdt >=20 > + // ACPI table count starts with 0. >=20 > + // Then add ACPI tables found by RSDT/XSDT and FADT. >=20 > // >=20 > - HWChangeSize =3D HandleCount + 1; >=20 > - HWChange =3D AllocateZeroPool (sizeof(UINT32) * HWChangeSize); >=20 > - ASSERT(HWChange !=3D NULL); >=20 > + AcpiTableCount =3D CountTableInSDT (Xsdt, sizeof (UINT64)); >=20 > + if (AcpiTableCount =3D=3D 0) { >=20 > + AcpiTableCount =3D CountTableInSDT (Rsdt, sizeof (UINT32)); >=20 > + } 6. No need to choose between XSDT and RSDT again. Use #5 comment to choose = between the two. >=20 >=20 >=20 > - if (HWChange =3D=3D NULL) return; 7. Why not return earlier but here? 8. I will stop reviewing. Can you please review within your team first then= send out? >=20 > + // >=20 > + // Allocate memory for founded ACPI tables. >=20 > + // >=20 > + TableCrcRecord =3D AllocateZeroPool (sizeof (UINT32) *=20 > + AcpiTableCount); >=20 > + if (TableCrcRecord =3D=3D NULL) { >=20 > + return; >=20 > + } >=20 >=20 >=20 > // >=20 > - // add HWChange inputs: PCI devices >=20 > + // Search XSDT >=20 > // >=20 > - for (Index =3D 0; HandleCount > 0; HandleCount--) { >=20 > - PciId =3D 0; >=20 > - Status =3D gBS->HandleProtocol (HandleBuffer[Index], &gEfiPciIoProto= colGuid, > (VOID **) &PciIo); >=20 > - if (!EFI_ERROR (Status)) { >=20 > - Status =3D PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, 0, 1, &Pci= Id); >=20 > - if (EFI_ERROR (Status)) { >=20 > - continue; >=20 > + AcpiTableCount =3D 0; >=20 > + EntryCount =3D (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER= )) / > sizeof (UINT64); >=20 > + BasePtr =3D (UINTN)(Xsdt + 1); >=20 > + for (Index =3D 0; Index < EntryCount; Index++) { >=20 > + EntryPtr =3D 0; >=20 > + Table =3D NULL; >=20 > + CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof=20 > + (UINT64)), > sizeof (UINT64)); >=20 > + Table =3D (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr)); >=20 > + if (Table) { >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator=20 > + (Table); >=20 > + } >=20 > + >=20 > + if (Table->Signature =3D=3D > EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) { >=20 > + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof > (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE)); >=20 > + // >=20 > + // Locate FACS in FADT >=20 > + // >=20 > + CopyMem ((VOID *)&EntryPtr, (VOID *)&Fadt.XFirmwareCtrl, sizeof > (UINT64)); >=20 > + if (EntryPtr !=3D 0) { >=20 > + FacsPtr =3D (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE > *)(UINTN)((EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table)- > >XFirmwareCtrl; >=20 > + >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator > ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr); >=20 > + } else { >=20 > + FacsPtr =3D (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE > *)(UINTN)((EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table)- > >FirmwareCtrl; >=20 > + >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator > ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl); >=20 > + } >=20 > + >=20 > + // >=20 > + // Locate DSDT in FADT >=20 > + // >=20 > + CopyMem ((VOID *)&EntryPtr, (VOID *)&Fadt.XDsdt, sizeof=20 > + (UINT64)); >=20 > + if (EntryPtr !=3D 0) { >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator > ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr); >=20 > + } else { >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator > ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt); >=20 > } >=20 > - HWChange[Index++] =3D PciId; >=20 > } >=20 > } >=20 >=20 >=20 > - // >=20 > - // Locate FACP Table >=20 > - // >=20 > - Handle =3D 0; >=20 > - Status =3D LocateAcpiTableBySignature ( >=20 > - EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE, >=20 > - (EFI_ACPI_DESCRIPTION_HEADER **) &pFADT, >=20 > - &Handle >=20 > - ); >=20 > - if (EFI_ERROR (Status) || (pFADT =3D=3D NULL)) { >=20 > - return; //Table not found or out of memory resource for pFADT table >=20 > + if (FacsPtr->Signature !=3D > EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) { >=20 > + // >=20 > + // Search RSDT >=20 > + // >=20 > + AcpiTableCount =3D 0; >=20 > + EntryCount =3D (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEAD= ER)) / > sizeof (UINT32); >=20 > + BasePtr =3D (UINTN)(Rsdt + 1); >=20 > + for (Index =3D 0; Index < EntryCount; Index++) { >=20 > + EntryPtr =3D 0; >=20 > + Table =3D NULL; >=20 > + CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof=20 > + (UINT32)), > sizeof (UINT32)); >=20 > + Table =3D (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr)); >=20 > + if (Table) { >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator=20 > + (Table); >=20 > + } >=20 > + >=20 > + if (Table->Signature =3D=3D > EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) { >=20 > + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof > (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE)); >=20 > + // >=20 > + // Locate FACS in FADT >=20 > + // >=20 > + if (Fadt.FirmwareCtrl) { >=20 > + FacsPtr =3D (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE > *)(UINTN)((EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table)- > >FirmwareCtrl; >=20 > + >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator > ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl); >=20 > + } >=20 > + >=20 > + // >=20 > + // Locate DSDT in FADT >=20 > + // >=20 > + if (Fadt.Dsdt) { >=20 > + TableCrcRecord[AcpiTableCount++] =3D AcpiTableCrcCalculator > ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt); >=20 > + } >=20 > + } >=20 > + } >=20 > } >=20 >=20 >=20 > // >=20 > - // add HWChange inputs: others >=20 > + // FACS not found >=20 > // >=20 > - HWChange[Index++] =3D (UINT32)pFADT->XDsdt; >=20 > + if (FacsPtr->Signature !=3D > EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) { >=20 > + return; >=20 > + } >=20 >=20 >=20 > // >=20 > - // Calculate CRC value with HWChange data. >=20 > + // Calculate HWSignature data. >=20 > // >=20 > - Status =3D gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC); >=20 > - DEBUG ((DEBUG_INFO, "CRC =3D %x and Status =3D %r\n", CRC, Status)); >=20 > + Status =3D gBS->CalculateCrc32 (TableCrcRecord, AcpiTableCount, > &HWSignature); >=20 > + DEBUG ((DEBUG_INFO, "HardwareSignature =3D %x and Status =3D %r\n", > HWSignature, Status)); >=20 >=20 >=20 > // >=20 > // Set HardwareSignature value based on CRC value. >=20 > // >=20 > - FacsPtr =3D (EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE > *)(UINTN)pFADT->FirmwareCtrl; >=20 > - FacsPtr->HardwareSignature =3D CRC; >=20 > - FreePool (HWChange); >=20 > + FacsPtr->HardwareSignature =3D HWSignature; >=20 > + FreePool (TableCrcRecord); >=20 > + DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__)); >=20 > } >=20 >=20 >=20 > VOID >=20 > @@ -1329,7 +1486,7 @@ AcpiEndOfDxeEvent ( > // >=20 > // Calculate Hardware Signature value based on current platform=20 > configurations >=20 > // >=20 > - IsHardwareChange (); >=20 > + IsAcpiTableChange (); >=20 > } >=20 >=20 >=20 > /** >=20 > diff --git=20 > a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf > b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf > index 694492112b..f47cc3908d 100644 > --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf > +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf > @@ -128,6 +128,7 @@ > gEfiGlobalVariableGuid ## CONSUMES >=20 > gEfiHobListGuid ## CONSUMES >=20 > gEfiEndOfDxeEventGroupGuid ## CONSUMES >=20 > + gEfiAcpiTableGuid ## CONSUMES >=20 >=20 >=20 > [Depex] >=20 > gEfiAcpiTableProtocolGuid AND >=20 > -- > 2.39.2.windows.1 >=20 >=20 >=20 > -=3D-=3D-=3D-=3D-=3D-=3D > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#104685):=20 > https://edk2.groups.io/g/devel/message/104685 > Mute This Topic: https://groups.io/mt/98824282/1712937 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]=20 > -=3D-=3D-=3D-=3D-=3D-=3D >=20