From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.136, mailfrom: zhichao.gao@intel.com) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by groups.io with SMTP; Sun, 18 Aug 2019 18:18:34 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Aug 2019 18:18:34 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,403,1559545200"; d="scan'208";a="202091599" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 18 Aug 2019 18:18:34 -0700 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 18 Aug 2019 18:18:33 -0700 Received: from shsmsx153.ccr.corp.intel.com (10.239.6.53) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 18 Aug 2019 18:18:34 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.80]) by SHSMSX153.ccr.corp.intel.com ([169.254.12.215]) with mapi id 14.03.0439.000; Mon, 19 Aug 2019 09:18:31 +0800 From: "Gao, Zhichao" To: "devel@edk2.groups.io" , "krzysztof.koch@arm.com" CC: "Carsey, Jaben" , "Ni, Ray" , "Sami.Mujawar@arm.com" , "Matteo.Carlini@arm.com" , "nd@arm.com" Subject: Re: [edk2-devel] [PATCH v1 05/11] ShellPkg: acpiview: SLIT: Validate System Locality count Thread-Topic: [edk2-devel] [PATCH v1 05/11] ShellPkg: acpiview: SLIT: Validate System Locality count Thread-Index: AQHVU2sn8cTIjNKO9Eun9NSKtXUilqcBruBw Date: Mon, 19 Aug 2019 01:18:30 +0000 Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B82386D@SHSMSX101.ccr.corp.intel.com> References: <20190815131121.52644-1-krzysztof.koch@arm.com> <20190815131121.52644-6-krzysztof.koch@arm.com> In-Reply-To: <20190815131121.52644-6-krzysztof.koch@arm.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: zhichao.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Krzysztof Koch > Sent: Thursday, August 15, 2019 9:11 PM > To: devel@edk2.groups.io > Cc: Carsey, Jaben ; Ni, Ray ; > Gao, Zhichao ; Sami.Mujawar@arm.com; > Matteo.Carlini@arm.com; nd@arm.com > Subject: [edk2-devel] [PATCH v1 05/11] ShellPkg: acpiview: SLIT: Validat= e > System Locality count >=20 > 1. Check if the 'Number of System Localities' provided can be represente= d in > the SLIT table. The table 'Length' field is a 32-bit value while the 'Nu= mber of > System Localities' field is 64-bit long. >=20 > 2. Check if the SLIT matrix fits in the table buffer. If N is the SLIT l= ocality count, > then the matrix used to represent the localities is N*N bytes long. The = ACPI > table length must be big enough to fit the matrix. >=20 > 3. Remove (now) redundant 64x64 bit multiplication. Why removing? This change is added to fixed the issue build error with IA3= 2 multiplication of two 64 bits data. The change of #3 should be removed from the patch. Keeping the variable size as UINT64 wouldn't affect the result. Thanks, Zhichao >=20 > Signed-off-by: Krzysztof Koch > --- >=20 > Notes: > v1: > - Validate the 'Number of System Localities' Field [Krzysztof] > - Remove redundant 64x64 bit multiplication [Krzysztof] >=20 > ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c = | > 47 +++++++++++++++++--- > 1 file changed, 42 insertions(+), 5 deletions(-) >=20 > diff --git > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c > index > 17e2166a09d8615b714e0c51d4d93d293fcdf601..e4625ee8b13907893a9b6990 > ecb956baf91cc3b9 100644 > --- > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c > +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitPars > +++ er.c > @@ -30,7 +30,7 @@ STATIC CONST ACPI_PARSER SlitParser[] =3D { > /** > Macro to get the value of a System Locality **/ -#define > SLIT_ELEMENT(Ptr, i, j) *(Ptr + (MultU64x64 (i, LocalityCount)) + j) > +#define SLIT_ELEMENT(Ptr, i, j) *(Ptr + (i * LocalityCount) + j) >=20 > /** > This function parses the ACPI SLIT table. > @@ -57,9 +57,9 @@ ParseAcpiSlit ( > ) > { > UINT32 Offset; > - UINT64 Count; > - UINT64 Index; > - UINT64 LocalityCount; > + UINT32 Count; > + UINT32 Index; > + UINT32 LocalityCount; > UINT8* LocalityPtr; > CHAR16 Buffer[80]; // Used for AsciiName param of ParseAcpi >=20 > @@ -87,8 +87,45 @@ ParseAcpiSlit ( > return; > } >=20 > + /* > + Despite the 'Number of System Localities' being a 64-bit field in S= LIT, > + the maximum number of localities that can be represented in SLIT is > limited > + by the 'Length' field of the ACPI table. > + > + Since the ACPI table length field is 32-bit wide. The maximum numbe= r of > + localities that can be represented in SLIT can be calculated as: > + > + MaxLocality =3D sqrt (MAX_UINT32 - sizeof > (EFI_ACPI_6_3_SYSTEM_LOCALITY_DISTANCE_INFORMATION_TABLE_HEAD > ER)) > + =3D 65535 > + =3D MAX_UINT16 > + */ > + if (*SlitSystemLocalityCount > MAX_UINT16) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: The Number of System Localities provided can't be > represented " \ > + L"in the SLIT table. SlitSystemLocalityCount =3D %ld. " \ > + L"MaxLocalityCountAllowed =3D %d.\n", > + *SlitSystemLocalityCount, > + MAX_UINT16 > + ); > + return; > + } > + > + LocalityCount =3D (UINT32)*SlitSystemLocalityCount; > + > + // Make sure system localities fit in the table buffer provided if > + (Offset + (LocalityCount * LocalityCount) > AcpiTableLength) { > + IncrementErrorCount (); > + Print ( > + L"ERROR: Invalid Number of System Localities. " \ > + L"SlitSystemLocalityCount =3D %ld. AcpiTableLength =3D %d.\n", > + *SlitSystemLocalityCount, > + AcpiTableLength > + ); > + return; > + } > + > LocalityPtr =3D Ptr + Offset; > - LocalityCount =3D *SlitSystemLocalityCount; >=20 > // We only print the Localities if the count is less than 16 > // If the locality count is more than 16 then refer to the > -- > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)' >=20 >=20 >=20 >=20