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.115, mailfrom: jaben.carsey@intel.com) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by groups.io with SMTP; Thu, 01 Aug 2019 13:46:08 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Aug 2019 13:46:08 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,335,1559545200"; d="scan'208";a="256768778" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga001.jf.intel.com with ESMTP; 01 Aug 2019 13:46:07 -0700 Received: from fmsmsx151.amr.corp.intel.com (10.18.125.4) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 1 Aug 2019 13:46:07 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.42]) by FMSMSX151.amr.corp.intel.com ([169.254.7.106]) with mapi id 14.03.0439.000; Thu, 1 Aug 2019 13:46:06 -0700 From: "Carsey, Jaben" To: "Kinney, Michael D" , Sami Mujawar , "devel@edk2.groups.io" CC: "Ni, Ray" , "Gao, Zhichao" Subject: Re: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error Thread-Topic: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error Thread-Index: AQHVN2/B5A2b81/7VEGSYI7o+3hpVqbFipMAgCHLwoD//43RQA== Date: Thu, 1 Aug 2019 20:46:06 +0000 Message-ID: References: <20190710223507.14396-1-michael.d.kinney@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTI5ZmM1OGMtZWU4My00NWNmLTljOTMtNDVlYjY1MTc4ODIxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiUURMUnRRdWVuM0tLQ1BwbkpVZkMrWklDUVY4T3FkSjl0dklQZEJ1WWJqQ3pZMTZ4QlNvcjFwZTFHemlmdlpCTiJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.1.200.106] MIME-Version: 1.0 Return-Path: jaben.carsey@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I think the shell DSC be updated to catch build errors in the NULL libs.=20 I am good with Mike's proposed solution below, but not very concerned with= the method to catch the error. Thanks -Jaben > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, August 01, 2019 1:30 PM > To: Sami Mujawar ; devel@edk2.groups.io; > Kinney, Michael D > Cc: Carsey, Jaben ; Ni, Ray ; > Gao, Zhichao > Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix IA32 link error >=20 > Hi Sami, >=20 > I agree with your feedback. I saw that there was a larger > patch set for the ShellPkg, so I let that complete before > returning to this topic. >=20 > The reason that I noticed this issue in the first place is > when I added the acpiview command to a platform and there > was an IA32 build failure. It would be better if the ShellPkg > build caught this issue. Adding the acpiview command to > the standard shell build adds 50K to an uncompressed DEBUG > IA32 build. >=20 > 869,312 Shell_7C04A583-9E3E-4f1c-AD65-E05268D0B4D1.efi > 920,928 Shell_EA4BB293-2D7F-4456-A681-1F22F42CD0BC.efi >=20 > Should acpiview be added to the standard ShellPkg build, > or should I add an extra build of the Shell to the > ShellPkg DSC file with a different GUID to make sure > the shell builds when all NULL libs are included without > any !if statements. For example: >=20 > ShellPkg/Application/Shell/Shell.inf { > > FILE_GUID =3D EA4BB293-2D7F-4456-A681-1F22F42CD0BC > > gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE > >=20 > NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comma > ndsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Comma > ndsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Comma > ndsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Com > mandsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1Com > mandsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Com > mandsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1 > CommandsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2 > CommandsLib.inf >=20 > NULL|ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCo > mmandLib.inf > } >=20 > Thanks, >=20 > Mike >=20 > > -----Original Message----- > > From: Sami Mujawar [mailto:Sami.Mujawar@arm.com] > > Sent: Thursday, July 11, 2019 1:24 AM > > To: devel@edk2.groups.io; Kinney, Michael D > > > > Cc: Carsey, Jaben ; Ni, Ray > > ; Gao, Zhichao > > > > Subject: RE: [edk2-devel] [Patch] ShellPkg/AcpiView: > > Fix IA32 link error > > > > Hi Mike, > > > > Since LocalityCount is 64-bit wide the SLIT validation > > code could possibly end up in an infinite loop. I am > > not aware of a platform that has a large enough > > LocalityCount to hit this condition. However, would it > > be good to have a check that limits the validation to > > MAX_UINT32? > > > > e.g. Something like > > if (LocalityCount < MAX_UINT32) { > > // Validate > > for (Count =3D 0; Count < LocalityCount; Count++) { > > for (Index =3D 0; Index < LocalityCount; Index++) { > > ... > > } else { > > Print (L"INFO: Skipping validation of System > > Localities as locality count is > MAX_UINT32\n"); } > > > > Regards, > > > > Sami Mujawar > > > > -----Original Message----- > > From: devel@edk2.groups.io On > > Behalf Of Michael D Kinney via Groups.Io > > Sent: 10 July 2019 11:35 PM > > To: devel@edk2.groups.io > > Cc: Jaben Carsey ; Ray Ni > > ; Zhichao Gao > > Subject: [edk2-devel] [Patch] ShellPkg/AcpiView: Fix > > IA32 link error > > > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D1970 > > > > Update local variable in ParseAcpiSlot() to be UINT32 > > instead of UINT64 to avoid 64-bit multiply operation in > > the SLIT_ELEMENT() macro. > > > > Cc: Jaben Carsey > > Cc: Ray Ni > > Cc: Zhichao Gao > > Signed-off-by: Michael D Kinney > > > > --- > > > > .../UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser > > .c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git > > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/ > > Slit/SlitParser.c > > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/ > > Slit/SlitParser.c > > index 1f9dac66ee..af85c9aa1c 100644 > > --- > > a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/ > > Slit/SlitParser.c > > +++ > > b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/ > > Slit/SlitPars > > +++ er.c > > @@ -57,8 +57,8 @@ ParseAcpiSlit ( > > ) > > { > > UINT32 Offset; > > - UINT64 Count; > > - UINT64 Index; > > + UINT32 Count; > > + UINT32 Index; > > UINT64 LocalityCount; > > UINT8* LocalityPtr; > > CHAR16 Buffer[80]; // Used for AsciiName param of > > ParseAcpi > > -- > > 2.21.0.windows.1 > > > > > >=20 > > > > IMPORTANT NOTICE: The contents of this email and any > > attachments are confidential and may also be > > privileged. If you are not the intended recipient, > > please notify the sender immediately and do not > > disclose the contents to any other person, use it for > > any purpose, or store or copy the information in any > > medium. Thank you.