public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Dong, Eric" <eric.dong@intel.com>
To: "Ni, Ray" <ray.ni@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Laszlo Ersek <lersek@redhat.com>
Subject: Re: [PATCH 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove XD enable/disable logic
Date: Wed, 13 Nov 2019 06:50:59 +0000	[thread overview]
Message-ID: <ED077930C258884BBCB450DB737E662259F3EB1C@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <20191111053515.261224-2-ray.ni@intel.com>

Reviewed-by: Eric Dong <eric.dong@intel.com>

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Monday, November 11, 2019 11:05 AM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: [PATCH 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove XD
> enable/disable logic
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2329
> 
> 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.
> 
> 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.
> 
> 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.
> 
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> ---
>  .../CpuCommonFeaturesLib.c                    | 11 ---
>  .../CpuCommonFeaturesLib.inf                  |  3 +-
>  .../CpuCommonFeaturesLib/ExecuteDisable.c     | 95 -------------------
>  3 files changed, 1 insertion(+), 108 deletions(-)  delete mode 100644
> UefiCpuPkg/Library/CpuCommonFeaturesLib/ExecuteDisable.c
> 
> 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 = RegisterCpuFeature (
> -               "Execute Disable",
> -               NULL,
> -               ExecuteDisableSupport,
> -               ExecuteDisableInitialize,
> -               CPU_FEATURE_XD,
> -               CPU_FEATURE_END
> -               );
> -    ASSERT_EFI_ERROR (Status);
> -  }
>    if (IsCpuFeatureSupported (CPU_FEATURE_FASTSTRINGS)) {
>      Status = 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.<BR>
> +# Copyright (c) 2017 - 2019, Intel Corporation. All rights
> +reserved.<BR>
>  #
>  #  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/UefiCpuPkg/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.<BR>
> -  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
> function.
> -  @param[in]  CpuInfo          A pointer to the
> REGISTER_CPU_FEATURE_INFORMATION
> -                               structure for the CPU executing this
> function.
> -  @param[in]  ConfigData       A pointer to the configuration buffer
> returned
> -                               by
> CPU_FEATURE_GET_CONFIG_DATA.  NULL if
> -                               CPU_FEATURE_GET_CONFIG_DATA
> was not provided 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 <= 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 != 0);
> -}
> -
> -/**
> -  Initializes Execute Disable feature to specific state.
> -
> -  @param[in]  ProcessorNumber  The index of the CPU executing this
> function.
> -  @param[in]  CpuInfo          A pointer to the
> REGISTER_CPU_FEATURE_INFORMATION
> -                               structure for the CPU executing this
> function.
> -  @param[in]  ConfigData       A pointer to the configuration buffer
> returned
> -                               by
> CPU_FEATURE_GET_CONFIG_DATA.  NULL if
> -                               CPU_FEATURE_GET_CONFIG_DATA
> was not provided in
> -                               RegisterCpuFeature().
> -  @param[in]  State            If TRUE, then the Execute Disable
> feature must be enabled.
> -                               If FALSE, then the Execute Disable
> feature 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,
> only program
> -  // MSR_IA32_EFER for thread 0 in each core.
> -  //
> -  if (IS_SILVERMONT_PROCESSOR (CpuInfo->DisplayFamily,
> CpuInfo->DisplayModel)) {
> -    if (CpuInfo->ProcessorInfo.Location.Thread != 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;
> -}
> --
> 2.21.0.windows.1


  parent reply	other threads:[~2019-11-13  6:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-11  5:35 [PATCH 0/2] UefiCpuPkg/CpuFeature: Remove XD enable/disable logic Ni, Ray
2019-11-11  5:35 ` [PATCH 1/2] UefiCpuPkg/CpuCommonFeaturesLib: " Ni, Ray
2019-11-11 10:23   ` Laszlo Ersek
2019-11-13  6:50   ` Dong, Eric [this message]
2019-11-11  5:35 ` [PATCH 2/2] UefiCpuPkg/RegisterCpuFeature: Remove CPU_FEATURE_XD macro Ni, Ray
2019-11-13  6:50   ` Dong, Eric

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ED077930C258884BBCB450DB737E662259F3EB1C@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox