[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0b326053b1a91d0325cd6907bab784e8f9a8cadb.camel@intel.com>
Date: Thu, 1 Feb 2024 05:35:04 +0000
From: "Zhang, Rui" <rui.zhang@...el.com>
To: "rafael@...nel.org" <rafael@...nel.org>
CC: "peterz@...radead.org" <peterz@...radead.org>, "mingo@...hat.com"
<mingo@...hat.com>, "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"kan.liang@...ux.intel.com" <kan.liang@...ux.intel.com>, "x86@...nel.org"
<x86@...nel.org>
Subject: Re: [PATCH 0/5] intel_rapl & perf rapl: combine PMU support
On Wed, 2024-01-31 at 15:40 +0100, Rafael J. Wysocki wrote:
> On Wed, Jan 31, 2024 at 3:24 PM Zhang Rui <rui.zhang@...el.com>
> wrote:
> >
> > This patch series is made based on the patch series posted at
> > https://lore.kernel.org/all/20240131113713.74779-1-rui.zhang@intel.com/
> >
> > Problem statement
> > -----------------
> > MSR RAPL powercap sysfs is done in
> > drivers/powercap/intel_rapl_msr.c.
> > MSR RAPL PMU is done in arch/x86/events/rapl.c.
> >
> > They maintain two separate CPU model lists, describing the same
> > feature
> > available on the same set of hardware. This increases unnecessary
> > maintenance burden a lot.
> >
> > Now we need to introduce TPMI RAPL PMU support, which again shares
> > most
> > of the logic with MSR RAPL PMU.
> >
> > Solution
> > --------
> > Introducing PMU support as part of RAPL framework and remove
> > current MSR
> > RAPL PMU code.
> >
> > The idea is that, if a RAPL Package device is registered to RAPL
> > framework, and is ready for energy reporting and control via
> > powercap
> > sysfs, then it is also ready for PMU.
> >
> > So introducing PMU support in RAPL framework that works for all
> > registered RAPL Package devices. With this, we can remove current
> > MSR
> > RAPL PMU completely.
> >
> > Given that MSR RAPL and TPMI RAPL driver won't funtion on the same
> > platform, the new RAPL PMU can be fully compatible with current MSR
> > RAPL
> > PMU, including using the same PMU name and events
> > name/id/unit/scale.
> >
> > For example, on platforms use either MSR or TPMI, use the same
> > command
> > perf stat -e power/energy-pkg/ -e power/energy-ram/ -e
> > power/energy-cores/ FOO
> > to get the energy consumption when the events are in "perf list"
> > output.
> >
> > Notes
> > -----
> > There are indeed some functional changes introduced, due to the
> > divergency between the two CPU model lists. This includes,
> > 1. Fix BROADWELL_D in intel_rapl driver to use fixed Dram domain
> > energy
> > unit.
> > 2. Enable PMU for some Intel platforms, which were missing in
> > arch/x86/events/rapl.c. This includes
> > ICELAKE_NNPI
> > ROCKETLAKE
> > LUNARLAKE_M
> > LAKEFIELD
> > ATOM_SILVERMONT
> > ATOM_SILVERMONT_MID
> > ATOM_AIRMONT
> > ATOM_AIRMONT_MID
> > ATOM_TREMONT
> > ATOM_TREMONT_D
> > ATOM_TREMONT_L
> > 3. Change the logic for enumerating AMD/HYGON platforms
> > Previously, it was
> > X86_MATCH_FEATURE(X86_FEATURE_RAPL,
> > &model_amd_hygon)
> > And now it is
> > X86_MATCH_VENDOR_FAM(AMD, 0x17, &rapl_defaults_amd)
> > X86_MATCH_VENDOR_FAM(AMD, 0x19, &rapl_defaults_amd)
> > X86_MATCH_VENDOR_FAM(HYGON, 0x18, &rapl_defaults_amd)
> >
> > Any comments/concerns are welcome.
>
> Say the first patch in the series is applied and the last one is not.
> Will anything break?
No. Without the last patch
1. for platforms using TPMI RAPL, .enable_pmu flag is set and PMU is
registered via RAPL framework
2. for platforms using MSR RAPL, it doesn't set .enable_pmu flag, and
the PMU is registered by arch/x86/events/rapl.c
>
> Regardless of the above. if any existing code is moved unmodified by
> this series to a new location,
intel_rapl PMU support shares a lot of code with
arch/x86/events/rapl.c, but still there are quite a lot of differences.
Including
1. dynamic PMU probing
2. using intel_rapl wrappers to get energy units and read energy
counter
etc.
thanks,
rui
> it would be nice to be able to see that
> in the patches. Otherwise, some subtle differences may be missed.
Powered by blists - more mailing lists