Re: [edk2-devel] [PATCH] UefiCpuPkg: Adding a new TSC library by using CPUID(0x15) TSC leaf

["Liming Gao" <liming.gao@intel.com>]

Donald:
  Please also attach the patch linker in BZs. 

  And, please submit another BZ for edk2-platforms\Platform\Intel\KabylakeOpenBoardPkg to apply this new library instance. 

Thanks
Liming
>-----Original Message-----
>From: Kuo, Donald
>Sent: Tuesday, August 20, 2019 10:44 AM
>To: devel@edk2.groups.io; lersek@redhat.com; Gao, Liming
><liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
>Cc: Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Chan, Amy
><amy.chan@intel.com>; Chaganty, Rangasai V
><rangasai.v.chaganty@intel.com>; Lai, Luke <luke.lai@intel.com>; Li, Kevin Y
><kevin.y.li@intel.com>; leif.lindholm@linaro.org; afish@apple.com; Kinney,
>Michael D <michael.d.kinney@intel.com>
>Subject: RE: [edk2-devel] [PATCH] UefiCpuPkg: Adding a new TSC library by
>using CPUID(0x15) TSC leaf
>
>Thanks Laszlo help to review and great feedbacks. That we did miss to fulfil BZ.
>
>I had updated Bugzilla https://bugzilla.tianocore.org/show_bug.cgi?id=1909
>for more documentation.
>
>As I know for the edk2-platforms should be consumed as KBL (7th Generation)
>platform in Client, and this feature based on SDM is supported on SKL (6th
>Generation, Family 06h) onwards. So it's ok to use as TimerLib instances for
>edk2-platforms.
>
>And I think the library is new instances for TimerLib for supported CPU, and
>those non-supported CPU will still keep using AcpiTimerlib as TimerLib
>instances.
>
>Thanks,
>Donald
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Saturday, August 17, 2019 4:40 AM
>> To: Gao, Liming <liming.gao@intel.com>; Kuo, Donald
>> <donald.kuo@intel.com>; Dong, Eric <eric.dong@intel.com>;
>> devel@edk2.groups.io
>> Cc: Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Chan,
>> Amy <amy.chan@intel.com>; Chaganty, Rangasai V
>> <rangasai.v.chaganty@intel.com>; Lai, Luke <luke.lai@intel.com>; Li, Kevin
>> Y <kevin.y.li@intel.com>; leif.lindholm@linaro.org; afish@apple.com;
>Kinney,
>> Michael D <michael.d.kinney@intel.com>
>> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg: Adding a new TSC library by
>> using CPUID(0x15) TSC leaf
>>
>> On 08/16/19 18:16, Laszlo Ersek wrote:
>> > On 08/15/19 06:02, Gao, Liming wrote:
>> >> Donald: This change is a new feature. Now, it is not in edk2 feature
>> >> planning list. If you want to catch it into 201908 stable tag, please
>> >> get approve from Stewards first. I have cc this mail to all Stewards.
>> > - I don't mind adding a new feature, as long as it gets properly
>> > reviewed by package owners before we enter the soft feature freeze.
>> >
>> > - Looking at the BZ
>> > <https://bugzilla.tianocore.org/show_bug.cgi?id=1909>, a bit more
>> > documentation would be nice.
>> >
>> > - On the negative side, I'm very much *not* a fan of adding features
>> > to the open source edk2 tree without actually *consuming* the feature
>> > in an open source tree. Are the new library instances going to be put
>> > to use in edk2-platforms, perhaps?
>> >
>> > We discussed this topic earlier on some of the stewards' calls. On one
>> > hand, it's not uncommon to see library instances from Intel enter core
>> > edk2 packages without any dependent platform code, or even a detailed
>> > problem statement / purpose description (see e.g. commit 5c9bb86f171c
>> > and its surrounding commits). On the other hand, attempts in the past,
>> > to add libraries with well demonstrated and direct in-tree use cases,
>> > to
>> > edk2 core, have been rejected, from other submitters. (Here's one
>> > example: <https://bugzilla.tianocore.org/show_bug.cgi?id=957>.) I'm
>> > not prying at proprietary platform information, but a new library
>> > added to
>> > edk2 core *should* be well-justified.
>> >
>> > The commit message on this patch is empty. It only references
>> > <https://bugzilla.tianocore.org/show_bug.cgi?id=1909>. And if I open
>> > the BZ, this is all I get:
>> >
>> >     Need a new TSC library to check the CPUID leaf (EAX=0x15) for TSC.
>> >     For new platform (start from SKL) can use CPUID and retire/remove
>> >     the current override from AcpiTimerLib.
>> >
>> > Does this read like an actual feature request? (TimerLib is an MdePkg
>> > library class, so not exactly "niche".)
>>
>> In comparison, the following email does read like a feature request:
>>
>> [edk2-devel] Determining TSC frequency programmatically
>> https://edk2.groups.io/g/devel/message/45750
>> http://mid.mail-archive.com/8EC14D0D-DFA5-412D-A4E1-
>> 4D641576D58E@protonmail.com
>>
>> If the posting is related to TianoCore#1909, then I urge the BZ assignee to
>> please reference the message in the TianoCore BZ.
>>
>> Thanks
>> Laszlo
>>
>>