Re: [PATCH] perf/amd: Prevent grouping of IBS events

From: Ian Rogers
Date: Tue Jun 20 2023 - 12:45:07 EST


On Tue, Jun 20, 2023 at 2:16 AM Ravi Bangoria <ravi.bangoria@xxxxxxx> wrote:
>
> IBS PMUs can have only one event active at any point in time. Restrict
> grouping of multiple IBS events.

Thanks Ravi,

can you provide an example/test for this? Should this be a weak group issue?

Thanks,
Ian

> Reported-by: Sandipan Das <sandipan.das@xxxxxxx>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxx>
> ---
> arch/x86/events/amd/ibs.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 371014802191..74e664266753 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -247,11 +247,33 @@ int forward_event_to_ibs(struct perf_event *event)
> return -ENOENT;
> }
>
> +/*
> + * Grouping of IBS events is not possible since IBS can have only
> + * one event active at any point in time.
> + */
> +static int validate_group(struct perf_event *event)
> +{
> + struct perf_event *sibling;
> +
> + if (event->group_leader == event)
> + return 0;
> +
> + if (event->group_leader->pmu == event->pmu)
> + return -EINVAL;
> +
> + for_each_sibling_event(sibling, event->group_leader) {
> + if (sibling->pmu == event->pmu)
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> static int perf_ibs_init(struct perf_event *event)
> {
> struct hw_perf_event *hwc = &event->hw;
> struct perf_ibs *perf_ibs;
> u64 max_cnt, config;
> + int ret;
>
> perf_ibs = get_ibs_pmu(event->attr.type);
> if (!perf_ibs)
> @@ -265,6 +287,10 @@ static int perf_ibs_init(struct perf_event *event)
> if (config & ~perf_ibs->config_mask)
> return -EINVAL;
>
> + ret = validate_group(event);
> + if (ret)
> + return ret;
> +
> if (hwc->sample_period) {
> if (config & perf_ibs->cnt_mask)
> /* raw max_cnt may not be set */
> --
> 2.41.0
>