mbox series

[0/5] intel_rapl & perf rapl: combine PMU support

Message ID 20240131142335.84218-1-rui.zhang@intel.com (mailing list archive)
Headers show
Series intel_rapl & perf rapl: combine PMU support | expand

Message

Zhang, Rui Jan. 31, 2024, 2:23 p.m. UTC 
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.

thanks,
rui

----------------------------------------------------------------
Zhang Rui (5):
      powercap: intel_rapl: Sort header files
      powercap: intel_rapl: Add PMU support
      powercap: intel_rapl_tpmi: Enable PMU support for TPMI RAPL
      powercap: intel_rapl: Fix BROADWELL_D
      powercap: intel_rapl_msr: Enable PMU support for MSR RAPL

 arch/x86/events/Kconfig              |   8 -
 arch/x86/events/Makefile             |   1 -
 arch/x86/events/rapl.c               | 871 -----------------------------------
 drivers/powercap/intel_rapl_common.c | 562 +++++++++++++++++++++-
 drivers/powercap/intel_rapl_msr.c    |   2 +
 drivers/powercap/intel_rapl_tpmi.c   |   1 +
 include/linux/intel_rapl.h           |  17 +
 7 files changed, 569 insertions(+), 893 deletions(-)
 delete mode 100644 arch/x86/events/rapl.c

Comments

Rafael J. Wysocki Jan. 31, 2024, 2:40 p.m. UTC | #1 
On Wed, Jan 31, 2024 at 3:24 PM Zhang Rui <rui.zhang@intel.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?

Regardless of the above. if any existing code is moved unmodified by
this series to a new location, it would be nice to be able to see that
in the patches.  Otherwise, some subtle differences may be missed.
Zhang, Rui Feb. 1, 2024, 5:35 a.m. UTC | #2 
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@intel.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.