Don't just check the raw PMU type, the only core PMU on homogeneous
x86, check raw and all dynamically added PMUs. Extend the
perf_pmu__warn_invalid_config to check all 4 config values. Rather
than process the format list once per event, store the computed masks
for each config value. Don't ignore the mask being zero, which is
likely for config2 and config3, add config_masks_present so config
values can be ignored only when no format information is present.
Signed-off-by: Ian Rogers <[email protected]>
---
tools/perf/util/parse-events.c | 13 +++++++++---
tools/perf/util/pmu.c | 38 ++++++++++++++++++++++++----------
tools/perf/util/pmu.h | 13 +++++++++++-
3 files changed, 49 insertions(+), 15 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 7f047ac11168..9f60607b0d86 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -245,9 +245,16 @@ __add_event(struct list_head *list, int *idx,
if (pmu)
perf_pmu__warn_invalid_formats(pmu);
- if (pmu && attr->type == PERF_TYPE_RAW)
- perf_pmu__warn_invalid_config(pmu, attr->config, name);
-
+ if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
+ perf_pmu__warn_invalid_config(pmu, attr->config, name,
+ PERF_PMU_FORMAT_VALUE_CONFIG, "config");
+ perf_pmu__warn_invalid_config(pmu, attr->config1, name,
+ PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
+ perf_pmu__warn_invalid_config(pmu, attr->config2, name,
+ PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
+ perf_pmu__warn_invalid_config(pmu, attr->config3, name,
+ PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
+ }
if (init_attr)
event_attr_init(attr);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 204ce3f02e63..b0443406fd57 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1611,37 +1611,53 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
return pmu->nr_caps;
}
-void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
- const char *name)
+static void perf_pmu__compute_config_masks(struct perf_pmu *pmu)
{
struct perf_pmu_format *format;
- __u64 masks = 0, bits;
- char buf[100];
- unsigned int i;
+
+ if (pmu->config_masks_computed)
+ return;
list_for_each_entry(format, &pmu->format, list) {
- if (format->value != PERF_PMU_FORMAT_VALUE_CONFIG)
+ unsigned int i;
+ __u64 *mask;
+
+ if (format->value >= PERF_PMU_FORMAT_VALUE_CONFIG_END)
continue;
+ pmu->config_masks_present = true;
+ mask = &pmu->config_masks[format->value];
+
for_each_set_bit(i, format->bits, PERF_PMU_FORMAT_BITS)
- masks |= 1ULL << i;
+ *mask |= 1ULL << i;
}
+ pmu->config_masks_computed = true;
+}
+
+void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
+ const char *name, int config_num,
+ const char *config_name)
+{
+ __u64 bits;
+ char buf[100];
+
+ perf_pmu__compute_config_masks(pmu);
/*
* Kernel doesn't export any valid format bits.
*/
- if (masks == 0)
+ if (!pmu->config_masks_present)
return;
- bits = config & ~masks;
+ bits = config & ~pmu->config_masks[config_num];
if (bits == 0)
return;
bitmap_scnprintf((unsigned long *)&bits, sizeof(bits) * 8, buf, sizeof(buf));
- pr_warning("WARNING: event '%s' not valid (bits %s of config "
+ pr_warning("WARNING: event '%s' not valid (bits %s of %s "
"'%llx' not supported by kernel)!\n",
- name ?: "N/A", buf, config);
+ name ?: "N/A", buf, config_name, config);
}
int perf_pmu__match(char *pattern, char *name, char *tok)
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 7a1535dc1f12..d98b0feec022 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -81,6 +81,10 @@ struct perf_pmu {
* perf_event_attr once.
*/
bool formats_checked;
+ /** @config_masks_present: Are there config format values? */
+ bool config_masks_present;
+ /** @config_masks_computed: Set when masks are lazily computed. */
+ bool config_masks_computed;
/**
* @max_precise: Number of levels of :ppp precision supported by the
* PMU, read from
@@ -125,6 +129,12 @@ struct perf_pmu {
/** @list: Element on pmus list in pmu.c. */
struct list_head list;
+ /**
+ * @config_masks: Derived from the PMU's format data, bits that are
+ * valid within the config value.
+ */
+ __u64 config_masks[PERF_PMU_FORMAT_VALUE_CONFIG_END];
+
/**
* @missing_features: Features to inhibit when events on this PMU are
* opened.
@@ -255,7 +265,8 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
int perf_pmu__caps_parse(struct perf_pmu *pmu);
void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
- const char *name);
+ const char *name, int config_num,
+ const char *config_name);
void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
int perf_pmu__match(char *pattern, char *name, char *tok);
--
2.41.0.rc0.172.g3f132b7071-goog
On Wed, May 31, 2023 at 7:36 PM Ian Rogers <[email protected]> wrote:
>
> Don't just check the raw PMU type, the only core PMU on homogeneous
> x86, check raw and all dynamically added PMUs. Extend the
> perf_pmu__warn_invalid_config to check all 4 config values. Rather
> than process the format list once per event, store the computed masks
> for each config value. Don't ignore the mask being zero, which is
> likely for config2 and config3, add config_masks_present so config
> values can be ignored only when no format information is present.
>
> Signed-off-by: Ian Rogers <[email protected]>
Acked-by: Namhyung Kim <[email protected]>
Thanks,
Namhyung
> ---
> tools/perf/util/parse-events.c | 13 +++++++++---
> tools/perf/util/pmu.c | 38 ++++++++++++++++++++++++----------
> tools/perf/util/pmu.h | 13 +++++++++++-
> 3 files changed, 49 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index 7f047ac11168..9f60607b0d86 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -245,9 +245,16 @@ __add_event(struct list_head *list, int *idx,
> if (pmu)
> perf_pmu__warn_invalid_formats(pmu);
>
> - if (pmu && attr->type == PERF_TYPE_RAW)
> - perf_pmu__warn_invalid_config(pmu, attr->config, name);
> -
> + if (pmu && (attr->type == PERF_TYPE_RAW || attr->type >= PERF_TYPE_MAX)) {
> + perf_pmu__warn_invalid_config(pmu, attr->config, name,
> + PERF_PMU_FORMAT_VALUE_CONFIG, "config");
> + perf_pmu__warn_invalid_config(pmu, attr->config1, name,
> + PERF_PMU_FORMAT_VALUE_CONFIG1, "config1");
> + perf_pmu__warn_invalid_config(pmu, attr->config2, name,
> + PERF_PMU_FORMAT_VALUE_CONFIG2, "config2");
> + perf_pmu__warn_invalid_config(pmu, attr->config3, name,
> + PERF_PMU_FORMAT_VALUE_CONFIG3, "config3");
> + }
> if (init_attr)
> event_attr_init(attr);
>
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 204ce3f02e63..b0443406fd57 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1611,37 +1611,53 @@ int perf_pmu__caps_parse(struct perf_pmu *pmu)
> return pmu->nr_caps;
> }
>
> -void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> - const char *name)
> +static void perf_pmu__compute_config_masks(struct perf_pmu *pmu)
> {
> struct perf_pmu_format *format;
> - __u64 masks = 0, bits;
> - char buf[100];
> - unsigned int i;
> +
> + if (pmu->config_masks_computed)
> + return;
>
> list_for_each_entry(format, &pmu->format, list) {
> - if (format->value != PERF_PMU_FORMAT_VALUE_CONFIG)
> + unsigned int i;
> + __u64 *mask;
> +
> + if (format->value >= PERF_PMU_FORMAT_VALUE_CONFIG_END)
> continue;
>
> + pmu->config_masks_present = true;
> + mask = &pmu->config_masks[format->value];
> +
> for_each_set_bit(i, format->bits, PERF_PMU_FORMAT_BITS)
> - masks |= 1ULL << i;
> + *mask |= 1ULL << i;
> }
> + pmu->config_masks_computed = true;
> +}
> +
> +void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> + const char *name, int config_num,
> + const char *config_name)
> +{
> + __u64 bits;
> + char buf[100];
> +
> + perf_pmu__compute_config_masks(pmu);
>
> /*
> * Kernel doesn't export any valid format bits.
> */
> - if (masks == 0)
> + if (!pmu->config_masks_present)
> return;
>
> - bits = config & ~masks;
> + bits = config & ~pmu->config_masks[config_num];
> if (bits == 0)
> return;
>
> bitmap_scnprintf((unsigned long *)&bits, sizeof(bits) * 8, buf, sizeof(buf));
>
> - pr_warning("WARNING: event '%s' not valid (bits %s of config "
> + pr_warning("WARNING: event '%s' not valid (bits %s of %s "
> "'%llx' not supported by kernel)!\n",
> - name ?: "N/A", buf, config);
> + name ?: "N/A", buf, config_name, config);
> }
>
> int perf_pmu__match(char *pattern, char *name, char *tok)
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 7a1535dc1f12..d98b0feec022 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -81,6 +81,10 @@ struct perf_pmu {
> * perf_event_attr once.
> */
> bool formats_checked;
> + /** @config_masks_present: Are there config format values? */
> + bool config_masks_present;
> + /** @config_masks_computed: Set when masks are lazily computed. */
> + bool config_masks_computed;
> /**
> * @max_precise: Number of levels of :ppp precision supported by the
> * PMU, read from
> @@ -125,6 +129,12 @@ struct perf_pmu {
> /** @list: Element on pmus list in pmu.c. */
> struct list_head list;
>
> + /**
> + * @config_masks: Derived from the PMU's format data, bits that are
> + * valid within the config value.
> + */
> + __u64 config_masks[PERF_PMU_FORMAT_VALUE_CONFIG_END];
> +
> /**
> * @missing_features: Features to inhibit when events on this PMU are
> * opened.
> @@ -255,7 +265,8 @@ int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
> int perf_pmu__caps_parse(struct perf_pmu *pmu);
>
> void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
> - const char *name);
> + const char *name, int config_num,
> + const char *config_name);
> void perf_pmu__warn_invalid_formats(struct perf_pmu *pmu);
>
> int perf_pmu__match(char *pattern, char *name, char *tok);
> --
> 2.41.0.rc0.172.g3f132b7071-goog
>