Dear Michael and Liming, I submited the patch as the changes need to be done anyway, though I think the library might be better moved to UefiCpuPkg. Also, is my understanding of the mask value being incorrect right? I was confused by the 'ULL' suffix, which makes it look like it was intended. Is it?
Regards, Marvin. > -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of > Marvin Häuser > Sent: Sunday, July 23, 2017 12:12 PM > To: [email protected] > Cc: [email protected]; [email protected] > Subject: [edk2] [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume > UefiCpuPkg LAPIC code. > > X86TimerLib is changed to use UefiCpuPkg LAPIC register definitions and > LocalApicLib to remove duplicated code. An implicite change is the value > returned by InternalX86GetApicBase() as it now returns the result of > GetLocalApicBaseAddress(), which is the full LAPIC address. This also > implicitely fixes the incorrect mask value used previously, which did not only > mask AcpiBase, but also the first nibble of AcpiBaseHi. This does not apply to > 32-bit platforms. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Marvin Haeuser <[email protected]> > --- > MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c | 35 +++++++-- > ----------- > MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf | 4 ++- > MdePkg/MdePkg.dsc | 3 ++ > 3 files changed, 18 insertions(+), 24 deletions(-) > > diff --git a/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > b/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > index 76c66fbce6fb..fa6e6f213029 100644 > --- a/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > +++ b/MdePkg/Library/SecPeiDxeTimerLibCpu/X86TimerLib.c > @@ -1,7 +1,7 @@ > /** @file > Timer Library functions built upon local APIC on IA32/x64. > > - Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.<BR> > + Copyright (c) 2006 - 2017, 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 @@ -13,18 +13,14 @@ **/ > > #include <Base.h> > +#include <Register/LocalApic.h> > +#include <Library/LocalApicLib.h> > #include <Library/TimerLib.h> > #include <Library/BaseLib.h> > #include <Library/IoLib.h> > #include <Library/PcdLib.h> > #include <Library/DebugLib.h> > > -#define APIC_SVR 0x0f0 > -#define APIC_LVTERR 0x370 > -#define APIC_TMICT 0x380 > -#define APIC_TMCCT 0x390 > -#define APIC_TDCR 0x3e0 > - > // > // The following array is used in calculating the frequency of local APIC // > timer. Refer to IA-32 developers' manual for more details. > @@ -54,30 +50,21 @@ InternalX86GetApicBase ( > VOID > ) > { > - UINTN MsrValue; > UINTN ApicBase; > > - MsrValue = (UINTN) AsmReadMsr64 (27); > - ApicBase = MsrValue & 0xffffff000ULL; > - > // > - // Check the APIC Global Enable bit (bit 11) in IA32_APIC_BASE MSR. > - // This bit will be 1, if local APIC is globally enabled. > + // Verify local APIC is under XAPIC mode. > // > - ASSERT ((MsrValue & BIT11) != 0); > + ASSERT (GetApicMode () == LOCAL_APIC_MODE_XAPIC); > > - // > - // Check the APIC Extended Mode bit (bit 10) in IA32_APIC_BASE MSR. > - // This bit will be 0, if local APIC is under XAPIC mode. > - // > - ASSERT ((MsrValue & BIT10) == 0); > + ApicBase = GetLocalApicBaseAddress (); > > // > // Check the APIC Software Enable/Disable bit (bit 8) in Spurious-Interrupt > // Vector Register. > // This bit will be 1, if local APIC is software enabled. > // > - ASSERT ((MmioRead32 (ApicBase + APIC_SVR) & BIT8) != 0); > + ASSERT ((MmioRead32 (ApicBase + XAPIC_SPURIOUS_VECTOR_OFFSET) & > BIT8) > + != 0); > > return ApicBase; > } > @@ -98,7 +85,9 @@ InternalX86GetTimerFrequency ( { > return > PcdGet32(PcdFSBClock) / > - mTimerLibLocalApicDivisor[MmioBitFieldRead32 (ApicBase + APIC_TDCR, > 0, 3)]; > + mTimerLibLocalApicDivisor[ > + MmioBitFieldRead32 (ApicBase + > XAPIC_TIMER_DIVIDE_CONFIGURATION_OFFSET, 0, 3) > + ]; > } > > /** > @@ -115,7 +104,7 @@ InternalX86GetTimerTick ( > IN UINTN ApicBase > ) > { > - return MmioRead32 (ApicBase + APIC_TMCCT); > + return MmioRead32 (ApicBase + > XAPIC_TIMER_CURRENT_COUNT_OFFSET); > } > > /** > @@ -131,7 +120,7 @@ InternalX86GetInitTimerCount ( > IN UINTN ApicBase > ) > { > - return MmioRead32 (ApicBase + APIC_TMICT); > + return MmioRead32 (ApicBase + XAPIC_TIMER_INIT_COUNT_OFFSET); > } > > /** > diff --git > a/MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf > b/MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf > index a00ebb0eeb64..286da09db174 100644 > --- a/MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf > +++ b/MdePkg/Library/SecPeiDxeTimerLibCpu/SecPeiDxeTimerLibCpu.inf > @@ -13,7 +13,7 @@ > # Note that for IA-32 and x64, this library only supports xAPIC mode. If > x2APIC # support is desired, the SecPeiDxeTimerLibUefiCpu library can be > used. > # > -# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2007 - 2017, 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 @@ -48,6 +48,7 > @@ [Sources.IPF] > > [Packages] > MdePkg/MdePkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > > > [LibraryClasses] > @@ -57,6 +58,7 @@ [LibraryClasses.IA32, LibraryClasses.X64] > PcdLib > IoLib > DebugLib > + LocalApicLib > > [LibraryClasses.IPF] > PalLib > diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc index > 010ce533d7ea..8988d1947566 100644 > --- a/MdePkg/MdePkg.dsc > +++ b/MdePkg/MdePkg.dsc > @@ -35,6 +35,9 @@ [PcdsFixedAtBuild] > [PcdsFixedAtBuild.IPF] > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000 > > +[LibraryClasses] > + > +LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.i > +nf > + > > ########################################################## > ######################################### > # > # Components Section - list of the modules and components that will be > processed by compilation > -- > 2.12.2.windows.2 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
