From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id D6CE821AEB0B5 for ; Tue, 25 Jul 2017 19:26:27 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Jul 2017 19:28:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,413,1496127600"; d="scan'208";a="291480512" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga004.fm.intel.com with ESMTP; 25 Jul 2017 19:28:29 -0700 Received: from fmsmsx111.amr.corp.intel.com (10.18.116.5) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 25 Jul 2017 19:28:28 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx111.amr.corp.intel.com (10.18.116.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 25 Jul 2017 19:28:28 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.146]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.94]) with mapi id 14.03.0319.002; Wed, 26 Jul 2017 10:28:24 +0800 From: "Fan, Jeff" To: =?iso-8859-1?Q?Marvin_H=E4user?= , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" Thread-Topic: [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume UefiCpuPkg LAPIC code. Thread-Index: AQHTA5wlcfBDCD5qrEmhamQ76DeIzKJhMXOQgAH8Z8CAAAWKQIACMpSg Date: Wed, 26 Jul 2017 02:28:23 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4C62E463@shsmsx102.ccr.corp.intel.com> References: In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNjliNzgwYjAtNmQ3NS00Mzg1LWI4NTktNjI3MmNlMDNhOWE4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6InhjcGdUT0hhSitIVkVtT0lYT2h3NTMwb0F4N1I4MFJ0UG9PZk92czdZVXc9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume UefiCpuPkg LAPIC code. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Jul 2017 02:26:28 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Marvin, Could the following updating fix your issue? ApicBase =3D MsrValue & ~0xfff; Thanks! Jeff -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Marv= in H=E4user Sent: Tuesday, July 25, 2017 12:57 AM To: edk2-devel@lists.01.org Cc: Kinney, Michael D Subject: Re: [edk2] [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume UefiCpuPkg= LAPIC code. Hi Michael, Sorry, I did not notice that UefiCpuPkg already has a similiar library. Could you please answer my question regarding the mask value which I consid= ered incorrect nevertheless? I am getting compile-time errors as the mask promotes the value to UINT64, = which is, on 32-bit platforms, implicitely casted to UINT32 (UINTN), result= ing in the appropiate data loss warning. Thanks for your answer! Regards, Marvin.=20 > -----Original Message----- > From: Kinney, Michael D [mailto:michael.d.kinney@intel.com] > Sent: Monday, July 24, 2017 6:40 PM > To: Marvin H=E4user ; edk2-=20 > devel@lists.01.org; Kinney, Michael D > Cc: Gao, Liming > Subject: RE: [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume UefiCpuPkg=20 > LAPIC code. >=20 > Hi Marvin, >=20 > We should not add a dependency on the UefiCpuPkg to the MdePkg. I=20 > agree that a better location of the local APIC based timer lib would=20 > be the UefiCpuPkg, and there is already one there called=20 > SecPeiDxeTimerLibUefiCpu. >=20 > The TimeLib in the MdePkg was created before the UefiCpuPkg was=20 > created, which explains the current state of the MdePkg. >=20 > The MdePkg version has other limitations noted in the INF and=20 > recommends the UefiCpuPkg version be used. >=20 > Thanks, >=20 > Mike >=20 > > -----Original Message----- > > From: Marvin H=E4user [mailto:Marvin.Haeuser@outlook.com] > > Sent: Sunday, July 23, 2017 3:19 AM > > To: edk2-devel@lists.01.org > > Cc: Kinney, Michael D ; Gao, Liming=20 > > > > Subject: RE: [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume > UefiCpuPkg > > LAPIC code. > > > > Dear Michael and Liming, > > > > I submited the patch as the changes need to be done anyway, though I=20 > > think the library might be better moved to UefiCpuPkg. > > Also, is my understanding of the mask value being incorrect right? I=20 > > was confused by the 'ULL' suffix, which makes it look like it was=20 > > intended. Is it? > > > > Regards, > > Marvin. > > > > > -----Original Message----- > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On > > Behalf Of > > > Marvin H=E4user > > > Sent: Sunday, July 23, 2017 12:12 PM > > > To: edk2-devel@lists.01.org > > > Cc: michael.d.kinney@intel.com; liming.gao@intel.com > > > Subject: [edk2] [PATCH] MdePkg/SecPeiDxeTimerLibCpu: Consume=20 > > > 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 > > > --- > > > 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.
> > > + Copyright (c) 2006 - 2017, Intel Corporation. All rights=20 > > > + reserved.
> > > 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 > > > +#include > > > +#include > > > #include > > > #include > > > #include > > > #include > > > #include > > > > > > -#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 =3D (UINTN) AsmReadMsr64 (27); > > > - ApicBase =3D 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) !=3D 0); > > > + ASSERT (GetApicMode () =3D=3D 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) =3D=3D 0); > > > + ApicBase =3D 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) !=3D 0); > > > + ASSERT ((MmioRead32 (ApicBase + > > XAPIC_SPURIOUS_VECTOR_OFFSET) & > > > BIT8) > > > + !=3D 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.
> > > +# Copyright (c) 2007 - 2017, Intel Corporation. All rights=20 > > > +reserved.
> > > # > > > # 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|0x0ffffc0000 > > 00 > > > > > > +[LibraryClasses] > > > + > > > > > +LocalApicLib|UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2Ap > > icLib.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 > > > edk2-devel@lists.01.org > > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel