From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.31; helo=mga06.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel@lists.01.org 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 2A35521E87992 for ; Fri, 29 Sep 2017 17:05:54 -0700 (PDT) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga104.jf.intel.com with ESMTP; 29 Sep 2017 17:09:10 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,454,1500966000"; d="scan'208";a="157180122" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga005.fm.intel.com with ESMTP; 29 Sep 2017 17:09:10 -0700 Received: from fmsmsx121.amr.corp.intel.com (10.18.125.36) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 29 Sep 2017 17:09:10 -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; Fri, 29 Sep 2017 17:09:10 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Sat, 30 Sep 2017 08:09:08 +0800 From: "Wang, Jian J" To: "Kinney, Michael D" , "edk2-devel@lists.01.org" CC: "Yao, Jiewen" Thread-Topic: [PATCH] UefiCpuPkg/CpuDxe: Fix assert issue on IA32 platform Thread-Index: AQHTOOV09kB2x4JKeUiKzed1W8LwKaLMC4QwgACB/kA= Date: Sat, 30 Sep 2017 00:09:07 +0000 Message-ID: References: <20170929054014.6136-1-jian.j.wang@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: Fix assert issue on IA32 platform 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: Sat, 30 Sep 2017 00:05:55 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Mike, Thanks for the feedback. I'll update the patch as what you suggested. And i= f no more other comments, I'll check in the updated one today (or tonight). Jian > -----Original Message----- > From: Kinney, Michael D > Sent: Saturday, September 30, 2017 12:27 AM > To: Wang, Jian J ; edk2-devel@lists.01.org; Kinney= , > Michael D > Cc: Yao, Jiewen > Subject: RE: [PATCH] UefiCpuPkg/CpuDxe: Fix assert issue on IA32 platform >=20 > Jian, >=20 > The names of the functions do not look right to me. >=20 > I think the checks being made check to see if paging > is enabled. Also, the check being made is to see if both > paging is enabled and page address extensions is enabled. >=20 > If you use the IA32_CR0 and IA32_CR4 structures from > BaseLib.h, the updates will also be easier to understand > than using BITxx macros. >=20 > Maybe use the following: >=20 >=20 > BOOLEAN > IsPagingAndPageAddressExtensionsEnabled ( > VOID > ) > { > IA32_CR0 Cr0; > IA32_CR4 Cr4; >=20 > Cr0.UintN =3D AsmReadCr0 (); > Cr4.UintN =3D AsmReadCr4 (); > return ((Cr0.Bits.PG !=3D 0) && (Cr4.Bits.PAE !=3D 0)); > } >=20 > Best regards, >=20 > Mike >=20 > > -----Original Message----- > > From: Wang, Jian J > > Sent: Thursday, September 28, 2017 10:40 PM > > To: edk2-devel@lists.01.org > > Cc: Yao, Jiewen ; Kinney, Michael D > > > > Subject: [PATCH] UefiCpuPkg/CpuDxe: Fix assert issue on IA32 > > platform > > > > This patch is to fix an assert issue during booting IA32 > > platforms > > such as OvmfIa32 or Quark. This issue is caused by trying to > > access > > page table on a platform without page table. A check is added > > to > > avoid the assert. > > > > Bug tracker: https://bugzilla.tianocore.org/show_bug.cgi?id=3D724 > > > > c: Star Zeng > > Cc: Jiewen Yao > > Cc: Michael Kinney > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > --- > > UefiCpuPkg/CpuDxe/CpuDxe.c | 48 > > ++++++++++++++++++++++++++++++++++++---------- > > 1 file changed, 38 insertions(+), 10 deletions(-) > > > > diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c > > b/UefiCpuPkg/CpuDxe/CpuDxe.c > > index 4e8fa100e0..85a520079f 100644 > > --- a/UefiCpuPkg/CpuDxe/CpuDxe.c > > +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c > > @@ -683,7 +683,7 @@ SetGcdMemorySpaceAttributes ( > > > > **/ > > VOID > > -RefreshGcdMemoryAttributes ( > > +RefreshMemoryAttributesFromMtrr ( > > VOID > > ) > > { > > @@ -704,14 +704,9 @@ RefreshGcdMemoryAttributes ( > > UINT32 > > FirmwareVariableMtrrCount; > > UINT8 DefaultMemoryType; > > > > - if (!IsMtrrSupported ()) { > > - return; > > - } > > - > > FirmwareVariableMtrrCount =3D GetFirmwareVariableMtrrCount (); > > ASSERT (FirmwareVariableMtrrCount <=3D > > MTRR_NUMBER_OF_VARIABLE_MTRR); > > > > - mIsFlushingGCD =3D TRUE; > > MemorySpaceMap =3D NULL; > > > > // > > @@ -862,11 +857,44 @@ RefreshGcdMemoryAttributes ( > > if (MemorySpaceMap !=3D NULL) { > > FreePool (MemorySpaceMap); > > } > > +} > > > > - // > > - // Update page attributes > > - // > > - RefreshGcdMemoryAttributesFromPaging(); > > +/** > > + Check if paging is enabled or not. > > +**/ > > +BOOLEAN > > +IsPagingSupported ( > > + VOID > > + ) > > +{ > > + return ( > > + (AsmReadCr0 () & BIT31) !=3D 0 > > + && > > + (AsmReadCr4 () & BIT5) !=3D 0 > > + ); > > +} > > + > > +/** > > + Refreshes the GCD Memory Space attributes according to MTRRs > > and Paging. > > + > > + This function refreshes the GCD Memory Space attributes > > according to MTRRs > > + and page tables. > > + > > +**/ > > +VOID > > +RefreshGcdMemoryAttributes ( > > + VOID > > + ) > > +{ > > + mIsFlushingGCD =3D TRUE; > > + > > + if (IsMtrrSupported ()) { > > + RefreshMemoryAttributesFromMtrr (); > > + } > > + > > + if (IsPagingSupported ()) { > > + RefreshGcdMemoryAttributesFromPaging (); > > + } > > > > mIsFlushingGCD =3D FALSE; > > } > > -- > > 2.14.1.windows.1