Jeff, The copyright dates need to be updated to 2016 in the following 2 files:
SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c With those updates, Reviewed-by: Michael Kinney <[email protected]> Mike > -----Original Message----- > From: Fan, Jeff > Sent: Tuesday, August 2, 2016 1:59 AM > To: [email protected] > Cc: Kinney, Michael D <[email protected]>; Tian, Feng > <[email protected]>; > Mudusuru, Giri P <[email protected]>; Laszlo Ersek <[email protected]> > Subject: [Patch v5 01/48] UefiCpuPkg/LocalApic.h: Remove duplicated/conflicted > definitions > > #define MSR_IA32_APIC_BASE_ADDRESS is duplicated with #define > MSR_IA32_APIC_BASE > defined in UefiCpuPkg/Include/Register/ArchitecturalMsr.h, so we could remove > it > and update the modules to use MSR_IA32_APIC_BASE from ArchitecturalMsr.h. > > Structure MSR_IA32_APIC_BASE conflicts with #define MSR_IA32_APIC_BASE defined > in UefiCpuPkg/Include/Register/ArchitecturalMsr.h, so we could remove it and > update the modules to use structure MSR_IA32_APIC_BASE_REGISTER from > ArchitecturalMsr.h. > > v5: > 1. Update SourceLevelDebugPkg to use APIC Base MSR from ArchitecturalMsr.h. > > Cc: Michael Kinney <[email protected]> > Cc: Feng Tian <[email protected]> > Cc: Giri P Mudusuru <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Jeff Fan <[email protected]> > Reviewed-by: Giri P Mudusuru <[email protected]> > Reviewed-by: Laszlo Ersek <[email protected]> > --- > .../DebugAgent/DebugAgentCommon/DebugAgent.h | 1 + > .../Library/DebugAgent/DebugAgentCommon/DebugMp.c | 5 ++- > UefiCpuPkg/CpuMpPei/CpuMpPei.h | 1 + > UefiCpuPkg/CpuMpPei/PeiMpServices.c | 20 ++++----- > UefiCpuPkg/Include/Register/LocalApic.h | 20 +-------- > UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 29 ++++++------ > .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 51 > +++++++++++----------- > 7 files changed, 58 insertions(+), 69 deletions(-) > > diff --git > a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h > b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h > index 64e4c3e..18b93a3 100644 > --- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h > +++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.h > @@ -34,6 +34,7 @@ > #include <Library/PrintLib.h> > #include <Library/PeCoffGetEntryPointLib.h> > #include <Library/PeCoffExtraActionLib.h> > +#include <Register/ArchitecturalMsr.h> > > #include <TransferProtocol.h> > #include <ImageDebugSupport.h> > diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c > b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c > index bdb6742..db9eb6a 100644 > --- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c > +++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugMp.c > @@ -141,6 +141,8 @@ IsBsp ( > IN UINT32 ProcessorIndex > ) > { > + MSR_IA32_APIC_BASE_REGISTER MsrApicBase; > + > // > // If there are less than 2 CPUs detected, then the currently executing CPU > // must be the BSP. This avoids an access to an MSR that may not be > supported > @@ -150,7 +152,8 @@ IsBsp ( > return TRUE; > } > > - if (AsmMsrBitFieldRead64 (MSR_IA32_APIC_BASE_ADDRESS, 8, 8) == 1) { > + MsrApicBase.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > + if (MsrApicBase.Bits.BSP == 1) { > if (mDebugMpContext.BspIndex != ProcessorIndex) { > AcquireMpSpinLock (&mDebugMpContext.MpContextSpinLock); > mDebugMpContext.BspIndex = ProcessorIndex; > diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > index b2e578b..0d1a14a 100644 > --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h > +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h > @@ -25,6 +25,7 @@ > > #include <Register/Cpuid.h> > #include <Register/LocalApic.h> > +#include <Register/Msr.h> > > #include <Library/BaseLib.h> > #include <Library/BaseMemoryLib.h> > diff --git a/UefiCpuPkg/CpuMpPei/PeiMpServices.c > b/UefiCpuPkg/CpuMpPei/PeiMpServices.c > index e784377..e06fdf1 100644 > --- a/UefiCpuPkg/CpuMpPei/PeiMpServices.c > +++ b/UefiCpuPkg/CpuMpPei/PeiMpServices.c > @@ -1,7 +1,7 @@ > /** @file > Implementation of Multiple Processor PPI services. > > - Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -729,9 +729,9 @@ PeiSwitchBSP ( > IN BOOLEAN EnableOldBSP > ) > { > - PEI_CPU_MP_DATA *PeiCpuMpData; > - UINTN CallerNumber; > - MSR_IA32_APIC_BASE ApicBaseMsr; > + PEI_CPU_MP_DATA *PeiCpuMpData; > + UINTN CallerNumber; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > PeiCpuMpData = GetMpHobData (); > if (PeiCpuMpData == NULL) { > @@ -774,9 +774,9 @@ PeiSwitchBSP ( > // > // Clear the BSP bit of MSR_IA32_APIC_BASE > // > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > - ApicBaseMsr.Bits.Bsp = 0; > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > + ApicBaseMsr.Bits.BSP = 0; > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > > PeiCpuMpData->BSPInfo.State = CPU_SWITCH_STATE_IDLE; > PeiCpuMpData->APInfo.State = CPU_SWITCH_STATE_IDLE; > @@ -805,9 +805,9 @@ PeiSwitchBSP ( > // > // Set the BSP bit of MSR_IA32_APIC_BASE on new BSP > // > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > - ApicBaseMsr.Bits.Bsp = 1; > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > + ApicBaseMsr.Bits.BSP = 1; > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > // > // Set old BSP enable state > // > diff --git a/UefiCpuPkg/Include/Register/LocalApic.h > b/UefiCpuPkg/Include/Register/LocalApic.h > index 346cce6..cfb6d76 100644 > --- a/UefiCpuPkg/Include/Register/LocalApic.h > +++ b/UefiCpuPkg/Include/Register/LocalApic.h > @@ -1,7 +1,7 @@ > /** @file > IA32 Local APIC Definitions. > > - Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -16,11 +16,6 @@ > #define __LOCAL_APIC_H__ > > // > -// Definitions for IA32 architectural MSRs > -// > -#define MSR_IA32_APIC_BASE_ADDRESS 0x1B > - > -// > // Definition for Local APIC registers and related values > // > #define XAPIC_ID_OFFSET 0x20 > @@ -53,19 +48,6 @@ > #define LOCAL_APIC_DESTINATION_SHORTHAND_ALL_INCLUDING_SELF 2 > #define LOCAL_APIC_DESTINATION_SHORTHAND_ALL_EXCLUDING_SELF 3 > > -typedef union { > - struct { > - UINT32 Reserved0:8; ///< Reserved. > - UINT32 Bsp:1; ///< Processor is BSP. > - UINT32 Reserved1:1; ///< Reserved. > - UINT32 Extd:1; ///< Enable x2APIC mode. > - UINT32 En:1; ///< xAPIC global enable/disable. > - UINT32 ApicBaseLow:20; ///< APIC Base physical address. The actual > field width > depends on physical address width. > - UINT32 ApicBaseHigh:32; > - } Bits; > - UINT64 Uint64; > -} MSR_IA32_APIC_BASE; > - > // > // Local APIC Version Register. > // > diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > index 1fca66e..8d0fb02 100644 > --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c > @@ -3,7 +3,7 @@ > > This local APIC library instance supports xAPIC mode only. > > - Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -15,6 +15,7 @@ > **/ > > #include <Register/Cpuid.h> > +#include <Register/Msr.h> > #include <Register/LocalApic.h> > > #include <Library/BaseLib.h> > @@ -67,7 +68,7 @@ GetLocalApicBaseAddress ( > VOID > ) > { > - MSR_IA32_APIC_BASE ApicBaseMsr; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > if (!LocalApicBaseAddressMsrSupported ()) { > // > @@ -77,10 +78,10 @@ GetLocalApicBaseAddress ( > return PcdGet32 (PcdCpuLocalApicBaseAddress); > } > > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > > - return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHigh, 32)) + > - (((UINTN)ApicBaseMsr.Bits.ApicBaseLow) << 12); > + return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHi, 32)) + > + (((UINTN)ApicBaseMsr.Bits.ApicBase) << 12); > } > > /** > @@ -97,7 +98,7 @@ SetLocalApicBaseAddress ( > IN UINTN BaseAddress > ) > { > - MSR_IA32_APIC_BASE ApicBaseMsr; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0); > > @@ -108,12 +109,12 @@ SetLocalApicBaseAddress ( > return; > } > > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > > - ApicBaseMsr.Bits.ApicBaseLow = (UINT32) (BaseAddress >> 12); > - ApicBaseMsr.Bits.ApicBaseHigh = (UINT32) (RShiftU64((UINT64) BaseAddress, > 32)); > + ApicBaseMsr.Bits.ApicBase = (UINT32) (BaseAddress >> 12); > + ApicBaseMsr.Bits.ApicBaseHi = (UINT32) (RShiftU64((UINT64) BaseAddress, > 32)); > > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > } > > /** > @@ -246,18 +247,18 @@ GetApicMode ( > { > DEBUG_CODE ( > { > - MSR_IA32_APIC_BASE ApicBaseMsr; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > // > // Check to see if the CPU supports the APIC Base Address MSR > // > if (LocalApicBaseAddressMsrSupported ()) { > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > // > // Local APIC should have been enabled > // > - ASSERT (ApicBaseMsr.Bits.En != 0); > - ASSERT (ApicBaseMsr.Bits.Extd == 0); > + ASSERT (ApicBaseMsr.Bits.EN != 0); > + ASSERT (ApicBaseMsr.Bits.EXTD == 0); > } > } > ); > diff --git a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > index 38f5370..4c42696 100644 > --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c > @@ -4,7 +4,7 @@ > This local APIC library instance supports x2APIC capable processors > which have xAPIC and x2APIC modes. > > - Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR> > This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD > License > which accompanies this distribution. The full text of the license may be > found at > @@ -16,6 +16,7 @@ > **/ > > #include <Register/Cpuid.h> > +#include <Register/Msr.h> > #include <Register/LocalApic.h> > > #include <Library/BaseLib.h> > @@ -68,7 +69,7 @@ GetLocalApicBaseAddress ( > VOID > ) > { > - MSR_IA32_APIC_BASE ApicBaseMsr; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > if (!LocalApicBaseAddressMsrSupported ()) { > // > @@ -78,10 +79,10 @@ GetLocalApicBaseAddress ( > return PcdGet32 (PcdCpuLocalApicBaseAddress); > } > > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > > - return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHigh, 32)) + > - (((UINTN)ApicBaseMsr.Bits.ApicBaseLow) << 12); > + return (UINTN)(LShiftU64 ((UINT64) ApicBaseMsr.Bits.ApicBaseHi, 32)) + > + (((UINTN)ApicBaseMsr.Bits.ApicBase) << 12); > } > > /** > @@ -98,7 +99,7 @@ SetLocalApicBaseAddress ( > IN UINTN BaseAddress > ) > { > - MSR_IA32_APIC_BASE ApicBaseMsr; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > ASSERT ((BaseAddress & (SIZE_4KB - 1)) == 0); > > @@ -109,12 +110,12 @@ SetLocalApicBaseAddress ( > return; > } > > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > > - ApicBaseMsr.Bits.ApicBaseLow = (UINT32) (BaseAddress >> 12); > - ApicBaseMsr.Bits.ApicBaseHigh = (UINT32) (RShiftU64((UINT64) BaseAddress, > 32)); > + ApicBaseMsr.Bits.ApicBase = (UINT32) (BaseAddress >> 12); > + ApicBaseMsr.Bits.ApicBaseHi = (UINT32) (RShiftU64((UINT64) BaseAddress, > 32)); > > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > } > > /** > @@ -301,7 +302,7 @@ GetApicMode ( > VOID > ) > { > - MSR_IA32_APIC_BASE ApicBaseMsr; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > if (!LocalApicBaseAddressMsrSupported ()) { > // > @@ -310,12 +311,12 @@ GetApicMode ( > return LOCAL_APIC_MODE_XAPIC; > } > > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > // > // Local APIC should have been enabled > // > - ASSERT (ApicBaseMsr.Bits.En != 0); > - if (ApicBaseMsr.Bits.Extd != 0) { > + ASSERT (ApicBaseMsr.Bits.EN != 0); > + if (ApicBaseMsr.Bits.EXTD != 0) { > return LOCAL_APIC_MODE_X2APIC; > } else { > return LOCAL_APIC_MODE_XAPIC; > @@ -339,8 +340,8 @@ SetApicMode ( > IN UINTN ApicMode > ) > { > - UINTN CurrentMode; > - MSR_IA32_APIC_BASE ApicBaseMsr; > + UINTN CurrentMode; > + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; > > if (!LocalApicBaseAddressMsrSupported ()) { > // > @@ -355,9 +356,9 @@ SetApicMode ( > case LOCAL_APIC_MODE_XAPIC: > break; > case LOCAL_APIC_MODE_X2APIC: > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > - ApicBaseMsr.Bits.Extd = 1; > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > + ApicBaseMsr.Bits.EXTD = 1; > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > break; > default: > ASSERT (FALSE); > @@ -369,12 +370,12 @@ SetApicMode ( > // Transition from x2APIC mode to xAPIC mode is a two-step process: > // x2APIC -> Local APIC disabled -> xAPIC > // > - ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE_ADDRESS); > - ApicBaseMsr.Bits.Extd = 0; > - ApicBaseMsr.Bits.En = 0; > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > - ApicBaseMsr.Bits.En = 1; > - AsmWriteMsr64 (MSR_IA32_APIC_BASE_ADDRESS, ApicBaseMsr.Uint64); > + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); > + ApicBaseMsr.Bits.EXTD = 0; > + ApicBaseMsr.Bits.EN = 0; > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > + ApicBaseMsr.Bits.EN = 1; > + AsmWriteMsr64 (MSR_IA32_APIC_BASE, ApicBaseMsr.Uint64); > break; > case LOCAL_APIC_MODE_X2APIC: > break; > -- > 2.7.4.windows.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
