From nobody Mon Oct 6 17:06:25 2025 Received: from mail-pj1-f74.google.com (mail-pj1-f74.google.com [209.85.216.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 346642139C9 for ; Sat, 19 Jul 2025 03:05:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.74 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752894359; cv=none; b=FZ1aaXD366mI84LQIzuJ26GK+rLFSWQnv3RoX0P4gCE0a8V5iSaNlajASlk+sB6uxaC3deCYYGbhBW4/0x2cNt0D5PxpFdzT3vxNO/9Rcl1M4gglxLJxa+XdbVEi8G4pF0p6qUWHfzfpmtiI/laSAd9cMkAcfEI4g/JcKeyZ7o4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752894359; c=relaxed/simple; bh=QX1ZFZoy77kXJVY+shs+MAXOEsT89Rxp1CS196e90wA=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Content-Type; b=Yvifo4KbqSfzw9gEi91OD8ZYzFzYnMxe74ZRRxZVjB+gtEX+wpty+xWzCajLUbqB3RN/0tMGI6poLFrtOjOW70a03yl15WUSr0TLPJd3lNFRFOO42gqPPE16rEC8d8CdJX5x/e5MxKUX/jxiwlIaewF1IyP4xPobKk9Ev4ux+20= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=dVbEUPn8; arc=none smtp.client-ip=209.85.216.74 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--irogers.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="dVbEUPn8" Received: by mail-pj1-f74.google.com with SMTP id 98e67ed59e1d1-3122368d82bso3408176a91.0 for ; Fri, 18 Jul 2025 20:05:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1752894357; x=1753499157; darn=vger.kernel.org; h=to:from:subject:message-id:references:mime-version:in-reply-to:date :from:to:cc:subject:date:message-id:reply-to; bh=fsxL8L8W7y9gnvueti74PHIwOUalGfFDynZg+KuuTMM=; b=dVbEUPn8cW1y+B1Zs6pwNL/NrQVdcs2o2+hYDs7zWsprFFrneHZQKnKxI5b5QAc/Ny JwuvV5A1bwo0+TmmKTCrXbYRYuRKbcT8nj6xYiUvCYWpJn6hwUGUeqvC/cMEQpaAq5yi 540Mj66AJSRuxtS2d0Ax+VgpLveTcLOkcNI7WMnlYqu6i/QkNvFqhnGBvYoa4PDBlQKF SIEropqi8LA1tBVAw3jqHPCDqQQALjMMIf8J15wGWonwrYgYqaIDglnyWMgregw9knF5 22XylNN/dAEc0o/C1DdXdKnT/QUepMbG5JA2mymVlRmwFJNNfDme6M/EDI2EVv9Yruhw 8g3Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1752894357; x=1753499157; h=to:from:subject:message-id:references:mime-version:in-reply-to:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=fsxL8L8W7y9gnvueti74PHIwOUalGfFDynZg+KuuTMM=; b=YgWbicCI2bG0CFZF6quSUVxgbBQhc7N0RngnXLucrykCTVYrdee7R9GL7MYEJTkrk3 wcuCPnw2lIVS3aOhtF2hHar6LwvyOn9pxUKjUfqwAOi5Djlx7sJBE0jCN50+K72CX0mI p8sQPgD+Si9JMAEYCeRfd3qqJIuCOKg8uppMBPn4RVztv+MhZPmg24SbcqcAoMDtpXjJ q6RdTSEltyfp/MhKy18FjalODWhbqsS9SGKx1YdPS02UQkG4aZrxlcxUi6OhroMidjsx aE590TMBshSbeDDFvr1nvttbNVXZOYBCkoWk5c+QxpOATxTE35YVlcD3k8yoNe41RTwY QaFQ== X-Forwarded-Encrypted: i=1; AJvYcCV//N+liQaQCqqHDMw6XmHfzWrWqZWBjY7rhrdUKgCcAh5jKpSTrV6QKLHrh32H7ONQdzXm8lFnBjYZweg=@vger.kernel.org X-Gm-Message-State: AOJu0Yz457Bi4r4H7trQDNhyAbDaasZYuqfv9mnL8blToueLFLMtKQn2 hdxdAiMnAw53NAYahbYWFQ5N08i3ihwCu1dakf/92R/RESuIBiwozTMK7YoAFZdAW1sltuTBMFM vQztzITYLMA== X-Google-Smtp-Source: AGHT+IGb8aCsnBZ7OL/AttuP80279VXoqk9vMo41Oz3GuEun0Q8Ghgt7v5m0wHiqWt+NLJm3ykTL51XIhEVe X-Received: from pjbpm18.prod.google.com ([2002:a17:90b:3c52:b0:314:626:7b97]) (user=irogers job=prod-delivery.src-stubby-dispatcher) by 2002:a17:90b:270b:b0:311:ab20:159a with SMTP id 98e67ed59e1d1-31c9f47ce74mr17738254a91.29.1752894357573; Fri, 18 Jul 2025 20:05:57 -0700 (PDT) Date: Fri, 18 Jul 2025 20:05:15 -0700 In-Reply-To: <20250719030517.1990983-1-irogers@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20250719030517.1990983-1-irogers@google.com> X-Mailer: git-send-email 2.50.0.727.gbf7dc18ff4-goog Message-ID: <20250719030517.1990983-14-irogers@google.com> Subject: [PATCH v3 13/15] perf topdown: Use attribute to see an event is a topdown metic or slots From: Ian Rogers To: Thomas Falcon , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , "Liang, Kan" , Ravi Bangoria , James Clark , Dapeng Mi , Weilin Wang , Andi Kleen , linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" The string comparisons were overly broad and could fire for the incorrect PMU and events. Switch to using the config in the attribute then add a perf test to confirm the attribute config values match those of parsed events of that name and don't match others. This exposed matches for slots events that shouldn't have matched as the slots fixed counter event, such as topdown.slots_p. Fixes: fbc798316bef ("perf x86/topdown: Refine helper arch_is_topdown_metri= cs()") Signed-off-by: Ian Rogers --- v2: In test rename topdown_pmu to p_core_pmu for clarity. --- tools/perf/arch/x86/include/arch-tests.h | 4 ++ tools/perf/arch/x86/tests/Build | 1 + tools/perf/arch/x86/tests/arch-tests.c | 1 + tools/perf/arch/x86/tests/topdown.c | 76 ++++++++++++++++++++++++ tools/perf/arch/x86/util/evsel.c | 46 ++++---------- tools/perf/arch/x86/util/topdown.c | 31 ++++------ tools/perf/arch/x86/util/topdown.h | 4 ++ 7 files changed, 108 insertions(+), 55 deletions(-) create mode 100644 tools/perf/arch/x86/tests/topdown.c diff --git a/tools/perf/arch/x86/include/arch-tests.h b/tools/perf/arch/x86= /include/arch-tests.h index 4fd425157d7d..8713e9122d4c 100644 --- a/tools/perf/arch/x86/include/arch-tests.h +++ b/tools/perf/arch/x86/include/arch-tests.h @@ -2,6 +2,8 @@ #ifndef ARCH_TESTS_H #define ARCH_TESTS_H =20 +#include "tests/tests.h" + struct test_suite; =20 /* Tests */ @@ -17,6 +19,8 @@ int test__amd_ibs_via_core_pmu(struct test_suite *test, i= nt subtest); int test__amd_ibs_period(struct test_suite *test, int subtest); int test__hybrid(struct test_suite *test, int subtest); =20 +DECLARE_SUITE(x86_topdown); + extern struct test_suite *arch_tests[]; =20 #endif diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Bu= ild index 01d5527f38c7..311b6b53d3d8 100644 --- a/tools/perf/arch/x86/tests/Build +++ b/tools/perf/arch/x86/tests/Build @@ -11,6 +11,7 @@ endif perf-test-$(CONFIG_X86_64) +=3D bp-modify.o perf-test-y +=3D amd-ibs-via-core-pmu.o perf-test-y +=3D amd-ibs-period.o +perf-test-y +=3D topdown.o =20 ifdef SHELLCHECK SHELL_TESTS :=3D gen-insn-x86-dat.sh diff --git a/tools/perf/arch/x86/tests/arch-tests.c b/tools/perf/arch/x86/t= ests/arch-tests.c index bfee2432515b..29ec1861ccef 100644 --- a/tools/perf/arch/x86/tests/arch-tests.c +++ b/tools/perf/arch/x86/tests/arch-tests.c @@ -53,5 +53,6 @@ struct test_suite *arch_tests[] =3D { &suite__amd_ibs_via_core_pmu, &suite__amd_ibs_period, &suite__hybrid, + &suite__x86_topdown, NULL, }; diff --git a/tools/perf/arch/x86/tests/topdown.c b/tools/perf/arch/x86/test= s/topdown.c new file mode 100644 index 000000000000..8d0ea7a4bbc1 --- /dev/null +++ b/tools/perf/arch/x86/tests/topdown.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "arch-tests.h" +#include "../util/topdown.h" +#include "evlist.h" +#include "parse-events.h" +#include "pmu.h" +#include "pmus.h" + +static int event_cb(void *state, struct pmu_event_info *info) +{ + char buf[256]; + struct parse_events_error parse_err; + int *ret =3D state, err; + struct evlist *evlist =3D evlist__new(); + struct evsel *evsel; + + if (!evlist) + return -ENOMEM; + + parse_events_error__init(&parse_err); + snprintf(buf, sizeof(buf), "%s/%s/", info->pmu->name, info->name); + err =3D parse_events(evlist, buf, &parse_err); + if (err) { + parse_events_error__print(&parse_err, buf); + *ret =3D TEST_FAIL; + } + parse_events_error__exit(&parse_err); + evlist__for_each_entry(evlist, evsel) { + bool fail =3D false; + bool p_core_pmu =3D evsel->pmu->type =3D=3D PERF_TYPE_RAW; + const char *name =3D evsel__name(evsel); + + if (strcasestr(name, "uops_retired.slots") || + strcasestr(name, "topdown.backend_bound_slots") || + strcasestr(name, "topdown.br_mispredict_slots") || + strcasestr(name, "topdown.memory_bound_slots") || + strcasestr(name, "topdown.bad_spec_slots") || + strcasestr(name, "topdown.slots_p")) { + if (arch_is_topdown_slots(evsel) || arch_is_topdown_metrics(evsel)) + fail =3D true; + } else if (strcasestr(name, "slots")) { + if (arch_is_topdown_slots(evsel) !=3D p_core_pmu || + arch_is_topdown_metrics(evsel)) + fail =3D true; + } else if (strcasestr(name, "topdown")) { + if (arch_is_topdown_slots(evsel) || + arch_is_topdown_metrics(evsel) !=3D p_core_pmu) + fail =3D true; + } else if (arch_is_topdown_slots(evsel) || arch_is_topdown_metrics(evsel= )) { + fail =3D true; + } + if (fail) { + pr_debug("Broken topdown information for '%s'\n", evsel__name(evsel)); + *ret =3D TEST_FAIL; + } + } + evlist__delete(evlist); + return 0; +} + +static int test__x86_topdown(struct test_suite *test __maybe_unused, int s= ubtest __maybe_unused) +{ + int ret =3D TEST_OK; + struct perf_pmu *pmu =3D NULL; + + if (!topdown_sys_has_perf_metrics()) + return TEST_OK; + + while ((pmu =3D perf_pmus__scan_core(pmu)) !=3D NULL) { + if (perf_pmu__for_each_event(pmu, /*skip_duplicate_pmus=3D*/false, &ret,= event_cb)) + break; + } + return ret; +} + +DEFINE_SUITE("x86 topdown", x86_topdown); diff --git a/tools/perf/arch/x86/util/evsel.c b/tools/perf/arch/x86/util/ev= sel.c index 3dd29ba2c23b..9bc80fff3aa0 100644 --- a/tools/perf/arch/x86/util/evsel.c +++ b/tools/perf/arch/x86/util/evsel.c @@ -23,47 +23,25 @@ void arch_evsel__set_sample_weight(struct evsel *evsel) bool evsel__sys_has_perf_metrics(const struct evsel *evsel) { struct perf_pmu *pmu; - u32 type =3D evsel->core.attr.type; =20 - /* - * The PERF_TYPE_RAW type is the core PMU type, e.g., "cpu" PMU - * on a non-hybrid machine, "cpu_core" PMU on a hybrid machine. - * The slots event is only available for the core PMU, which - * supports the perf metrics feature. - * Checking both the PERF_TYPE_RAW type and the slots event - * should be good enough to detect the perf metrics feature. - */ -again: - switch (type) { - case PERF_TYPE_HARDWARE: - case PERF_TYPE_HW_CACHE: - type =3D evsel->core.attr.config >> PERF_PMU_TYPE_SHIFT; - if (type) - goto again; - break; - case PERF_TYPE_RAW: - break; - default: + if (!topdown_sys_has_perf_metrics()) return false; - } - - pmu =3D evsel->pmu; - if (pmu && perf_pmu__is_fake(pmu)) - pmu =3D NULL; =20 - if (!pmu) { - while ((pmu =3D perf_pmus__scan_core(pmu)) !=3D NULL) { - if (pmu->type =3D=3D PERF_TYPE_RAW) - break; - } - } - return pmu && perf_pmu__have_event(pmu, "slots"); + /* + * The PERF_TYPE_RAW type is the core PMU type, e.g., "cpu" PMU on a + * non-hybrid machine, "cpu_core" PMU on a hybrid machine. The + * topdown_sys_has_perf_metrics checks the slots event is only available + * for the core PMU, which supports the perf metrics feature. Checking + * both the PERF_TYPE_RAW type and the slots event should be good enough + * to detect the perf metrics feature. + */ + pmu =3D evsel__find_pmu(evsel); + return pmu && pmu->type =3D=3D PERF_TYPE_RAW; } =20 bool arch_evsel__must_be_in_group(const struct evsel *evsel) { - if (!evsel__sys_has_perf_metrics(evsel) || !evsel->name || - strcasestr(evsel->name, "uops_retired.slots")) + if (!evsel__sys_has_perf_metrics(evsel)) return false; =20 return arch_is_topdown_metrics(evsel) || arch_is_topdown_slots(evsel); diff --git a/tools/perf/arch/x86/util/topdown.c b/tools/perf/arch/x86/util/= topdown.c index d1c654839049..66b231fbf52e 100644 --- a/tools/perf/arch/x86/util/topdown.c +++ b/tools/perf/arch/x86/util/topdown.c @@ -1,6 +1,4 @@ // SPDX-License-Identifier: GPL-2.0 -#include "api/fs/fs.h" -#include "util/evsel.h" #include "util/evlist.h" #include "util/pmu.h" #include "util/pmus.h" @@ -8,6 +6,9 @@ #include "topdown.h" #include "evsel.h" =20 +// cmask=3D0, inv=3D0, pc=3D0, edge=3D0, umask=3D4, event=3D0 +#define TOPDOWN_SLOTS 0x0400 + /* Check whether there is a PMU which supports the perf metrics. */ bool topdown_sys_has_perf_metrics(void) { @@ -32,31 +33,19 @@ bool topdown_sys_has_perf_metrics(void) return has_perf_metrics; } =20 -#define TOPDOWN_SLOTS 0x0400 bool arch_is_topdown_slots(const struct evsel *evsel) { - if (evsel->core.attr.config =3D=3D TOPDOWN_SLOTS) - return true; - - return false; + return evsel->core.attr.type =3D=3D PERF_TYPE_RAW && + evsel->core.attr.config =3D=3D TOPDOWN_SLOTS && + evsel->core.attr.config1 =3D=3D 0; } =20 bool arch_is_topdown_metrics(const struct evsel *evsel) { - int config =3D evsel->core.attr.config; - const char *name_from_config; - struct perf_pmu *pmu; - - /* All topdown events have an event code of 0. */ - if ((config & 0xFF) !=3D 0) - return false; - - pmu =3D evsel__find_pmu(evsel); - if (!pmu || !pmu->is_core) - return false; - - name_from_config =3D perf_pmu__name_from_config(pmu, config); - return name_from_config && strcasestr(name_from_config, "topdown"); + // cmask=3D0, inv=3D0, pc=3D0, edge=3D0, umask=3D0x80-0x87, event=3D0 + return evsel->core.attr.type =3D=3D PERF_TYPE_RAW && + (evsel->core.attr.config & 0xFFFFF8FF) =3D=3D 0x8000 && + evsel->core.attr.config1 =3D=3D 0; } =20 /* diff --git a/tools/perf/arch/x86/util/topdown.h b/tools/perf/arch/x86/util/= topdown.h index 1bae9b1822d7..2349536cf882 100644 --- a/tools/perf/arch/x86/util/topdown.h +++ b/tools/perf/arch/x86/util/topdown.h @@ -2,6 +2,10 @@ #ifndef _TOPDOWN_H #define _TOPDOWN_H 1 =20 +#include + +struct evsel; + bool topdown_sys_has_perf_metrics(void); bool arch_is_topdown_slots(const struct evsel *evsel); bool arch_is_topdown_metrics(const struct evsel *evsel); --=20 2.50.0.727.gbf7dc18ff4-goog