Re: [PATCH V2 08/25] perf/x86: Hybrid PMU support for hardware cache event

[Liang, Kan]

On 3/11/2021 6:07 AM, Peter Zijlstra wrote:

On Wed, Mar 10, 2021 at 08:37:44AM -0800, kan.li...@linux.intel.com wrote:

From: Kan Liang <kan.li...@linux.intel.com>

The hardware cache events are different among hybrid PMUs. Each hybrid
PMU should have its own hw cache event table.

Reviewed-by: Andi Kleen <a...@linux.intel.com>
Signed-off-by: Kan Liang <kan.li...@linux.intel.com>
---
  arch/x86/events/core.c       | 11 +++++++++--
  arch/x86/events/perf_event.h |  9 +++++++++
  2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 039a851..1db4a67 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -352,6 +352,7 @@ set_ext_hw_attr(struct hw_perf_event *hwc, struct 
perf_event *event)
  {
        struct perf_event_attr *attr = &event->attr;
        unsigned int cache_type, cache_op, cache_result;
+       struct x86_hybrid_pmu *pmu = is_hybrid() ? hybrid_pmu(event->pmu) : 
NULL;
        u64 config, val;

config = attr->config;

@@ -371,7 +372,10 @@ set_ext_hw_attr(struct hw_perf_event *hwc, struct 
perf_event *event)
                return -EINVAL;
        cache_result = array_index_nospec(cache_result, 
PERF_COUNT_HW_CACHE_RESULT_MAX);

- val = hw_cache_event_ids[cache_type][cache_op][cache_result];

+       if (pmu)
+               val = 
pmu->hw_cache_event_ids[cache_type][cache_op][cache_result];
+       else
+               val = hw_cache_event_ids[cache_type][cache_op][cache_result];

if (val == 0)

                return -ENOENT;
@@ -380,7 +384,10 @@ set_ext_hw_attr(struct hw_perf_event *hwc, struct 
perf_event *event)
                return -EINVAL;

hwc->config |= val;

-       attr->config1 = hw_cache_extra_regs[cache_type][cache_op][cache_result];
+       if (pmu)
+               attr->config1 = 
pmu->hw_cache_extra_regs[cache_type][cache_op][cache_result];
+       else
+               attr->config1 = 
hw_cache_extra_regs[cache_type][cache_op][cache_result];

Why not:

        attr->config1 = hybrid(event->pmu, 
hw_cache_extra_regs)[cache_type][cache_op][cache_result];

?

Because hw_cache_extra_regs is not part of the struct x86_pmu.


Thanks,
Kan