Messages in this thread | | | From | "Zhang, Rui" <> | Subject | Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake | Date | Fri, 6 Jan 2023 14:45:22 +0000 |
| |
On Fri, 2023-01-06 at 12:33 +0100, Ingo Molnar wrote: > * Borislav Petkov <bp@alien8.de> wrote: > > > On Fri, Jan 06, 2023 at 11:56:18AM +0100, Ingo Molnar wrote: > > > It's a trade-off in any case: there's a point where quirk flags > > > or even > > > feature flags become harder to read and harder to maintain than > > > cleanly > > > separated per model driver functions. > > > > Yeah, no, singular: a synthetic feature *flag*: X86_FEATURE_RAPL. > > > > cpu/intel.c can set it and driver can test it. > > > > Everything else inside the driver. > > > > Until Intel can get their act together and actually do a CPUID bit > > like AMD. :-P > > > > But when you think about it, whether the model matching happens in > > the driver or > > in cpu/intel.c doesn't matter a whole lot. > > > > All that matters is, they should finally give it a CPUID bit. > > The other thing that matters here are the RAPL *incompatibilities* > between > model variants, which are significant AFAICS. > > With a CPUID we get a kind of semi-compatible hardware interface with > well > defined semantics & expansion.
Agreed. > > With 'non-architectural', per-model RAPL features we get very little > of > that...
Exactly.
The main purpose of the model list in RAPL PMU code and the intel_rapl driver is to differentiate the model-specific behavior, say, some models use standard energy unit retrieved from MSR some models use a fixed energy unit for Dram Domain and some models use a fixed energy unit for Psys Domain etc.
> > Which is why it's a trade-off that is hard to judge in advance: maybe > we > can simplify the code via a synthethic CPUID[s], maybe it will just > be > another zoo of per-model feature flags...
Agreed.
> Likely won't be able to tell for sure until we see patches. > Yeah, let me cook up a RFC series later and we can continue with that.
thanks, rui
| |