From: "Laszlo Ersek" <lersek@redhat.com>
To: ray.ni@intel.com
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v4 0/8] Support 5-level paging in DXE long mode
Date: Wed, 7 Aug 2019 22:18:10 +0200 [thread overview]
Message-ID: <43246e53-9f23-9af6-4cc1-c57021fcde02@redhat.com> (raw)
In-Reply-To: <20190801095831.274356-1-ray.ni@intel.com>
Hi Ray,
On 08/01/19 11:58, Ni, Ray wrote:
> v4:
> Move all files under UefiCpuPkg/Include/Register/ to MdePkg.
> NOTE: Changes like updating BaseLib.h to include Cpuid.h is not
> included.
Sorry about the delay. I've wanted to regression-test this series after
Eric approves it. That happened on 2 Aug 2019, but I've been away until
this morning. So I'm coming to the series now.
(1) A reminder: please make sure that you get Reviewed-by or Acked-by
for the MdePkg and MdeModulePkg patches too, from their respective
package maintainers. In particular, patch #5 modifies two packages at
once, so the maintainers of both packages will have to approve it. The
same applies to patch #7.
(2) The series causes a build failure for OVMF:
> OvmfPkg/PlatformPei/FeatureControl.c:14:35: fatal error: Register/Msr/Core2Msr.h: No such file or directory
> #include <Register/Msr/Core2Msr.h>
That's because patch #7 moves
UefiCpuPkg/Include/Register/Msr/Core2Msr.h
under MdePkg, without replacement under UefiCpuPkg.
Can you please *prepend* the following OvmfPkg patch to the series:
> diff --git a/OvmfPkg/PlatformPei/FeatureControl.c b/OvmfPkg/PlatformPei/FeatureControl.c
> index 837da2119adb..1a9d75022f48 100644
> --- a/OvmfPkg/PlatformPei/FeatureControl.c
> +++ b/OvmfPkg/PlatformPei/FeatureControl.c
> @@ -11,7 +11,7 @@
> #include <Library/PeiServicesLib.h>
> #include <Library/QemuFwCfgLib.h>
> #include <Ppi/MpServices.h>
> -#include <Register/Msr/Core2Msr.h>
> +#include <Register/ArchitecturalMsr.h>
>
> #include "Platform.h"
>
> @@ -37,7 +37,7 @@ WriteFeatureControl (
> IN OUT VOID *WorkSpace
> )
> {
> - AsmWriteMsr64 (MSR_CORE2_FEATURE_CONTROL, mFeatureControlValue);
> + AsmWriteMsr64 (MSR_IA32_FEATURE_CONTROL, mFeatureControlValue);
> }
>
> /**
This change is a no-op, behaviorally (both macros expand to 0x0000003A),
but MSR_IA32_FEATURE_CONTROL matches the original intent in OvmfPkg
better -- the code is not Core2 specific --, plus it allows your series
to build, without any changes to the v4 patches #1 through #8.
(3) With said OvmfPkg/PlatformPei update in place, and your v4 patches
applied on top, I couldn't find any regressions. Please post a v5 with
the OvmfPkg/PlatformPei patch at the front, and -- if you change
*nothing* in the v4 patches --, please add:
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>
to the v5 patches #2 through #9.
Thanks!
Laszlo
> v3:
> Move UefiCpuPkg/Include/Register/Cpuid.h to MdePkg/Include/Register/Intel/ directory.
> Create UefiCpuPkg/Include/Register/Cpuid.h to include MdePkg/Include/Register/Intel/Cpuid.h.
> NOTE:
> Changes like moving Amd/Cpuid.h to MdePkg is not included.
> Changes like updating BaseLib.h to include Cpuid.h is not included.
>
> v2:
> Refined the patch according to reviewers' all comments except:
> 0A0h cannot be changed to A0h or build fails.
> A big change in this patch is Cpuid.h is moved from UefiCpuPkg to MdePkg.
> The move is based on real requirement when certain modules that cannot
> depend on UefiCpuPkg but needs to reference structures defined in SDM.
>
> Ray Ni (8):
> UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled
> UefiCpuPkg/CpuDxe: Remove unnecessary macros
> UefiCpuPkg/CpuDxe: Support parsing 5-level page table
> MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable
> MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg
> MdeModulePkg/DxeIpl: Create 5-level page table for long mode
> UefiCpuPkg|MdePkg: Move Register/ folder to MdePkg/Include/
> UefiCpuPkg: Update code to include register definitions from MdePkg
>
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
> .../Core/DxeIplPeim/X64/VirtualMemory.c | 229 +-
> MdeModulePkg/MdeModulePkg.dec | 7 +
> MdeModulePkg/MdeModulePkg.uni | 8 +
> .../Include/Register/Amd/Cpuid.h | 0
> .../Include/Register/Amd/Fam17Msr.h | 0
> .../Include/Register/Amd/Msr.h | 4 +-
> .../Register/Intel}/ArchitecturalMsr.h | 8 +-
> .../Include/Register/Intel}/Cpuid.h | 6 +-
> .../Include/Register/Intel}/LocalApic.h | 6 +-
> .../Include/Register/Intel}/Microcode.h | 6 +-
> MdePkg/Include/Register/Intel/Msr.h | 44 +
> .../Include/Register/Intel}/Msr/AtomMsr.h | 4 +-
> .../Register/Intel}/Msr/BroadwellMsr.h | 4 +-
> .../Include/Register/Intel}/Msr/Core2Msr.h | 4 +-
> .../Include/Register/Intel}/Msr/CoreMsr.h | 4 +-
> .../Include/Register/Intel}/Msr/GoldmontMsr.h | 4 +-
> .../Register/Intel}/Msr/GoldmontPlusMsr.h | 4 +-
> .../Include/Register/Intel}/Msr/HaswellEMsr.h | 4 +-
> .../Include/Register/Intel}/Msr/HaswellMsr.h | 4 +-
> .../Register/Intel}/Msr/IvyBridgeMsr.h | 4 +-
> .../Include/Register/Intel}/Msr/NehalemMsr.h | 4 +-
> .../Include/Register/Intel}/Msr/P6Msr.h | 4 +-
> .../Include/Register/Intel}/Msr/Pentium4Msr.h | 2 +-
> .../Include/Register/Intel}/Msr/PentiumMMsr.h | 2 +-
> .../Include/Register/Intel}/Msr/PentiumMsr.h | 2 +-
> .../Register/Intel}/Msr/SandyBridgeMsr.h | 2 +-
> .../Register/Intel}/Msr/SilvermontMsr.h | 2 +-
> .../Include/Register/Intel}/Msr/SkylakeMsr.h | 2 +-
> .../Include/Register/Intel}/Msr/Xeon5600Msr.h | 2 +-
> .../Include/Register/Intel}/Msr/XeonDMsr.h | 2 +-
> .../Include/Register/Intel}/Msr/XeonE7Msr.h | 2 +-
> .../Include/Register/Intel}/Msr/XeonPhiMsr.h | 2 +-
> .../Register/Intel}/SmramSaveStateMap.h | 6 +-
> .../Include/Register/Intel}/StmApi.h | 12 +-
> .../Register/Intel}/StmResourceDescriptor.h | 6 +-
> .../Include/Register/Intel}/StmStatusCode.h | 6 +-
> UefiCpuPkg/Application/Cpuid/Cpuid.c | 2 +-
> UefiCpuPkg/CpuDxe/CpuDxe.h | 4 +-
> UefiCpuPkg/CpuDxe/CpuPageTable.c | 63 +-
> UefiCpuPkg/CpuDxe/CpuPageTable.h | 3 +-
> UefiCpuPkg/CpuMpPei/CpuPaging.c | 6 +-
> .../Include/Library/RegisterCpuFeaturesLib.h | 2 +-
> .../Include/Library/SmmCpuFeaturesLib.h | 2 +-
> UefiCpuPkg/Include/Protocol/SmMonitorInit.h | 4 +-
> .../Include/Register/ArchitecturalMsr.h | 6565 +----------------
> UefiCpuPkg/Include/Register/Cpuid.h | 3990 +---------
> UefiCpuPkg/Include/Register/LocalApic.h | 175 +-
> UefiCpuPkg/Include/Register/Microcode.h | 187 +-
> UefiCpuPkg/Include/Register/Msr.h | 36 +-
> .../Include/Register/SmramSaveStateMap.h | 179 +-
> UefiCpuPkg/Include/Register/StmApi.h | 941 +--
> .../Library/BaseXApicLib/BaseXApicLib.c | 6 +-
> .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 6 +-
> .../CpuCommonFeaturesLib/CpuCommonFeatures.h | 6 +-
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 12 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 3 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 14 +-
> UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 4 +-
> .../SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 6 +-
> UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c | 6 +-
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 4 +-
> UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 2 -
> 64 files changed, 405 insertions(+), 12249 deletions(-)
> rename {UefiCpuPkg => MdePkg}/Include/Register/Amd/Cpuid.h (100%)
> rename {UefiCpuPkg => MdePkg}/Include/Register/Amd/Fam17Msr.h (100%)
> rename {UefiCpuPkg => MdePkg}/Include/Register/Amd/Msr.h (78%)
> copy {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/ArchitecturalMsr.h (96%)
> copy {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Cpuid.h (96%)
> copy {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/LocalApic.h (95%)
> copy {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Microcode.h (95%)
> create mode 100644 MdePkg/Include/Register/Intel/Msr.h
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/AtomMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/BroadwellMsr.h (95%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/Core2Msr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/CoreMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/GoldmontMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/GoldmontPlusMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/HaswellEMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/HaswellMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/IvyBridgeMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/NehalemMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/P6Msr.h (95%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/Pentium4Msr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/PentiumMMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/PentiumMsr.h (95%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/SandyBridgeMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/SilvermontMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/SkylakeMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/Xeon5600Msr.h (95%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/XeonDMsr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/XeonE7Msr.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/Msr/XeonPhiMsr.h (96%)
> copy {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/SmramSaveStateMap.h (93%)
> copy {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/StmApi.h (96%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/StmResourceDescriptor.h (92%)
> rename {UefiCpuPkg/Include/Register => MdePkg/Include/Register/Intel}/StmStatusCode.h (94%)
>
prev parent reply other threads:[~2019-08-07 20:18 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-01 9:58 [PATCH v4 0/8] Support 5-level paging in DXE long mode Ni, Ray
2019-08-01 9:58 ` [PATCH v4 1/8] UefiCpuPkg/MpInitLib: Enable 5-level paging for AP when BSP's enabled Ni, Ray
2019-08-02 1:08 ` Dong, Eric
2019-08-01 9:58 ` [PATCH v4 2/8] UefiCpuPkg/CpuDxe: Remove unnecessary macros Ni, Ray
2019-08-02 1:13 ` Dong, Eric
2019-08-01 9:58 ` [PATCH v4 3/8] UefiCpuPkg/CpuDxe: Support parsing 5-level page table Ni, Ray
2019-08-02 1:20 ` Dong, Eric
2019-08-01 9:58 ` [PATCH v4 4/8] MdeModulePkg/DxeIpl: Introduce PCD PcdUse5LevelPageTable Ni, Ray
2019-08-02 1:22 ` Dong, Eric
2019-08-08 5:30 ` Wu, Hao A
2019-08-08 5:46 ` [edk2-devel] " Liming Gao
2019-08-01 9:58 ` [PATCH v4 5/8] MdePkg/Cpuid.h: Move Cpuid.h from UefiCpuPkg to MdePkg Ni, Ray
2019-08-02 1:28 ` Dong, Eric
2019-08-08 3:03 ` Liming Gao
2019-08-01 9:58 ` [PATCH v4 6/8] MdeModulePkg/DxeIpl: Create 5-level page table for long mode Ni, Ray
2019-08-02 2:23 ` Dong, Eric
2019-08-08 5:34 ` Wu, Hao A
2019-08-01 9:58 ` [PATCH v4 7/8] UefiCpuPkg|MdePkg: Move Register/ folder to MdePkg/Include/ Ni, Ray
2019-08-02 2:25 ` Dong, Eric
2019-08-08 3:06 ` Liming Gao
2019-08-01 9:58 ` [PATCH v4 8/8] UefiCpuPkg: Update code to include register definitions from MdePkg Ni, Ray
2019-08-02 2:29 ` Dong, Eric
2019-08-07 20:18 ` Laszlo Ersek [this message]
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=43246e53-9f23-9af6-4cc1-c57021fcde02@redhat.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