Skip to content

Commit

Permalink
perf/x86/intel: Clean up the hybrid CPU type handling code
Browse files Browse the repository at this point in the history
There is a fairly long list of grievances about the current code. The
main beefs:

   1. hybrid_big_small assumes that the *HARDWARE* (CPUID) provided
      core types are a bitmap. They are not. If Intel happened to
      make a core type of 0xff, hilarity would ensue.
   2. adl_get_hybrid_cpu_type() utterly inscrutable.  There are
      precisely zero comments and zero changelog about what it is
      attempting to do.

According to Kan, the adl_get_hybrid_cpu_type() is there because some
Alder Lake (ADL) CPUs can do some silly things. Some ADL models are
*supposed* to be hybrid CPUs with big and little cores, but there are
some SKUs that only have big cores. CPUID(0x1a) on those CPUs does
not say that the CPUs are big cores. It apparently just returns 0x0.
It confuses perf because it expects to see either 0x40 (Core) or
0x20 (Atom).

The perf workaround for this is to watch for a CPU core saying it is
type 0x0. If that happens on an Alder Lake, it calls
x86_pmu.get_hybrid_cpu_type() and just assumes that the core is a
Core (0x40) CPU.

To fix up the mess, separate out the CPU types and the 'pmu' types.
This allows 'hybrid_pmu_type' bitmaps without worrying that some
future CPU type will set multiple bits.

Since the types are now separate, add a function to glue them back
together again. Actual comment on the situation in the glue
function (find_hybrid_pmu_for_cpu()).

Also, give ->get_hybrid_cpu_type() a real return type and make it
clear that it is overriding the *CPU* type, not the PMU type.

Rename cpu_type to pmu_type in the struct x86_hybrid_pmu to reflect the
change.

Originally-by: Dave Hansen <dave.hansen@linux.intel.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20230829125806.3016082-6-kan.liang@linux.intel.com
  • Loading branch information
Kan Liang authored and Ingo Molnar committed Aug 29, 2023
1 parent 299a5fc commit b0560bf
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 40 deletions.
6 changes: 3 additions & 3 deletions arch/x86/events/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -1887,9 +1887,9 @@ ssize_t events_hybrid_sysfs_show(struct device *dev,

str = pmu_attr->event_str;
for (i = 0; i < x86_pmu.num_hybrid_pmus; i++) {
if (!(x86_pmu.hybrid_pmu[i].cpu_type & pmu_attr->pmu_type))
if (!(x86_pmu.hybrid_pmu[i].pmu_type & pmu_attr->pmu_type))
continue;
if (x86_pmu.hybrid_pmu[i].cpu_type & pmu->cpu_type) {
if (x86_pmu.hybrid_pmu[i].pmu_type & pmu->pmu_type) {
next_str = strchr(str, ';');
if (next_str)
return snprintf(page, next_str - str + 1, "%s", str);
Expand Down Expand Up @@ -2169,7 +2169,7 @@ static int __init init_hw_perf_events(void)
hybrid_pmu->pmu.capabilities |= PERF_PMU_CAP_EXTENDED_HW_TYPE;

err = perf_pmu_register(&hybrid_pmu->pmu, hybrid_pmu->name,
(hybrid_pmu->cpu_type == hybrid_big) ? PERF_TYPE_RAW : -1);
(hybrid_pmu->pmu_type == hybrid_big) ? PERF_TYPE_RAW : -1);
if (err)
break;
}
Expand Down
69 changes: 47 additions & 22 deletions arch/x86/events/intel/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -3849,7 +3849,7 @@ static inline bool require_mem_loads_aux_event(struct perf_event *event)
return false;

if (is_hybrid())
return hybrid_pmu(event->pmu)->cpu_type == hybrid_big;
return hybrid_pmu(event->pmu)->pmu_type == hybrid_big;

return true;
}
Expand Down Expand Up @@ -4341,9 +4341,9 @@ adl_get_event_constraints(struct cpu_hw_events *cpuc, int idx,
{
struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);

if (pmu->cpu_type == hybrid_big)
if (pmu->pmu_type == hybrid_big)
return glc_get_event_constraints(cpuc, idx, event);
else if (pmu->cpu_type == hybrid_small)
else if (pmu->pmu_type == hybrid_small)
return tnt_get_event_constraints(cpuc, idx, event);

WARN_ON(1);
Expand Down Expand Up @@ -4413,9 +4413,9 @@ mtl_get_event_constraints(struct cpu_hw_events *cpuc, int idx,
{
struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);

if (pmu->cpu_type == hybrid_big)
if (pmu->pmu_type == hybrid_big)
return rwc_get_event_constraints(cpuc, idx, event);
if (pmu->cpu_type == hybrid_small)
if (pmu->pmu_type == hybrid_small)
return cmt_get_event_constraints(cpuc, idx, event);

WARN_ON(1);
Expand All @@ -4426,18 +4426,18 @@ static int adl_hw_config(struct perf_event *event)
{
struct x86_hybrid_pmu *pmu = hybrid_pmu(event->pmu);

if (pmu->cpu_type == hybrid_big)
if (pmu->pmu_type == hybrid_big)
return hsw_hw_config(event);
else if (pmu->cpu_type == hybrid_small)
else if (pmu->pmu_type == hybrid_small)
return intel_pmu_hw_config(event);

WARN_ON(1);
return -EOPNOTSUPP;
}

static u8 adl_get_hybrid_cpu_type(void)
static enum hybrid_cpu_type adl_get_hybrid_cpu_type(void)
{
return hybrid_big;
return HYBRID_INTEL_CORE;
}

/*
Expand Down Expand Up @@ -4613,22 +4613,47 @@ static void update_pmu_cap(struct x86_hybrid_pmu *pmu)
}
}

static bool init_hybrid_pmu(int cpu)
static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
{
struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu);
u8 cpu_type = get_this_hybrid_cpu_type();
struct x86_hybrid_pmu *pmu = NULL;
int i;

if (!cpu_type && x86_pmu.get_hybrid_cpu_type)
cpu_type = x86_pmu.get_hybrid_cpu_type();
/*
* This is running on a CPU model that is known to have hybrid
* configurations. But the CPU told us it is not hybrid, shame
* on it. There should be a fixup function provided for these
* troublesome CPUs (->get_hybrid_cpu_type).
*/
if (cpu_type == HYBRID_INTEL_NONE) {
if (x86_pmu.get_hybrid_cpu_type)
cpu_type = x86_pmu.get_hybrid_cpu_type();
else
return NULL;
}

/*
* This essentially just maps between the 'hybrid_cpu_type'
* and 'hybrid_pmu_type' enums:
*/
for (i = 0; i < x86_pmu.num_hybrid_pmus; i++) {
if (x86_pmu.hybrid_pmu[i].cpu_type == cpu_type) {
pmu = &x86_pmu.hybrid_pmu[i];
break;
}
enum hybrid_pmu_type pmu_type = x86_pmu.hybrid_pmu[i].pmu_type;

if (cpu_type == HYBRID_INTEL_CORE &&
pmu_type == hybrid_big)
return &x86_pmu.hybrid_pmu[i];
if (cpu_type == HYBRID_INTEL_ATOM &&
pmu_type == hybrid_small)
return &x86_pmu.hybrid_pmu[i];
}

return NULL;
}

static bool init_hybrid_pmu(int cpu)
{
struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu);
struct x86_hybrid_pmu *pmu = find_hybrid_pmu_for_cpu();

if (WARN_ON_ONCE(!pmu || (pmu->pmu.type == -1))) {
cpuc->pmu = NULL;
return false;
Expand Down Expand Up @@ -5679,7 +5704,7 @@ static bool is_attr_for_this_pmu(struct kobject *kobj, struct attribute *attr)
struct perf_pmu_events_hybrid_attr *pmu_attr =
container_of(attr, struct perf_pmu_events_hybrid_attr, attr.attr);

return pmu->cpu_type & pmu_attr->pmu_type;
return pmu->pmu_type & pmu_attr->pmu_type;
}

static umode_t hybrid_events_is_visible(struct kobject *kobj,
Expand Down Expand Up @@ -5716,7 +5741,7 @@ static umode_t hybrid_format_is_visible(struct kobject *kobj,
container_of(attr, struct perf_pmu_format_hybrid_attr, attr.attr);
int cpu = hybrid_find_supported_cpu(pmu);

return (cpu >= 0) && (pmu->cpu_type & pmu_attr->pmu_type) ? attr->mode : 0;
return (cpu >= 0) && (pmu->pmu_type & pmu_attr->pmu_type) ? attr->mode : 0;
}

static struct attribute_group hybrid_group_events_td = {
Expand Down Expand Up @@ -6607,7 +6632,7 @@ __init int intel_pmu_init(void)
/* Initialize big core specific PerfMon capabilities.*/
pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_CORE_IDX];
pmu->name = "cpu_core";
pmu->cpu_type = hybrid_big;
pmu->pmu_type = hybrid_big;
intel_pmu_init_glc(&pmu->pmu);
pmu->late_ack = true;
if (cpu_feature_enabled(X86_FEATURE_HYBRID_CPU)) {
Expand Down Expand Up @@ -6643,7 +6668,7 @@ __init int intel_pmu_init(void)
/* Initialize Atom core specific PerfMon capabilities.*/
pmu = &x86_pmu.hybrid_pmu[X86_HYBRID_PMU_ATOM_IDX];
pmu->name = "cpu_atom";
pmu->cpu_type = hybrid_small;
pmu->pmu_type = hybrid_small;
intel_pmu_init_grt(&pmu->pmu);
pmu->mid_ack = true;
pmu->num_counters = x86_pmu.num_counters;
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/events/intel/ds.c
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ static u64 __adl_latency_data_small(struct perf_event *event, u64 status,
{
u64 val;

WARN_ON_ONCE(hybrid_pmu(event->pmu)->cpu_type == hybrid_big);
WARN_ON_ONCE(hybrid_pmu(event->pmu)->pmu_type == hybrid_big);

dse &= PERF_PEBS_DATA_SOURCE_MASK;
val = hybrid_var(event->pmu, pebs_data_source)[dse];
Expand Down
35 changes: 21 additions & 14 deletions arch/x86/events/perf_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -652,10 +652,29 @@ enum {
#define PERF_PEBS_DATA_SOURCE_MAX 0x10
#define PERF_PEBS_DATA_SOURCE_MASK (PERF_PEBS_DATA_SOURCE_MAX - 1)

enum hybrid_cpu_type {
HYBRID_INTEL_NONE,
HYBRID_INTEL_ATOM = 0x20,
HYBRID_INTEL_CORE = 0x40,
};

enum hybrid_pmu_type {
not_hybrid,
hybrid_small = BIT(0),
hybrid_big = BIT(1),

hybrid_big_small = hybrid_big | hybrid_small, /* only used for matching */
};

#define X86_HYBRID_PMU_ATOM_IDX 0
#define X86_HYBRID_PMU_CORE_IDX 1

#define X86_HYBRID_NUM_PMUS 2

struct x86_hybrid_pmu {
struct pmu pmu;
const char *name;
u8 cpu_type;
enum hybrid_pmu_type pmu_type;
cpumask_t supported_cpus;
union perf_capabilities intel_cap;
u64 intel_ctrl;
Expand Down Expand Up @@ -721,18 +740,6 @@ extern struct static_key_false perf_is_hybrid;
__Fp; \
})

enum hybrid_pmu_type {
hybrid_big = 0x40,
hybrid_small = 0x20,

hybrid_big_small = hybrid_big | hybrid_small,
};

#define X86_HYBRID_PMU_ATOM_IDX 0
#define X86_HYBRID_PMU_CORE_IDX 1

#define X86_HYBRID_NUM_PMUS 2

/*
* struct x86_pmu - generic x86 pmu
*/
Expand Down Expand Up @@ -940,7 +947,7 @@ struct x86_pmu {
*/
int num_hybrid_pmus;
struct x86_hybrid_pmu *hybrid_pmu;
u8 (*get_hybrid_cpu_type) (void);
enum hybrid_cpu_type (*get_hybrid_cpu_type) (void);
};

struct x86_perf_task_context_opt {
Expand Down

0 comments on commit b0560bf

Please sign in to comment.