From nobody Sat Apr 20 07:55:22 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) client-ip=66.175.222.12; envelope-from=bounce+27952+65006+1787277+3901457@groups.io; helo=web01.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+65006+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1599145913; cv=none; d=zohomail.com; s=zohoarc; b=kJnKkwDcjVC5d1Y+R712gar6X7j17GK+PkPQi6MEG+7tnDjaDpIA9ReKKFwq48ABuayRAuGrA0BjAqPy0aHT3H7HC8yyu73WbPt4P7QzlW8iWQ3jM3ESrF6vU+b2Sh6IOJ5DD6YYknZYxCzRU1d24NAQtEN+vStufCMGUB0zAFI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1599145913; h=Content-Transfer-Encoding:Cc:Date:From:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Sender:Subject:To; bh=Yhai1/mSi5k9fKYqC1HCorO4OnWzBPisIQs5DMyh89A=; b=Le7MbscuZY8gQgiD3D2wqv4uWp78QYZsjuR/JIaxuGB/UanWJbEjs247mpc3iQMSXfe53fRDXN1oaXVWhTpvUS2RDHBfAi38nys3NWyCy8jvXQY9Fexm0ws2U4YsybEMfDnbj7LJt2dZFho53ZQ1nNUDtOWjOt6LQLGXIauRrSc= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.12 as permitted sender) smtp.mailfrom=bounce+27952+65006+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from web01.groups.io (web01.groups.io [66.175.222.12]) by mx.zohomail.com with SMTPS id 1599145913193536.0511691558153; Thu, 3 Sep 2020 08:11:53 -0700 (PDT) Return-Path: X-Received: by 127.0.0.2 with SMTP id ggPWYY1788612xjYxP6xwE6T; Thu, 03 Sep 2020 08:11:52 -0700 X-Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web10.16330.1599145911563847634 for ; Thu, 03 Sep 2020 08:11:51 -0700 IronPort-SDR: bTEn2Nlaf8/ra0WJiizHnTc6laJbDQPunLGWJQydf+sU++gt8ZpIpH3KTWXXRlSuYTHBVPwPsn j7/+gr6BaCrg== X-IronPort-AV: E=McAfee;i="6000,8403,9733"; a="154990545" X-IronPort-AV: E=Sophos;i="5.76,387,1592895600"; d="scan'208";a="154990545" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Sep 2020 08:11:50 -0700 IronPort-SDR: 5wrBJ/eMm/3fQFuxoN7JxWrdN4LHqDSZ0mGdhzOHmxYG2bILZrsrxKc2omHaSBct1zO9PXUhVs UtsyIxPl1/8Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.76,387,1592895600"; d="scan'208";a="503093858" X-Received: from ydong10-desktop.ccr.corp.intel.com ([10.239.154.145]) by fmsmga005.fm.intel.com with ESMTP; 03 Sep 2020 08:11:49 -0700 From: "Dong, Eric" To: devel@edk2.groups.io Cc: Ray Ni , Laszlo Ersek Subject: [edk2-devel] [PATCH v2] UefiCpuPkg/MpInitLib: Add check for CR3/GDT/IDT. Date: Thu, 3 Sep 2020 23:11:47 +0800 Message-Id: <20200903151147.1196-1-eric.dong@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,eric.dong@intel.com X-Gm-Message-State: CRiufah0AUBQgEfagDDrCqslx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1599145912; bh=uH1er7GX0iyLMOk7k8Czl2C3pEv/BF8NSES6cAZ8rLA=; h=Cc:Date:From:Reply-To:Subject:To; b=jRSBCjqrOiduP6uFbid1PypQRGcxYW44GrIpmm3TWPWci9oaH0gTctDHOL2XLzfJdwJ WKnvw+OjJysygcWJXUPFvfTEsPjENbg3QyjSiZ4+wINkx4B7dxzZqnG1FBeDIIxX1ggD9 Cw9XGmeIMlbnDEd1oQWuUrP/5+2FjHuMb+8= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2954 AP needs to run from real mode to 32bit mode to LONG mode. Page table (pointed by CR3) and GDT are necessary to set up to correct value when CPU execution mode is switched to LONG mode. AP uses the same location page table (CR3) and GDT as what BSP uses. But when the page table or GDT is above 4GB, it's impossible for CPU to use because GDTR.base and CR3 are 32bits before switching to LONG mode. This patch adds check for the CR3, GDT.Base and IDT.Base to not above 32 bits restriction. Change-Id: I231180f45d9f542641082c57d001e38e3f6759d5 Signed-off-by: Eric Dong Cc: Ray Ni Cc: Laszlo Ersek --- V2: Change the check point. Just in the different caller to make the logic clear. V1 patch add check just before the use of the code. It make the logic complicated. UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 9 +++ UefiCpuPkg/Library/MpInitLib/MpLib.c | 76 +++++++++++++++++++ UefiCpuPkg/Library/MpInitLib/MpLib.h | 12 +++ UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 1 + UefiCpuPkg/UefiCpuPkg.dec | 4 + 6 files changed, 103 insertions(+) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Lib= rary/MpInitLib/DxeMpInitLib.inf index 1771575c69..20851f251a 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf @@ -76,3 +76,4 @@ gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## = SOMETIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## = CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## = CONSUMES + gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck ## = CONSUMES \ No newline at end of file diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/M= pInitLib/DxeMpLib.c index 2c00d72dde..f598372c4d 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -394,6 +394,15 @@ MpInitChangeApLoopCallback ( { CPU_MP_DATA *CpuMpData; =20 + // + // Check the CR3/GDT/IDT before waking up AP. + // If the check return fail, it will block later + // OS boot, so halt the system here. + // + if (!ValidCR3GdtIdtCheck()) { + CpuDeadLoop (); + } + CpuMpData =3D GetCpuMpData (); CpuMpData->PmCodeSegment =3D GetProtectedModeCS (); CpuMpData->Pm16CodeSegment =3D GetProtectedMode16CS (); diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpIn= itLib/MpLib.c index 07426274f6..69a0372df7 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -1909,6 +1909,54 @@ CheckAllAPs ( return EFI_NOT_READY; } =20 +/** + Check whether CR3/GDT/IDT value valid for AP. + + @retval TRUE Pass the check. + @retval FALSE Fail the check. + +**/ +BOOLEAN +ValidCR3GdtIdtCheck ( + VOID + ) +{ + IA32_DESCRIPTOR Gdtr; + IA32_DESCRIPTOR Idtr; + + if (!PcdGetBool (PcdEnableCpuApCr3GdtIdtCheck)) { + return TRUE; + } + + // + // AP needs to run from real mode to 32bit mode to LONG mode. Page table + // (pointed by CR3) and GDT are necessary to set up to correct value when + // CPU execution mode is switched to LONG mode. IDT also necessary if the + // exception happened. + // AP uses the same location page table (CR3) and GDT/IDT as what BSP us= es. + // But when the page table or GDT is above 4GB, it's impossible for CPU + // to use because GDTR.base and CR3 are 32bits before switching to LONG + // mode. + // Here add check for the CR3, GDT.Base and range, IDT.Base and range are + // not above 32 bits limitation. + // + if (AsmReadCr3 () >=3D BASE_4GB) { + return FALSE; + } + + AsmReadGdtr ((IA32_DESCRIPTOR *) &Gdtr); + if ((Gdtr.Base >=3D BASE_4GB) || (Gdtr.Base + Gdtr.Limit >=3D BASE_4GB))= { + return FALSE; + } + + AsmReadIdtr ((IA32_DESCRIPTOR *) &Idtr); + if ((Idtr.Base >=3D BASE_4GB) || (Idtr.Base + Idtr.Limit >=3D BASE_4GB))= { + return FALSE; + } + + return TRUE; +} + /** MP Initialize Library initialization. =20 @@ -2318,6 +2366,13 @@ SwitchBSPWorker ( return EFI_NOT_READY; } =20 + // + // Check whether CR3/GDT/IDT valid for AP. + // + if (!ValidCR3GdtIdtCheck()) { + return EFI_INVALID_PARAMETER; + } + CpuMpData->BSPInfo.State =3D CPU_SWITCH_STATE_IDLE; CpuMpData->APInfo.State =3D CPU_SWITCH_STATE_IDLE; CpuMpData->SwitchBspFlag =3D TRUE; @@ -2420,6 +2475,13 @@ EnableDisableApWorker ( return EFI_NOT_FOUND; } =20 + // + // Check whether CR3/GDT/IDT valid for AP. + // + if (!ValidCR3GdtIdtCheck()) { + return EFI_INVALID_PARAMETER; + } + if (!EnableAP) { SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateDisabled); } else { @@ -2607,6 +2669,13 @@ StartupAllCPUsWorker ( return EFI_DEVICE_ERROR; } =20 + // + // Check whether CR3/GDT/IDT valid for AP. + // + if (!ValidCR3GdtIdtCheck()) { + return EFI_INVALID_PARAMETER; + } + // // Update AP state // @@ -2772,6 +2841,13 @@ StartupThisAPWorker ( return EFI_INVALID_PARAMETER; } =20 + // + // Check whether CR3/GDT/IDT valid for AP. + // + if (!ValidCR3GdtIdtCheck()) { + return EFI_INVALID_PARAMETER; + } + // // Update AP state // diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpIn= itLib/MpLib.h index 02652eaae1..4ac5cb51e3 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h @@ -740,5 +740,17 @@ PlatformShadowMicrocode ( IN OUT CPU_MP_DATA *CpuMpData ); =20 +/** + Check whether CR3/GDT/IDT value valid for AP. + + @retval TRUE Pass the check. + @retval FALSE Fail the check. + +**/ +BOOLEAN +ValidCR3GdtIdtCheck ( + VOID + ); + #endif =20 diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Lib= rary/MpInitLib/PeiMpInitLib.inf index 34abf25d43..0ca86fcdaa 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf @@ -65,6 +65,7 @@ gUefiCpuPkgTokenSpaceGuid.PcdSevEsIsEnabled ## CONS= UMES gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase ## SOME= TIMES_CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdGhcbBase ## CONS= UMES + gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck ## CONS= UMES =20 [Ppis] gEdkiiPeiShadowMicrocodePpiGuid ## SOMETIMES_CONSUMES diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index d83c084467..467ec5e001 100644 --- a/UefiCpuPkg/UefiCpuPkg.dec +++ b/UefiCpuPkg/UefiCpuPkg.dec @@ -187,6 +187,10 @@ # @Prompt Configure stack size for Application Processor (AP) gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize|0x8000|UINT32|0x00000003 =20 + ## This value specifies whether need to check the CR3/GDT/IDT value for = AP. + # @Prompt Whether need to check the CR3/GDT/IDT value for AP + gUefiCpuPkgTokenSpaceGuid.PcdEnableCpuApCr3GdtIdtCheck|FALSE|BOOLEAN|0x3= 0000044 + ## Specifies stack size in the temporary RAM. 0 means half of TemporaryR= amSize. # @Prompt Stack size in the temporary RAM. gUefiCpuPkgTokenSpaceGuid.PcdPeiTemporaryRamStackSize|0|UINT32|0x10001003 --=20 2.23.0.windows.1 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#65006): https://edk2.groups.io/g/devel/message/65006 Mute This Topic: https://groups.io/mt/76609049/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-