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

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

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
> 
>