From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web12.2616.1573467822792026973 for ; Mon, 11 Nov 2019 02:23:43 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=hzTlGwy4; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573467821; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nZiIppyZYpsyUACSPx+4Q4ou8DskJbn67wL/4m4zg3M=; b=hzTlGwy4AjpLvqj2/Ky0DNbiT+UNk6EfjS3pG0cy44/DAQxR4Uj+nejQOn/SbMzjere3TR NOrdEgtaTbGyCcXGsPt8eJQ3Mcm0M2rFrFXdAI8Rh4dXNFOQakJJ9ahQK02Gpo9NaAm6Yi MPNmmXUF0S3AwrpXlrAM5lur3FqAlg0= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-307-HImiwHRLO_Sr7cHMWotvdw-1; Mon, 11 Nov 2019 05:23:38 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C89721034B5D; Mon, 11 Nov 2019 10:23:37 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-33.ams2.redhat.com [10.36.116.33]) by smtp.corp.redhat.com (Postfix) with ESMTP id C448C5EE0F; Mon, 11 Nov 2019 10:23:36 +0000 (UTC) Subject: Re: [PATCH 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove XD enable/disable logic To: Ray Ni , devel@edk2.groups.io Cc: Eric Dong References: <20191111053515.261224-1-ray.ni@intel.com> <20191111053515.261224-2-ray.ni@intel.com> From: "Laszlo Ersek" Message-ID: <2128bd0c-368f-4ee3-6546-ed37909437a9@redhat.com> Date: Mon, 11 Nov 2019 11:23:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20191111053515.261224-2-ray.ni@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-MC-Unique: HImiwHRLO_Sr7cHMWotvdw-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 11/11/19 06:35, Ray Ni wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2329 >=20 > XD (ExecutionDisable) feature, when turned on, allows page table > entry BIT63 set to 1 indicating the memory pointed by the page table > is disallowed to execute. > DxeIpl::CreateIdentityMappingPageTables() enables the XD when CPU > supports it. > Later DxeCore modifies the page table to set the BIT63 to protect > the stack/heap to disallow code execution in stack/heap. >=20 > UefiCpuPkg/CpuCommonFeaturesLib enables/disables the XD feature > according to PcdCpuFeaturesSetting. > When XD is disabled, GP fault is generated immediately because some > page entries have BIT63 set. >=20 > To fix this issue, this patch removes the XD feature logic from > UefiCpuPkg/CpuCommonFeaturesLib so the XD feature is only taken > care of by DxeIpl. >=20 > Signed-off-by: Ray Ni > Cc: Eric Dong > Cc: Laszlo Ersek > --- > .../CpuCommonFeaturesLib.c | 11 --- > .../CpuCommonFeaturesLib.inf | 3 +- > .../CpuCommonFeaturesLib/ExecuteDisable.c | 95 ------------------- > 3 files changed, 1 insertion(+), 108 deletions(-) > delete mode 100644 UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisabl= e.c >=20 > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib= .c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c > index 238632f88a..3ebd9392a9 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.c > @@ -66,17 +66,6 @@ CpuCommonFeaturesLibConstructor ( > ); > ASSERT_EFI_ERROR (Status); > } > - if (IsCpuFeatureSupported (CPU_FEATURE_XD)) { > - Status =3D RegisterCpuFeature ( > - "Execute Disable", > - NULL, > - ExecuteDisableSupport, > - ExecuteDisableInitialize, > - CPU_FEATURE_XD, > - CPU_FEATURE_END > - ); > - ASSERT_EFI_ERROR (Status); > - } > if (IsCpuFeatureSupported (CPU_FEATURE_FASTSTRINGS)) { > Status =3D RegisterCpuFeature ( > "FastStrings", > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib= .inf b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf > index 6347c8997d..7fbcd8da0e 100644 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf > +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeaturesLib.inf > @@ -4,7 +4,7 @@ > # This library registers CPU features defined in Intel(R) 64 and IA-32 > # Architectures Software Developer's Manual. > # > -# Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.
> # > # SPDX-License-Identifier: BSD-2-Clause-Patent > # > @@ -34,7 +34,6 @@ [Sources] > C1e.c > ClockModulation.c > Eist.c > - ExecuteDisable.c > FastStrings.c > FeatureControl.c > LimitCpuIdMaxval.c > diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c b/U= efiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c > deleted file mode 100644 > index 75ea16309d..0000000000 > --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c > +++ /dev/null > @@ -1,95 +0,0 @@ > -/** @file > - Execute Disable feature. > - > - Copyright (c) 2017, Intel Corporation. All rights reserved.
> - SPDX-License-Identifier: BSD-2-Clause-Patent > - > -**/ > - > -#include "CpuCommonFeatures.h" > - > -/** > - Detects if Execute Disable feature supported on current processor. > - > - @param[in] ProcessorNumber The index of the CPU executing this funct= ion. > - @param[in] CpuInfo A pointer to the REGISTER_CPU_FEATURE_INF= ORMATION > - structure for the CPU executing this func= tion. > - @param[in] ConfigData A pointer to the configuration buffer ret= urned > - by CPU_FEATURE_GET_CONFIG_DATA. NULL if > - CPU_FEATURE_GET_CONFIG_DATA was not provi= ded in > - RegisterCpuFeature(). > - > - @retval TRUE Execute Disable feature is supported. > - @retval FALSE Execute Disable feature is not supported. > - > - @note This service could be called by BSP/APs. > -**/ > -BOOLEAN > -EFIAPI > -ExecuteDisableSupport ( > - IN UINTN ProcessorNumber, > - IN REGISTER_CPU_FEATURE_INFORMATION *CpuInfo, > - IN VOID *ConfigData OPTIONAL > - ) > -{ > - UINT32 Eax; > - CPUID_EXTENDED_CPU_SIG_EDX Edx; > - > - AsmCpuid (CPUID_EXTENDED_FUNCTION, &Eax, NULL, NULL, NULL); > - if (Eax <=3D CPUID_EXTENDED_FUNCTION) { > - // > - // Extended CPUID functions are not supported on this processor. > - // > - return FALSE; > - } > - > - AsmCpuid (CPUID_EXTENDED_CPU_SIG, NULL, NULL, NULL, &Edx.Uint32); > - return (Edx.Bits.NX !=3D 0); > -} > - > -/** > - Initializes Execute Disable feature to specific state. > - > - @param[in] ProcessorNumber The index of the CPU executing this funct= ion. > - @param[in] CpuInfo A pointer to the REGISTER_CPU_FEATURE_INF= ORMATION > - structure for the CPU executing this func= tion. > - @param[in] ConfigData A pointer to the configuration buffer ret= urned > - by CPU_FEATURE_GET_CONFIG_DATA. NULL if > - CPU_FEATURE_GET_CONFIG_DATA was not provi= ded in > - RegisterCpuFeature(). > - @param[in] State If TRUE, then the Execute Disable feature= must be enabled. > - If FALSE, then the Execute Disable featur= e must be disabled. > - > - @retval RETURN_SUCCESS Execute Disable feature is initialized. > - > - @note This service could be called by BSP only. > -**/ > -RETURN_STATUS > -EFIAPI > -ExecuteDisableInitialize ( > - IN UINTN ProcessorNumber, > - IN REGISTER_CPU_FEATURE_INFORMATION *CpuInfo, > - IN VOID *ConfigData, OPTIONAL > - IN BOOLEAN State > - ) > -{ > - // > - // The scope of the MSR_IA32_EFER is core for below processor type, on= ly program > - // MSR_IA32_EFER for thread 0 in each core. > - // > - if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily, CpuInfo->DisplayM= odel)) { > - if (CpuInfo->ProcessorInfo.Location.Thread !=3D 0) { > - return RETURN_SUCCESS; > - } > - } > - > - CPU_REGISTER_TABLE_WRITE_FIELD ( > - ProcessorNumber, > - Msr, > - MSR_IA32_EFER, > - MSR_IA32_EFER_REGISTER, > - Bits.NXE, > - (State) ? 1 : 0 > - ); > - return RETURN_SUCCESS; > -} >=20 series Acked-by: Laszlo Ersek