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

["Donald Kuo" <donald.kuo@intel.com>]

Hi Liming,

Done.

Patch is attached to https://bugzilla.tianocore.org/show_bug.cgi?id=1909

Another BZ to apply CpuTimerLib will be tracking on:  https://bugzilla.tianocore.org/show_bug.cgi?id=2096

Thanks,
Donald

> -----Original Message-----
> From: Gao, Liming
> Sent: Tuesday, August 20, 2019 2:51 PM
> To: Kuo, Donald <donald.kuo@intel.com>; devel@edk2.groups.io;
> lersek@redhat.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
> 
> 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
> >>
> >>