Re: [PATCH V2] perf/x86/intel: Hide Topdown metrics events if the feature is not enumerated

[Dongli Zhang]

Tested-by: Dongli Zhang <dongli.zhang@xxxxxxxxxx>

Thank you very much!

Dongli Zhang

On 12/19/23 07:01, kan.liang@xxxxxxxxxxxxxxx wrote:
> From: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> 
> 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
> perf-metrics feature 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@xxxxxxxxxx>
> Closes: https://lore.kernel.org/lkml/CAM9d7cj8z+ryyzUHR+P1Dcpot2jjW+Qcc4CPQpfafTXN=LEU0Q@xxxxxxxxxxxxxx/
> Signed-off-by: Kan Liang <kan.liang@xxxxxxxxxxxxxxx>
> ---
> 
> Changes since V1:
> - Rebase on top of the 6.7-rc
> - Use the intel_cap.perf_metrics to repace the slots event to check
>   the enumeration of the perf metrics feature
>  
>  arch/x86/events/intel/core.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index e068a96aeb54..e0822ede9ccb 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -5699,8 +5699,22 @@ exra_is_visible(struct kobject *kobj, struct attribute *attr, int i)
>  	return x86_pmu.version >= 2 ? attr->mode : 0;
>  }
>  
> +static umode_t
> +td_is_visible(struct kobject *kobj, struct attribute *attr, int i)
> +{
> +	/*
> +	 * Hide the perf metrics topdown events
> +	 * if the feature is not enumerated.
> +	 */
> +	if (x86_pmu.num_topdown_events)
> +		return x86_pmu.intel_cap.perf_metrics ? attr->mode : 0;
> +
> +	return attr->mode;
> +}
> +
>  static struct attribute_group group_events_td  = {
>  	.name = "events",
> +	.is_visible = td_is_visible,
>  };
>  
>  static struct attribute_group group_events_mem = {
> @@ -5902,9 +5916,27 @@ static umode_t hybrid_format_is_visible(struct kobject *kobj,
>  	return (cpu >= 0) && (pmu->pmu_type & pmu_attr->pmu_type) ? attr->mode : 0;
>  }
>  
> +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 the big core supports perf metrics */
> +	if (pmu->pmu_type == hybrid_big)
> +		return pmu->intel_cap.perf_metrics ? attr->mode : 0;
> +
> +	return attr->mode;
> +}
> +
>  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 = {