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%)
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#45036): https://edk2.groups.io/g/devel/message/45036
Mute This Topic: https://groups.io/mt/32677226/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-