Re: [PATCH v2 0/3] perf/x86/amd/uncore: Add common helpers and Hygon support

[Qi Liu]

Hi Sandipan,

Sorry for the late reply, and thanks for the review.

On 7/3/2026 4:41 PM, Sandipan Das wrote:
> On 01-07-2026 15:04, Qi Liu wrote:
> > This series adds initial Hygon uncore PMU support.
> >
> > Hygon uncore PMUs use a programming model close to the AMD uncore PMU,
> > so this series factors out a small set of common helpers for perf event
> > handling, counter assignment and per-CPU context management.
> >
> > The Hygon-specific support is kept in the Hygon driver. This series
> > supports the Hygon DF and DF IOD PMUs. No Hygon-specific logic is
> > added to the AMD driver.
> >
> > Changes since RFC:
> >    - Address sashiko bot review.
> > Changes since v1:
> >    - Rename the helper namespace to amd_uncore_common_* to avoid
> >      an generic uncore_common_* prefix, and to keep it distinct
> >      from the existing amd_uncore_* names.
> >
> >
> > Qi Liu (3):
> >    perf/x86/amd/uncore: Add common PMU helper functions
> >    perf/x86/amd/uncore: Convert AMD driver to common PMU helpers
> >    perf/x86/amd/uncore: Add Hygon uncore PMU support
> >
> >   arch/x86/events/Kconfig                 |  17 +
> >   arch/x86/events/amd/Makefile            |   4 +
> >   arch/x86/events/amd/amd_uncore_common.c | 403 +++++++++++++++++
> >   arch/x86/events/amd/amd_uncore_common.h | 114 +++++
> >   arch/x86/events/amd/hygon_uncore.c      | 556 +++++++++++++++++++++++
> >   arch/x86/events/amd/uncore.c            | 578 +++---------------------
> >   arch/x86/include/asm/msr-index.h        |   2 +
> >   arch/x86/include/asm/perf_event.h       |  20 +
> >   include/linux/cpuhotplug.h              |   3 +
> >   9 files changed, 1192 insertions(+), 505 deletions(-)
> >   create mode 100644 arch/x86/events/amd/amd_uncore_common.c
> >   create mode 100644 arch/x86/events/amd/amd_uncore_common.h
> >   create mode 100644 arch/x86/events/amd/hygon_uncore.c
> I am trying to understand if the code movement is necessary.
> Wouldn't it be sufficient to just add the following?
>   - new UNCORE_TYPE_HYGON_DF and UNCORE_TYPE_HYGON_DF_IOD types
>   - Hygon-specific attr_groups, pmu->event_init(), pmu->add(),
>     amd_uncore->init() and amd_uncore->scan() implementations

Yes, Use UNCORE_TYPE_HYGON_DF and UNCORE_TYPE_HYGON_DF_IOD would help 
minimize code movement, but Hygon uncore will likely grow more 
Hygon-specific L3/DF/UMC support over time. If we put these PMUs into 
arch/x86/events/amd/uncore.c now, that file may need more Hygon-specific 
types and checks later. So for now, it may be better to keep a separate 
copy in hygon_uncore.c first, as suggested in the earlier review.

> Using new PMU names will also require tools-side changes. Additionally,
> to make the PMUs actually useful, new JSON events and metrics need to
> be added. Are there plans for that?

yes, new PMU names will need perf updates. New JSON events and metrics 
are also needed,  I'll add them later. Thanks!


Qi Liu