[edk2-devel] [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE

Kirkendall, Garrett posted 2 patches 3 weeks ago
Failed in applying to current master (apply log)
UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf         |  7 ++++
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf             |  2 ++
UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf |  2 ++
UefiCpuPkg/Include/Library/UefiCpuLib.h                      | 14 ++++++++
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38 ++++++++++++++++++++
UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++-----------
UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19 ++++++++--
UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20 +++++++++--
12 files changed, 113 insertions(+), 74 deletions(-)
create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c

[edk2-devel] [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE

Posted by Kirkendall, Garrett 3 weeks ago
AMD processor does not support MSR_IA32_MISC_ENABLE register.  Accessing
this register on AMD causes an unhandled exception in SmmEntry.nasm and
a subsequent failure to boot since this is too early in SMM path for the
exception handler to be loaded.

First, to distinguish between AMD and other processors, refactor
StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only
one copy in the source. All changed modules already include UefiCpuLib
either directly or indirectly so could not easly split first patch.

Second, Skip manipulation of MSR_IA32_MISC_ENABLE register if running
on an AMD processor.

Tested on AMD X64 hardware.
OvmfIa32 and OvmfIa32X64 on Intel hardware.

Garrett Kirkendall (2):
  UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
  UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE manipulation on
    AMD

 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf         |  7 ++++
 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf             |  2 ++
 UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf |  2 ++
 UefiCpuPkg/Include/Library/UefiCpuLib.h                      | 14 ++++++++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38 ++++++++++++++++++++
 UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
 UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++-----------
 UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19 ++++++++--
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20 +++++++++--
 12 files changed, 113 insertions(+), 74 deletions(-)
 create mode 100644 UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c

Changes at:
https://github.com/gkirkendall-amd/edk2/tree/smmentry_nasm_skip_msr_xd_bit_on_amd_v2

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>

-- 
2.27.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61497): https://edk2.groups.io/g/devel/message/61497
Mute This Topic: https://groups.io/mt/74960774/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE

Posted by Dong, Eric 3 weeks ago
Hi Garrett,

I create a pull request to verify your changes and it reports some errors for your changes. 
https://github.com/tianocore/edk2/pull/710

please help to resolve these errors before sending your new version changes, also you can create your PR to verify your new changes.

Thanks,
Eric

> -----Original Message-----
> From: Garrett Kirkendall <Garrett.Kirkendall@amd.com>
> Sent: Thursday, June 18, 2020 11:23 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE
> 
> AMD processor does not support MSR_IA32_MISC_ENABLE register.
> Accessing this register on AMD causes an unhandled exception in
> SmmEntry.nasm and a subsequent failure to boot since this is too early in
> SMM path for the exception handler to be loaded.
> 
> First, to distinguish between AMD and other processors, refactor
> StandardSignatureIsAuthenticAMD into BaseUefiCpuLib.  So there is only one
> copy in the source. All changed modules already include UefiCpuLib either
> directly or indirectly so could not easly split first patch.
> 
> Second, Skip manipulation of MSR_IA32_MISC_ENABLE register if running on
> an AMD processor.
> 
> Tested on AMD X64 hardware.
> OvmfIa32 and OvmfIa32X64 on Intel hardware.
> 
> Garrett Kirkendall (2):
>   UefiCpuPkg: Move StandardSignatureIsAuthenticAMD to BaseUefiCpuLib
>   UefiCpuPkg: PiSmmCpuDxeSmm skip MSR_IA32_MISC_ENABLE
> manipulation on
>     AMD
> 
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf         |  7 ++++
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf             |  2 ++
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.inf |  2 ++
>  UefiCpuPkg/Include/Library/UefiCpuLib.h                      | 14 ++++++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h               |  3 ++
>  UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c           | 38
> ++++++++++++++++++++
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c               | 25 ++-----------
>  UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 25 ++------
> -----
>  UefiCpuPkg/Library/MpInitLib/MpLib.c                         | 23 ------------
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c                       |  9 ++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm                 | 19
> ++++++++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm                  | 20
> +++++++++--
>  12 files changed, 113 insertions(+), 74 deletions(-)  create mode 100644
> UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.c
> 
> Changes at:
> https://github.com/gkirkendall-
> amd/edk2/tree/smmentry_nasm_skip_msr_xd_bit_on_amd_v2
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.com>
> 
> --
> 2.27.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61507): https://edk2.groups.io/g/devel/message/61507
Mute This Topic: https://groups.io/mt/74960774/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 0/2] AMD procesor MSR_IA32_MISC_ENABLE

Posted by Laszlo Ersek 3 weeks ago
Hi Garrett,

On 06/19/20 03:00, Dong, Eric wrote:
> Hi Garrett,
>
> I create a pull request to verify your changes and it reports some
> errors for your changes.  https://github.com/tianocore/edk2/pull/710
>
> please help to resolve these errors before sending your new version
> changes, also you can create your PR to verify your new changes.

It seems like the standalone "PcAtChipsetPkg.dsc" build failed.

The reason seems to be that PcAtChipsetPkg.dsc contains a lib class
resolution like

  LocalApicLib|UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf

but does not contain a UefiCpuLib class resolution.

Speaking more generally, the following DSC files reference at least one
of "BaseXApicLib.inf" and "BaseXApicX2ApicLib.inf":

- edk2:

  IntelFsp2WrapperPkg/IntelFsp2WrapperPkg.dsc
  OvmfPkg/OvmfPkgIa32.dsc
  OvmfPkg/OvmfPkgIa32X64.dsc
  OvmfPkg/OvmfPkgX64.dsc
  OvmfPkg/OvmfXen.dsc
  PcAtChipsetPkg/PcAtChipsetPkg.dsc
  SourceLevelDebugPkg/SourceLevelDebugPkg.dsc
  UefiCpuPkg/UefiCpuPkg.dsc
  UefiPayloadPkg/UefiPayloadPkgIa32.dsc
  UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc

- edk2-platforms:

  Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
  Platform/Intel/QuarkPlatformPkg/Quark.dsc
  Platform/Intel/QuarkPlatformPkg/QuarkMin.dsc
  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
  Platform/Intel/Vlv2TbltDevicePkg/PlatformPkgX64.dsc

Among these files, the following ones do *not* already have the

  UefiCpuLib|UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf

lib class resolution:

- edk2:

  PcAtChipsetPkg/PcAtChipsetPkg.dsc           [found by CI]
  SourceLevelDebugPkg/SourceLevelDebugPkg.dsc

- edk2-platforms:

  [none]

Therefore, this patch set is sound, it is just incomplete. We need two
patches *prepended*: one patch that adds the UefiCpuLib resolution to
"PcAtChipsetPkg/PcAtChipsetPkg.dsc", and another that adds the same
resolution to "SourceLevelDebugPkg/SourceLevelDebugPkg.dsc". Then the
current (v2) patches can be included verbatim as patch v3 #3, and patch
v3 #4.

Garrett: you can also invoke a personal CI run -- just submit a
github.com pull request with your v3 patches, before posting them to the
list. You will not be able to set the "push" label on your PR, so the CI
system will auto-close the PR (without merging it). But, it will tell
you the build result (pass or failure).

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61525): https://edk2.groups.io/g/devel/message/61525
Mute This Topic: https://groups.io/mt/74960774/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-