Messages in this thread | | | Date | Tue, 19 Dec 2023 10:04:16 -0500 | Subject | Re: [PATCH] perf/x86/intel: Hide Topdown metrics events if slots is not enumerated | From | "Liang, Kan" <> |
| |
On 2023-12-19 8:22 a.m., Dongli Zhang wrote: > I can still reproduce this issue with the most recent mainline linux kernel (at > least when the KVM is not the most recent). > > Any chance to have this patch in the linux kernel? > > (We may need to rename 'cpu_type' due to commit b0560bfd4b70 ("perf/x86/intel: > Clean up the hybrid CPU type handling code"))
I have re-based the patch on top of the latest 6.7-rc. https://lore.kernel.org/lkml/20231219150109.1596634-1-kan.liang@linux.intel.com/
Besides the 'cpu_type', I also use the intel_cap.perf_metrics to do the check, which should be more accurate than the slots event.
Please give it a try and let us know if the V2 works for you.
Thanks, Kan > > Thank you very much! > > Dongli Zhang > > On 10/27/22 09:17, Dongli Zhang wrote: >> Ping? Any plan for this patch? Currently "perf stat" will fail on Icelake VMs >> (without the topdown metric). The user will need to manually specify the events >> to trace. >> >> Thank you very much! >> >> Dongli Zhang >> >> On 10/9/22 10:03 PM, Dongli Zhang wrote: >>> Ping? >>> >>> Currently the default "perf stat" may fail on all Icelake KVM VMs. >>> >>> Thank you very much! >>> >>> Dongli Zhang >>> >>> On 9/22/22 13:15, kan.liang@linux.intel.com wrote: >>>> From: Kan Liang <kan.liang@linux.intel.com> >>>> >>>> The below error is observed on Ice Lake VM. >>>> >>>> $ perf stat >>>> Error: >>>> The sys_perf_event_open() syscall returned with 22 (Invalid argument) >>>> for event (slots). >>>> /bin/dmesg | grep -i perf may provide additional information. >>>> >>>> In a virtualization env, the Topdown metrics and the slots event haven't >>>> been supported yet. The guest CPUID doesn't enumerate them. However, the >>>> current kernel unconditionally exposes the slots event and the Topdown >>>> metrics events to sysfs, which misleads the perf tool and triggers the >>>> error. >>>> >>>> Hide the perf metrics topdown events and the slots event if the slots >>>> event is not enumerated. >>>> >>>> The big core of a hybrid platform can also supports the perf-metrics >>>> feature. Fix the hybrid platform as well. >>>> >>>> Reported-by: Dongli Zhang <dongli.zhang@oracle.com> >>>> Tested-by: Dongli Zhang <dongli.zhang@oracle.com> >>>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> >>>> --- >>>> arch/x86/events/intel/core.c | 33 ++++++++++++++++++++++++++++++++- >>>> 1 file changed, 32 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c >>>> index b16c91ac9219..a0a62b67c440 100644 >>>> --- a/arch/x86/events/intel/core.c >>>> +++ b/arch/x86/events/intel/core.c >>>> @@ -5335,6 +5335,19 @@ static struct attribute *intel_pmu_attrs[] = { >>>> NULL, >>>> }; >>>> >>>> +static umode_t >>>> +td_is_visible(struct kobject *kobj, struct attribute *attr, int i) >>>> +{ >>>> + /* >>>> + * Hide the perf metrics topdown events >>>> + * if the slots is not enumerated. >>>> + */ >>>> + if (x86_pmu.num_topdown_events) >>>> + return (x86_pmu.intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0; >>>> + >>>> + return attr->mode; >>>> +} >>>> + >>>> static umode_t >>>> tsx_is_visible(struct kobject *kobj, struct attribute *attr, int i) >>>> { >>>> @@ -5370,6 +5383,7 @@ default_is_visible(struct kobject *kobj, struct attribute *attr, int i) >>>> >>>> static struct attribute_group group_events_td = { >>>> .name = "events", >>>> + .is_visible = td_is_visible, >>>> }; >>>> >>>> static struct attribute_group group_events_mem = { >>>> @@ -5522,6 +5536,23 @@ static inline int hybrid_find_supported_cpu(struct x86_hybrid_pmu *pmu) >>>> return (cpu >= nr_cpu_ids) ? -1 : cpu; >>>> } >>>> >>>> +static umode_t hybrid_td_is_visible(struct kobject *kobj, >>>> + struct attribute *attr, int i) >>>> +{ >>>> + struct device *dev = kobj_to_dev(kobj); >>>> + struct x86_hybrid_pmu *pmu = >>>> + container_of(dev_get_drvdata(dev), struct x86_hybrid_pmu, pmu); >>>> + >>>> + if (!is_attr_for_this_pmu(kobj, attr)) >>>> + return 0; >>>> + >>>> + /* Only check the big core which supports perf metrics */ >>>> + if (pmu->cpu_type == hybrid_big) >>>> + return (pmu->intel_ctrl & INTEL_PMC_MSK_FIXED_SLOTS) ? attr->mode : 0; >>>> + >>>> + return attr->mode; >>>> +} >>>> + >>>> static umode_t hybrid_tsx_is_visible(struct kobject *kobj, >>>> struct attribute *attr, int i) >>>> { >>>> @@ -5548,7 +5579,7 @@ static umode_t hybrid_format_is_visible(struct kobject *kobj, >>>> >>>> static struct attribute_group hybrid_group_events_td = { >>>> .name = "events", >>>> - .is_visible = hybrid_events_is_visible, >>>> + .is_visible = hybrid_td_is_visible, >>>> }; >>>> >>>> static struct attribute_group hybrid_group_events_mem = { >
| |