2022-09-22 20:21:27

by Liang, Kan

[permalink] [raw]
Subject: [PATCH] perf/x86/intel: Hide Topdown metrics events if slots is not enumerated

From: Kan Liang <[email protected]>

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 <[email protected]>
Tested-by: Dongli Zhang <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
---
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 = {
--
2.35.1


2022-10-10 05:22:20

by Dongli Zhang

[permalink] [raw]
Subject: Re: [PATCH] perf/x86/intel: Hide Topdown metrics events if slots is not enumerated

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, [email protected] wrote:
> From: Kan Liang <[email protected]>
>
> 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 <[email protected]>
> Tested-by: Dongli Zhang <[email protected]>
> Signed-off-by: Kan Liang <[email protected]>
> ---
> 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 = {

2022-10-27 16:49:39

by Dongli Zhang

[permalink] [raw]
Subject: Re: [PATCH] perf/x86/intel: Hide Topdown metrics events if slots is not enumerated

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, [email protected] wrote:
>> From: Kan Liang <[email protected]>
>>
>> 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 <[email protected]>
>> Tested-by: Dongli Zhang <[email protected]>
>> Signed-off-by: Kan Liang <[email protected]>
>> ---
>> 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 = {

2023-12-19 13:23:56

by Dongli Zhang

[permalink] [raw]
Subject: Re: [PATCH] perf/x86/intel: Hide Topdown metrics events if slots is not enumerated

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"))

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, [email protected] wrote:
>>> From: Kan Liang <[email protected]>
>>>
>>> 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 <[email protected]>
>>> Tested-by: Dongli Zhang <[email protected]>
>>> Signed-off-by: Kan Liang <[email protected]>
>>> ---
>>> 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 = {

2023-12-19 15:04:31

by Liang, Kan

[permalink] [raw]
Subject: Re: [PATCH] perf/x86/intel: Hide Topdown metrics events if slots is not enumerated



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/[email protected]/

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, [email protected] wrote:
>>>> From: Kan Liang <[email protected]>
>>>>
>>>> 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 <[email protected]>
>>>> Tested-by: Dongli Zhang <[email protected]>
>>>> Signed-off-by: Kan Liang <[email protected]>
>>>> ---
>>>> 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 = {
>