From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=michael.d.kinney@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 6441521F303EE for ; Fri, 29 Sep 2017 10:04:08 -0700 (PDT) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga105.fm.intel.com with ESMTP; 29 Sep 2017 10:07:23 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,453,1500966000"; d="scan'208";a="154875475" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by orsmga005.jf.intel.com with ESMTP; 29 Sep 2017 10:07:23 -0700 Received: from orsmsx161.amr.corp.intel.com (10.22.240.84) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 29 Sep 2017 09:30:39 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.135]) by ORSMSX161.amr.corp.intel.com ([169.254.4.13]) with mapi id 14.03.0319.002; Fri, 29 Sep 2017 09:27:13 -0700 From: "Kinney, Michael D" To: "Wang, Jian J" , "edk2-devel@lists.01.org" , "Kinney, Michael D" CC: "Yao, Jiewen" Thread-Topic: [PATCH] UefiCpuPkg/CpuDxe: Fix assert issue on IA32 platform Thread-Index: AQHTOOV09kB2x4JKeUiKzed1W8LwKaLMC4Qw Date: Fri, 29 Sep 2017 16:27:11 +0000 Message-ID: References: <20170929054014.6136-1-jian.j.wang@intel.com> In-Reply-To: <20170929054014.6136-1-jian.j.wang@intel.com> 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.22.254.140] 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: Fri, 29 Sep 2017 17:04:08 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Jian, The names of the functions do not look right to me. 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. If you use the IA32_CR0 and IA32_CR4 structures from=20 BaseLib.h, the updates will also be easier to understand=20 than using BITxx macros. Maybe use the following: BOOLEAN IsPagingAndPageAddressExtensionsEnabled ( VOID ) { IA32_CR0 Cr0; IA32_CR4 Cr4; Cr0.UintN =3D AsmReadCr0 (); Cr4.UintN =3D AsmReadCr4 (); return ((Cr0.Bits.PG !=3D 0) && (Cr4.Bits.PAE !=3D 0)); } Best regards, Mike > -----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 >=20 > 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. >=20 > Bug tracker: https://bugzilla.tianocore.org/show_bug.cgi?id=3D724 >=20 > 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(-) >=20 > 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 ( >=20 > **/ > VOID > -RefreshGcdMemoryAttributes ( > +RefreshMemoryAttributesFromMtrr ( > VOID > ) > { > @@ -704,14 +704,9 @@ RefreshGcdMemoryAttributes ( > UINT32 > FirmwareVariableMtrrCount; > UINT8 DefaultMemoryType; >=20 > - if (!IsMtrrSupported ()) { > - return; > - } > - > FirmwareVariableMtrrCount =3D GetFirmwareVariableMtrrCount (); > ASSERT (FirmwareVariableMtrrCount <=3D > MTRR_NUMBER_OF_VARIABLE_MTRR); >=20 > - mIsFlushingGCD =3D TRUE; > MemorySpaceMap =3D NULL; >=20 > // > @@ -862,11 +857,44 @@ RefreshGcdMemoryAttributes ( > if (MemorySpaceMap !=3D NULL) { > FreePool (MemorySpaceMap); > } > +} >=20 > - // > - // 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 (); > + } >=20 > mIsFlushingGCD =3D FALSE; > } > -- > 2.14.1.windows.1