linux-kernel - Re: [PATCH V6 3/3] perf/x86/intel: Support PEBS counters snapshotting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241220142221.GN11133@noisy.programming.kicks-ass.net>
Date: Fri, 20 Dec 2024 15:22:21 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: kan.liang@...ux.intel.com
Cc: mingo@...hat.com, acme@...nel.org, namhyung@...nel.org,
	irogers@...gle.com, linux-kernel@...r.kernel.org,
	linux-perf-users@...r.kernel.org, ak@...ux.intel.com,
	eranian@...gle.com, dapeng1.mi@...ux.intel.com
Subject: Re: [PATCH V6 3/3] perf/x86/intel: Support PEBS counters snapshotting

On Wed, Dec 18, 2024 at 07:16:43AM -0800, kan.liang@...ux.intel.com wrote:

> @@ -3109,6 +3116,27 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
>  		if (!test_bit(bit, cpuc->active_mask))
>  			continue;
>  

> +		if (is_pebs_counter_event(event))
> +			x86_pmu.drain_pebs(regs, &data);
> +
>  		if (!intel_pmu_save_and_restart(event))
>  			continue;
>  
> @@ -4056,6 +4084,23 @@ static int intel_pmu_hw_config(struct perf_event *event)
>  		event->hw.flags |= PERF_X86_EVENT_PEBS_VIA_PT;
>  	}
>  
> +	if ((event->attr.sample_type & PERF_SAMPLE_READ) &&
> +	    (x86_pmu.intel_cap.pebs_format >= 6)) {

Right, so the event that has SAMPLE_READ on is 'event'

> +		struct perf_event *leader = event->group_leader;
> +		bool slots_leader = is_slots_event(leader);
> +
> +		if (slots_leader)
> +			leader = list_next_entry(leader, sibling_list);

Uh, what, why?

> +
> +		if (leader->attr.precise_ip) {
> +			event->hw.flags |= PERF_X86_EVENT_PEBS_CNTR;
> +			if (slots_leader) {
> +				leader->hw.flags |= PERF_X86_EVENT_PEBS_CNTR;
> +				event->group_leader->hw.flags |= PERF_X86_EVENT_PEBS_CNTR;
> +			}
> +		}

And this is more confusion. You want event to be a PEBS event, not the
leader, you don't care about the leader.


> +	}
> +
>  	if ((event->attr.type == PERF_TYPE_HARDWARE) ||
>  	    (event->attr.type == PERF_TYPE_HW_CACHE))
>  		return 0;

> +static inline bool is_pebs_counter_event(struct perf_event *event)
> +{
> +	return event->hw.flags & PERF_X86_EVENT_PEBS_CNTR;
> +}

For that drain_pebs() thing, you want all group members to have
PEBS_CNTR set.

That is, if PEBS>=6 and event is PEBS and event has SAMPLE_READ, then
mark the whole group with PEBS_CNTR

SAMPLE_READ doesn't particularly care who's the leader, the event that
has SAMPLE_READ will read the whole group. Heck they could all have
SAMPLE_READ and then all their samples will read each-other.