Date:   Fri, 12 Nov 2021 17:39:19 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Stephane Eranian <eranian@google.com>
Cc:     linux-kernel@vger.kernel.org, kim.phillips@amd.com,
        acme@redhat.com, jolsa@redhat.com, songliubraving@fb.com,
        mpe@ellerman.id.au, maddy@linux.ibm.com
Subject: Re: [PATCH v2 03/13] perf/x86/amd: add AMD Fam19h Branch Sampling
 support
Message-ID: <YY6YtwqSIWxP7GfR@hirez.programming.kicks-ass.net>
References: <20211111084415.663951-1-eranian@google.com>
 <20211111084415.663951-4-eranian@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211111084415.663951-4-eranian@google.com>
Precedence: bulk

On Thu, Nov 11, 2021 at 12:44:05AM -0800, Stephane Eranian wrote:
> +/*
> + * Because of the way BRS operates with an inactive and active phases, and
> + * the link to one counter, it is not possible to have two events using BRS
> + * scheduled at the same time. There would be an issue with enforcing the
> + * period of each one and given that the BRS saturates, it would not be possible
> + * to guarantee correlated content for all events. Therefore, in situations
> + * where multiple events want to use BRS, the kernel enforces mutual exclusion.
> + * Exclusion is enforced by chosing only one counter for events using BRS.
> + * The event scheduling logic will then automatically multiplex the
> + * events and ensure that at most one event is actively using BRS.
> + *
> + * The BRS counter could be any counter, but there is no constraint on Fam19h,
> + * therefore all counters are equal and thus we pick the first one: PMC0
> + */
> +static struct event_constraint amd_fam19h_brs_cntr0_constraint =
> +	EVENT_CONSTRAINT(0, 0x1, AMD64_RAW_EVENT_MASK);
> +
> +static struct event_constraint amd_fam19h_brs_pair_cntr0_constraint =
> +	__EVENT_CONSTRAINT(0, 0x1, AMD64_RAW_EVENT_MASK, 1, 0, PERF_X86_EVENT_PAIR);
> +
> +static struct event_constraint *
> +amd_get_event_constraints_f19h(struct cpu_hw_events *cpuc, int idx,
> +			  struct perf_event *event)
> +{
> +	struct hw_perf_event *hwc = &event->hw;
> +	bool has_brs = is_amd_brs(hwc);
> +
> +	/*
> +	 * In case BRS is used with an event requiring a counter pair,
> +	 * the kernel allows it but only on counter 0 & 1 to enforce
> +	 * multiplexing requiring to protect BRS in case of multiple
> +	 * BRS users
> +	 */
> +	if (amd_is_pair_event_code(hwc)) {
> +		return has_brs ? &amd_fam19h_brs_pair_cntr0_constraint
> +			       : &pair_constraint;
> +	}
> +
> +	if (has_brs)
> +		return &amd_fam19h_brs_cntr0_constraint;
> +
> +	return &unconstrained;
> +}

That still allows BRS to be used together with unrelated counters and it
*will* destroy their utility by delaying the NMI.

BRS could perhaps share the PMU with !sampling counters, but sharing
with sampling counters is just asking for trouble.