* [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake @ 2023-01-04 14:58 Zhang Rui 2023-01-04 14:58 ` [PATCH 2/2] perf/x86/rapl: Add support for Intel Emerald Rapids Zhang Rui ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Zhang Rui @ 2023-01-04 14:58 UTC (permalink / raw) To: peterz Cc: mingo, tglx, bp, dave.hansen, x86, linux-perf-users, linux-kernel, ak, kan.liang Meteor Lake RAPL support is the same as previous Sky Lake. Add Meteor Lake model for RAPL. Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- arch/x86/events/rapl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c index a829492bca4c..0ef255602aa8 100644 --- a/arch/x86/events/rapl.c +++ b/arch/x86/events/rapl.c @@ -807,6 +807,8 @@ static const struct x86_cpu_id rapl_model_match[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &model_skl), + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE, &model_skl), + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE_L, &model_skl), {}, }; MODULE_DEVICE_TABLE(x86cpu, rapl_model_match); -- 2.25.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] perf/x86/rapl: Add support for Intel Emerald Rapids 2023-01-04 14:58 [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Zhang Rui @ 2023-01-04 14:58 ` Zhang Rui 2023-01-04 20:08 ` [tip: perf/urgent] " tip-bot2 for Zhang Rui 2023-01-04 16:55 ` [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Dave Hansen 2023-01-04 20:08 ` [tip: perf/urgent] " tip-bot2 for Zhang Rui 2 siblings, 1 reply; 18+ messages in thread From: Zhang Rui @ 2023-01-04 14:58 UTC (permalink / raw) To: peterz Cc: mingo, tglx, bp, dave.hansen, x86, linux-perf-users, linux-kernel, ak, kan.liang Emerald Rapids RAPL support is the same as previous Sapphire Rapids. Add Emerald Rapids model for RAPL. Signed-off-by: Zhang Rui <rui.zhang@intel.com> --- arch/x86/events/rapl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c index 0ef255602aa8..8ccf4a36e386 100644 --- a/arch/x86/events/rapl.c +++ b/arch/x86/events/rapl.c @@ -804,6 +804,7 @@ static const struct x86_cpu_id rapl_model_match[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &model_spr), + X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X, &model_spr), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &model_skl), -- 2.25.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [tip: perf/urgent] perf/x86/rapl: Add support for Intel Emerald Rapids 2023-01-04 14:58 ` [PATCH 2/2] perf/x86/rapl: Add support for Intel Emerald Rapids Zhang Rui @ 2023-01-04 20:08 ` tip-bot2 for Zhang Rui 0 siblings, 0 replies; 18+ messages in thread From: tip-bot2 for Zhang Rui @ 2023-01-04 20:08 UTC (permalink / raw) To: linux-tip-commits; +Cc: Zhang Rui, Ingo Molnar, x86, linux-kernel The following commit has been merged into the perf/urgent branch of tip: Commit-ID: 57512b57dcfaf63c52d8ad2fb35321328cde31b0 Gitweb: https://git.kernel.org/tip/57512b57dcfaf63c52d8ad2fb35321328cde31b0 Author: Zhang Rui <rui.zhang@intel.com> AuthorDate: Wed, 04 Jan 2023 22:58:31 +08:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Wed, 04 Jan 2023 21:00:29 +01:00 perf/x86/rapl: Add support for Intel Emerald Rapids Emerald Rapids RAPL support is the same as previous Sapphire Rapids. Add Emerald Rapids model for RAPL. Signed-off-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Link: https://lore.kernel.org/r/20230104145831.25498-2-rui.zhang@intel.com --- arch/x86/events/rapl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c index 589c688..52e6e7e 100644 --- a/arch/x86/events/rapl.c +++ b/arch/x86/events/rapl.c @@ -806,6 +806,7 @@ static const struct x86_cpu_id rapl_model_match[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_L, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE_N, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, &model_spr), + X86_MATCH_INTEL_FAM6_MODEL(EMERALDRAPIDS_X, &model_spr), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &model_skl), ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-04 14:58 [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Zhang Rui 2023-01-04 14:58 ` [PATCH 2/2] perf/x86/rapl: Add support for Intel Emerald Rapids Zhang Rui @ 2023-01-04 16:55 ` Dave Hansen 2023-01-05 6:54 ` Zhang, Rui 2023-01-04 20:08 ` [tip: perf/urgent] " tip-bot2 for Zhang Rui 2 siblings, 1 reply; 18+ messages in thread From: Dave Hansen @ 2023-01-04 16:55 UTC (permalink / raw) To: Zhang Rui, peterz Cc: mingo, tglx, bp, dave.hansen, x86, linux-perf-users, linux-kernel, ak, kan.liang On 1/4/23 06:58, Zhang Rui wrote: > @@ -807,6 +807,8 @@ static const struct x86_cpu_id rapl_model_match[] __initconst = { > X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &model_skl), > X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &model_skl), > X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &model_skl), > + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE, &model_skl), > + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE_L, &model_skl), > {}, > }; Any chance this will ever get architectural enumeration? Or, are we doomed to grow this model list until the end of time? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-04 16:55 ` [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Dave Hansen @ 2023-01-05 6:54 ` Zhang, Rui 2023-01-05 9:55 ` Borislav Petkov 0 siblings, 1 reply; 18+ messages in thread From: Zhang, Rui @ 2023-01-05 6:54 UTC (permalink / raw) To: peterz, Hansen, Dave Cc: kan.liang, ak, x86, dave.hansen, bp, mingo, tglx, linux-kernel, linux-perf-users On Wed, 2023-01-04 at 08:55 -0800, Dave Hansen wrote: > On 1/4/23 06:58, Zhang Rui wrote: > > @@ -807,6 +807,8 @@ static const struct x86_cpu_id > > rapl_model_match[] __initconst = { > > X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &model_skl) > > , > > X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &model_skl), > > X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &model_skl), > > + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE, &model_skl) > > , > > + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE_L, &model_skl), > > {}, > > }; > > Any chance this will ever get architectural enumeration? We will get rid of the non-architechtural MSR interface on future server platforms. But for client, we still have to live with it so far. > Or, are we > doomed to grow this model list until the end of time? I thought of this before and got some ideas related. Say, instead of maintaining the model list in a series of drivers, can we have something similar to "cpu_feature" instead? The feature flags can be set in a central place, when adding the new CPU ID. Then all these drivers can just probe based on the feature flag. There are a list of drivers that could benefit from this, say, intel_rapl, RAPL PMU, turbostat, thermal tcc cooling, PMC, etc. currently, all these drivers have their own model lists. thanks, rui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-05 6:54 ` Zhang, Rui @ 2023-01-05 9:55 ` Borislav Petkov 2023-01-06 6:05 ` Zhang, Rui 0 siblings, 1 reply; 18+ messages in thread From: Borislav Petkov @ 2023-01-05 9:55 UTC (permalink / raw) To: Zhang, Rui Cc: peterz, Hansen, Dave, kan.liang, ak, x86, dave.hansen, mingo, tglx, linux-kernel, linux-perf-users On Thu, Jan 05, 2023 at 06:54:31AM +0000, Zhang, Rui wrote: > I thought of this before and got some ideas related. > Say, instead of maintaining the model list in a series of drivers, can > we have something similar to "cpu_feature" instead? Yes, you can define a synthetic X86_FEATURE flag and set it for each CPU model which supports the feature in arch/x86/kernel/cpu/intel.c so that at least all the model matching gunk is kept where it belongs, in the CPU init code and the other code simply tests that flag. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-05 9:55 ` Borislav Petkov @ 2023-01-06 6:05 ` Zhang, Rui 2023-01-06 10:14 ` Ingo Molnar 2023-01-06 10:39 ` Borislav Petkov 0 siblings, 2 replies; 18+ messages in thread From: Zhang, Rui @ 2023-01-06 6:05 UTC (permalink / raw) To: bp Cc: Hansen, Dave, ak, x86, dave.hansen, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel On Thu, 2023-01-05 at 10:55 +0100, Borislav Petkov wrote: > On Thu, Jan 05, 2023 at 06:54:31AM +0000, Zhang, Rui wrote: > > I thought of this before and got some ideas related. > > Say, instead of maintaining the model list in a series of drivers, > > can > > we have something similar to "cpu_feature" instead? > > Yes, you can define a synthetic X86_FEATURE flag and set it for each > CPU model > which supports the feature in arch/x86/kernel/cpu/intel.c so that at > least all > the model matching gunk is kept where it belongs, in the CPU init > code and the > other code simply tests that flag. Great, thanks for this info. But I still have a question. Take RAPL feature for example, the feature is not architectural, although 80% of the platforms may follow the same behavior, but there are still cases that behave differently. And so far, there are 8 different behaviors based on different models. In this case, can we have several different flags for the RAPL feature and make the RAPL driver probe on different RAPL flags? Or else, a model list is still needed. thanks, rui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 6:05 ` Zhang, Rui @ 2023-01-06 10:14 ` Ingo Molnar 2023-01-06 10:39 ` Borislav Petkov 1 sibling, 0 replies; 18+ messages in thread From: Ingo Molnar @ 2023-01-06 10:14 UTC (permalink / raw) To: Zhang, Rui Cc: bp, Hansen, Dave, ak, x86, dave.hansen, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel * Zhang, Rui <rui.zhang@intel.com> wrote: > On Thu, 2023-01-05 at 10:55 +0100, Borislav Petkov wrote: > > On Thu, Jan 05, 2023 at 06:54:31AM +0000, Zhang, Rui wrote: > > > I thought of this before and got some ideas related. > > > Say, instead of maintaining the model list in a series of drivers, > > > can > > > we have something similar to "cpu_feature" instead? > > > > Yes, you can define a synthetic X86_FEATURE flag and set it for each > > CPU model which supports the feature in arch/x86/kernel/cpu/intel.c so > > that at least all the model matching gunk is kept where it belongs, in > > the CPU init code and the other code simply tests that flag. > > Great, thanks for this info. > > But I still have a question. > > Take RAPL feature for example, the feature is not architectural, although > 80% of the platforms may follow the same behavior, but there are still > cases that behave differently. And so far, there are 8 different > behaviors based on different models. > > In this case, can we have several different flags for the RAPL feature > and make the RAPL driver probe on different RAPL flags? Or else, a model > list is still needed. Introducing a synthethic CPU flag only makes sense for behavior that is near-100% identical among models - ie. if the only thing missing is the CPUID enumeration. If RAPL details are continuously changing among CPU models, with no real architected compatibility guarantees, then it probably only makes sense to introduce the flag once it's enumerated at the CPUID level as well. Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 6:05 ` Zhang, Rui 2023-01-06 10:14 ` Ingo Molnar @ 2023-01-06 10:39 ` Borislav Petkov 2023-01-06 10:56 ` Ingo Molnar 2023-01-06 14:38 ` Zhang, Rui 1 sibling, 2 replies; 18+ messages in thread From: Borislav Petkov @ 2023-01-06 10:39 UTC (permalink / raw) To: Zhang, Rui Cc: Hansen, Dave, ak, x86, dave.hansen, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel On Fri, Jan 06, 2023 at 06:05:41AM +0000, Zhang, Rui wrote: > But I still have a question. > Take RAPL feature for example, the feature is not architectural, > although 80% of the platforms may follow the same behavior, but there > are still cases that behave differently. And so far, there are 8 > different behaviors based on different models. > > In this case, can we have several different flags for the RAPL feature > and make the RAPL driver probe on different RAPL flags? Or else, a > model list is still needed. Well, you asked about detecting CPUs supporting RAPL. Now you're asking about different RAPL "subfunctionality" or whatever. You could do the synthetic flag for feature detection because apparently giving it a CPUID flag is soo expensive (/sarcastic eyeroll) and then you can pick apart subfeatures in the RAPL code and do flags there, away from the x86 arch code because no one should see that. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 10:39 ` Borislav Petkov @ 2023-01-06 10:56 ` Ingo Molnar 2023-01-06 11:11 ` Borislav Petkov 2023-01-06 14:38 ` Zhang, Rui 1 sibling, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2023-01-06 10:56 UTC (permalink / raw) To: Borislav Petkov Cc: Zhang, Rui, Hansen, Dave, ak, x86, dave.hansen, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel * Borislav Petkov <bp@alien8.de> wrote: > On Fri, Jan 06, 2023 at 06:05:41AM +0000, Zhang, Rui wrote: > > But I still have a question. > > Take RAPL feature for example, the feature is not architectural, > > although 80% of the platforms may follow the same behavior, but there > > are still cases that behave differently. And so far, there are 8 > > different behaviors based on different models. > > > > In this case, can we have several different flags for the RAPL feature > > and make the RAPL driver probe on different RAPL flags? Or else, a > > model list is still needed. > > Well, you asked about detecting CPUs supporting RAPL. > > Now you're asking about different RAPL "subfunctionality" or whatever. > > You could do the synthetic flag for feature detection because apparently > giving it a CPUID flag is soo expensive (/sarcastic eyeroll) and then you > can pick apart subfeatures in the RAPL code and do flags there, away from > the x86 arch code because no one should see that. 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. Especially if internally at Intel RAPL functionality is not yet treated as an 'architected' feature, and new aspects are added in a less disciplined fashion ... Sometimes the addition of an 'architected' CPU feature iterates the feature-set non-trivially - as people consider it a last-minute chance to get in their favorite features without having to deal with backwards compatibility ... So I'm somewhat pessimistically leaning towards the current status quo of per model enumeration. Would be glad to be proven wrong too. :-) Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 10:56 ` Ingo Molnar @ 2023-01-06 11:11 ` Borislav Petkov 2023-01-06 11:33 ` Ingo Molnar 0 siblings, 1 reply; 18+ messages in thread From: Borislav Petkov @ 2023-01-06 11:11 UTC (permalink / raw) To: Ingo Molnar Cc: Zhang, Rui, Hansen, Dave, ak, x86, dave.hansen, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel 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. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 11:11 ` Borislav Petkov @ 2023-01-06 11:33 ` Ingo Molnar 2023-01-06 14:45 ` Zhang, Rui 0 siblings, 1 reply; 18+ messages in thread From: Ingo Molnar @ 2023-01-06 11:33 UTC (permalink / raw) To: Borislav Petkov Cc: Zhang, Rui, Hansen, Dave, ak, x86, dave.hansen, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel * 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. With 'non-architectural', per-model RAPL features we get very little of that... 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... Likely won't be able to tell for sure until we see patches. Thanks, Ingo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 11:33 ` Ingo Molnar @ 2023-01-06 14:45 ` Zhang, Rui 0 siblings, 0 replies; 18+ messages in thread From: Zhang, Rui @ 2023-01-06 14:45 UTC (permalink / raw) To: mingo, bp Cc: Hansen, Dave, ak, dave.hansen, x86, peterz, linux-perf-users, mingo, tglx, kan.liang, linux-kernel 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 10:39 ` Borislav Petkov 2023-01-06 10:56 ` Ingo Molnar @ 2023-01-06 14:38 ` Zhang, Rui 2023-01-06 14:49 ` Borislav Petkov 2023-01-06 14:50 ` Dave Hansen 1 sibling, 2 replies; 18+ messages in thread From: Zhang, Rui @ 2023-01-06 14:38 UTC (permalink / raw) To: bp Cc: Hansen, Dave, ak, dave.hansen, x86, peterz, linux-perf-users, mingo, tglx, kan.liang, Bityutskiy, Artem, linux-kernel Hi, Boris, On Fri, 2023-01-06 at 11:39 +0100, Borislav Petkov wrote: > On Fri, Jan 06, 2023 at 06:05:41AM +0000, Zhang, Rui wrote: > > But I still have a question. > > Take RAPL feature for example, the feature is not architectural, > > although 80% of the platforms may follow the same behavior, but > > there > > are still cases that behave differently. And so far, there are 8 > > different behaviors based on different models. > > > > In this case, can we have several different flags for the RAPL > > feature > > and make the RAPL driver probe on different RAPL flags? Or else, a > > model list is still needed. > > Well, you asked about detecting CPUs supporting RAPL. > > Now you're asking about different RAPL "subfunctionality" or > whatever. > Sorry that I was not clear enough. My original proposal is that, instead of maintaining model lists in a series of different drivers, can we use feature flags instead, and maintain them in a central place instead of different drivers. say, something like static const struct x86_cpu_id intel_pm_features[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L, X86_FEATURE_RAPL | X86_FEATURE_TCC_COOLING), X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, X86_FEATURE_RAPL | X86_FEATURE_UNCORE_FREQ), ... X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, X86_FEATURE_RAPL | X86_FEATURE_TCC_COOLING), X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, X86_FEATURE_RAPL | X86_FEATURE_UNCORE_FREQ), ... {}, }; And then set the feature flags based on this, and make the drivers test the feature flags. The goal of this is to do model list update in one place instead of 4 or more different drivers when a new model comes. If yes, then, the second question is that, there are cases like RAPL which has model specific behavior. To make the driver totally clean, I' m wondering if we can have different flags for one feature so that we don't need to maintain the exceptions in the driver. Say, something like static const struct x86_cpu_id intel_pm_features[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L, X86_FEATURE_RAPL_DEF AULT | X86_FEATURE_TCC_COOLING), X86_MATCH_INTEL_FAM6_MODEL(SKYL AKE_X, X86_FEATURE_RAPL_FIX_DRAM | X86_FEATURE_UNCORE_FREQ), ... X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, X86_FEATURE_RAPL_DEFA ULT | X86_FEATURE_TCC_COOLING), X86_MATCH_INTEL_FAM6_MODEL(SAPPH IRERAPIDS_X, X86_FEATURE_RAPL_FIX_PSYS | X86_FEATURE_UNCORE_FREQ), ... {}, }; > You could do the synthetic flag for feature detection because > apparently giving > it a CPUID flag is soo expensive (/sarcastic eyeroll) and then you > can pick > apart subfeatures in the RAPL code and do flags there, away from the > x86 arch > code because no one should see that. I got your point. thanks, rui ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 14:38 ` Zhang, Rui @ 2023-01-06 14:49 ` Borislav Petkov 2023-01-06 14:50 ` Dave Hansen 1 sibling, 0 replies; 18+ messages in thread From: Borislav Petkov @ 2023-01-06 14:49 UTC (permalink / raw) To: Zhang, Rui Cc: Hansen, Dave, ak, dave.hansen, x86, peterz, linux-perf-users, mingo, tglx, kan.liang, Bityutskiy, Artem, linux-kernel On Fri, Jan 06, 2023 at 02:38:10PM +0000, Zhang, Rui wrote: > And then set the feature flags based on this, and make the drivers test > the feature flags. That would be the purpose of synthetic flags. > The goal of this is to do model list update in one place instead of 4 > or more different drivers when a new model comes. Do you really have to update 4 different places each time? As said before, you have to do the model matching *somewhere*. If you have to do model matching in a lot of drivers - and it looks like you do - judging by $ git grep X86_MATCH_INTEL_FAM6 output, then doing the matching once in cpu/intel.c and setting a synthetic flag does make sense because all that matching code will disappear from all the drivers but be concentrated in one place. Thx. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 14:38 ` Zhang, Rui 2023-01-06 14:49 ` Borislav Petkov @ 2023-01-06 14:50 ` Dave Hansen 2023-01-07 14:07 ` Zhang, Rui 1 sibling, 1 reply; 18+ messages in thread From: Dave Hansen @ 2023-01-06 14:50 UTC (permalink / raw) To: Zhang, Rui, bp Cc: ak, dave.hansen, x86, peterz, linux-perf-users, mingo, tglx, kan.liang, Bityutskiy, Artem, linux-kernel On 1/6/23 06:38, Zhang, Rui wrote: > My original proposal is that, instead of maintaining model lists in a > series of different drivers, can we use feature flags instead, and > maintain them in a central place instead of different drivers. say, > something like > > static const struct x86_cpu_id intel_pm_features[] __initconst = { > X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L, X86_FEATURE_RAPL | X86_FEATURE_TCC_COOLING), > X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, X86_FEATURE_RAPL | X86_FEATURE_UNCORE_FREQ), > ... > X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, X86_FEATURE_RAPL | X86_FEATURE_TCC_COOLING), > X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, X86_FEATURE_RAPL | X86_FEATURE_UNCORE_FREQ), > ... > {}, > }; > And then set the feature flags based on this, and make the drivers test > the feature flags. That works if you have very few features. SKYLAKE_X looks to have on the order of 15 model-specific features, or at least references in the code. That means that the X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, ... list goes on for 15 features. It's even worse than that because you'd *like* to be able to scan up and down the list looking for, say, "all the CPUs that support RAPL". But, if you do that, you actually need a table -- a really wide table -- for *all* the features and a column for each. What we have now isn't bad. The only real way to fix this is to have the features enumerated *properly*, aka. architecturally. I get it, Intel doesn't want to dedicate CPUID bits and architecture to one-offs. But, at the point that there are a dozen CPU models across three or four different CPU generations, it's time to revisit it. Could you help our colleagues revisit it, please? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-06 14:50 ` Dave Hansen @ 2023-01-07 14:07 ` Zhang, Rui 0 siblings, 0 replies; 18+ messages in thread From: Zhang, Rui @ 2023-01-07 14:07 UTC (permalink / raw) To: Hansen, Dave, bp Cc: linux-kernel, ak, x86, dave.hansen, peterz, linux-perf-users, tglx, mingo, Bityutskiy, Artem, kan.liang On Fri, 2023-01-06 at 06:50 -0800, Dave Hansen wrote: > On 1/6/23 06:38, Zhang, Rui wrote: > > My original proposal is that, instead of maintaining model lists in > > a > > series of different drivers, can we use feature flags instead, and > > maintain them in a central place instead of different drivers. say, > > something like > > > > static const struct x86_cpu_id intel_pm_features[] __initconst = { > > X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_L, X86_FEATURE > > _RAPL | X86_FEATURE_TCC_COOLING), > > X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, X86_FEATURE > > _RAPL | X86_FEATURE_UNCORE_FREQ), > > ... > > X86_MATCH_INTEL_FAM6_MODEL(ALDERLAKE, X86_FEATURE > > _RAPL | X86_FEATURE_TCC_COOLING), > > X86_MATCH_INTEL_FAM6_MODEL(SAPPHIRERAPIDS_X, X86_FEATURE > > _RAPL | X86_FEATURE_UNCORE_FREQ), > > ... > > {}, > > }; > > And then set the feature flags based on this, and make the drivers > > test > > the feature flags. > > That works if you have very few features. SKYLAKE_X looks to have on > the order of 15 model-specific features, or at least references in > the code. > > That means that the > > X86_MATCH_INTEL_FAM6_MODEL(SKYLAKE_X, ... > > list goes on for 15 features. It's even worse than that because > you'd > *like* to be able to scan up and down the list looking for, say, "all > the CPUs that support RAPL". But, if you do that, you actually need > a > table -- a really wide table -- for *all* the features and a column > for > each. That's true. > > What we have now isn't bad. The only real way to fix this is to have > the features enumerated *properly*, aka. architecturally. > > I get it, Intel doesn't want to dedicate CPUID bits and architecture > to > one-offs. > But, at the point that there are a dozen CPU models across > three or four different CPU generations, it's time to revisit > it. Could > you help our colleagues revisit it, please? For this RAPL case, I think the biggest problem is the RAPL *incompatibilities* between model variants as Ingo pointed out. So a CPUID bit can not solve all the problems. But given that the biggest inconsistency is the energy unit used on different generations, I can also check with our colleagues if there is a software visible way to get the "fixed" energy units rather than hardcoding it in the driver using a model list. thanks, rui ^ permalink raw reply [flat|nested] 18+ messages in thread
* [tip: perf/urgent] perf/x86/rapl: Add support for Intel Meteor Lake 2023-01-04 14:58 [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Zhang Rui 2023-01-04 14:58 ` [PATCH 2/2] perf/x86/rapl: Add support for Intel Emerald Rapids Zhang Rui 2023-01-04 16:55 ` [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Dave Hansen @ 2023-01-04 20:08 ` tip-bot2 for Zhang Rui 2 siblings, 0 replies; 18+ messages in thread From: tip-bot2 for Zhang Rui @ 2023-01-04 20:08 UTC (permalink / raw) To: linux-tip-commits; +Cc: Zhang Rui, Ingo Molnar, x86, linux-kernel The following commit has been merged into the perf/urgent branch of tip: Commit-ID: f52853a668bfeddd79f319d536a506f68cc2b478 Gitweb: https://git.kernel.org/tip/f52853a668bfeddd79f319d536a506f68cc2b478 Author: Zhang Rui <rui.zhang@intel.com> AuthorDate: Wed, 04 Jan 2023 22:58:30 +08:00 Committer: Ingo Molnar <mingo@kernel.org> CommitterDate: Wed, 04 Jan 2023 21:00:28 +01:00 perf/x86/rapl: Add support for Intel Meteor Lake Meteor Lake RAPL support is the same as previous Sky Lake. Add Meteor Lake model for RAPL. Signed-off-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Ingo Molnar <mingo@kernel.org> Link: https://lore.kernel.org/r/20230104145831.25498-1-rui.zhang@intel.com --- arch/x86/events/rapl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/x86/events/rapl.c b/arch/x86/events/rapl.c index ae5779e..589c688 100644 --- a/arch/x86/events/rapl.c +++ b/arch/x86/events/rapl.c @@ -809,6 +809,8 @@ static const struct x86_cpu_id rapl_model_match[] __initconst = { X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_P, &model_skl), X86_MATCH_INTEL_FAM6_MODEL(RAPTORLAKE_S, &model_skl), + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE, &model_skl), + X86_MATCH_INTEL_FAM6_MODEL(METEORLAKE_L, &model_skl), {}, }; MODULE_DEVICE_TABLE(x86cpu, rapl_model_match); ^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-01-07 14:07 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-01-04 14:58 [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Zhang Rui 2023-01-04 14:58 ` [PATCH 2/2] perf/x86/rapl: Add support for Intel Emerald Rapids Zhang Rui 2023-01-04 20:08 ` [tip: perf/urgent] " tip-bot2 for Zhang Rui 2023-01-04 16:55 ` [PATCH 1/2] perf/x86/rapl: Add support for Intel Meteor Lake Dave Hansen 2023-01-05 6:54 ` Zhang, Rui 2023-01-05 9:55 ` Borislav Petkov 2023-01-06 6:05 ` Zhang, Rui 2023-01-06 10:14 ` Ingo Molnar 2023-01-06 10:39 ` Borislav Petkov 2023-01-06 10:56 ` Ingo Molnar 2023-01-06 11:11 ` Borislav Petkov 2023-01-06 11:33 ` Ingo Molnar 2023-01-06 14:45 ` Zhang, Rui 2023-01-06 14:38 ` Zhang, Rui 2023-01-06 14:49 ` Borislav Petkov 2023-01-06 14:50 ` Dave Hansen 2023-01-07 14:07 ` Zhang, Rui 2023-01-04 20:08 ` [tip: perf/urgent] " tip-bot2 for Zhang Rui
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.