> From: Tyler Retzlaff [mailto:roret...@linux.microsoft.com] > Sent: Friday, 13 January 2023 20.22 > > On Fri, Jan 13, 2023 at 07:44:57AM +0000, Tomasz Duszynski wrote: > > > > >From: Tyler Retzlaff <roret...@linux.microsoft.com> > > >Sent: Wednesday, January 11, 2023 10:06 PM > > > > > >On Wed, Jan 11, 2023 at 09:39:35AM +0000, Tomasz Duszynski wrote: > > >> Hi Tyler, > > >> > > >> >From: Tyler Retzlaff <roret...@linux.microsoft.com> > > >> >Sent: Wednesday, January 11, 2023 1:32 AM > > >> > > > >> >hi, > > >> > > > >> >don't interpret this as an objection to the functionality but > this > > >> >looks like a clear example of something that doesn't belong in > the > > >> >EAL. has there been a discussion as to whether or not this should > be in a separate library? > > >> > > >> No, I don't recall anybody having any concerns about the code > > >> placement. Rationale behind making this part of eal was based on > the > > >> fact that tracing itself is a part of eal and since this was meant > to be extension to tracing, > > >code placement decision came out naturally. > > >> > > >> During development phase idea evolved a bit and what initially was > > >> supposed to be solely yet another tracepoint become generic API to > > >> read pmu and tracepoint based on that. Which means both can be > used independently. > > >> > > >> That said, since this code has both platform agnostic and platform > specific parts this can either > > >be split into: > > >> 1. library + eal platform code > > >> 2. all under eal > > >> > > >> Either approach seems legit. Thoughts? > > >> > > >> > > > >> >a basic test is whether or not an implementation exists or can be > > >> >reasonably provided for all platforms and that isn't strictly > evident > > >> >here. red flag is to see yet more code being added conditionally > compiled for a single platform. > > >> > > >> Even libs are not entirely pristine and have platform specific > ifdefs > > >> lurking so not sure where this red flag is coming from. > > > > > >i think red flag was probably the wrong term to use sorry for that. > > >rather i should say it is an indicator that the api probably doesn't > belong in the eal. > > > > > >fundamentally the purpose of the abstraction library is to relieve > the application from having to > > >do conditional compilation and/or execution for the subject apis > coming from eal. including and > > >exporting apis that work for only one platform is in direct > contradiction. > > > > > >please explore adding this as a separate library, it is understood > that there are tradeoffs > > >involved. > > > > > >thanks! > > > > Any ideas how to name the library? > > naming is always so hard and i'm definitely not authoritative. > > it seems like lib/pmu would be the least churn against your existing > patch series, here are some other suggestions that might work.
+1 to lib/pmu Less work, as Tyler already mentioned. Furthermore: Both Intel and ARM use the term Performance Monitoring Unit (abbreviated PMU). Microsoft does too [1]. [1]: https://learn.microsoft.com/en-us/windows-hardware/test/wpt/recording-pmu-events RISC-V uses the term Hardware Performance Monitor (abbreviated HPM). I haven't checked other CPU vendors.
