linux-kernel - Re: [PATCH V9 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: <ab878ced-9f8e-470b-b3fd-f12f7f042519@linux.intel.com>
Date: Thu, 23 Jan 2025 10:36:23 -0500
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: mingo@...hat.com, acme@...nel.org, namhyung@...nel.org,
 irogers@...gle.com, adrian.hunter@...el.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 V9 3/3] perf/x86/intel: Support PEBS counters snapshotting



On 2025-01-23 4:14 a.m., Peter Zijlstra wrote:
> On Thu, Jan 16, 2025 at 04:50:01PM -0500, Liang, Kan wrote:
>>
>>
>> On 2025-01-16 3:56 p.m., Peter Zijlstra wrote:
>>> On Thu, Jan 16, 2025 at 09:42:25PM +0100, Peter Zijlstra wrote:
>>>> On Thu, Jan 16, 2025 at 10:55:46AM -0500, Liang, Kan wrote:
>>>>
>>>>>> Also, I think I found you another bug... Consider what happens to the
>>>>>> counter value when we reschedule a HES_STOPPED counter, then we skip
>>>>>> x86_pmu_start(RELOAD) on step2, which leave the counter value with
>>>>>> 'random' crap from whatever was there last.
>>>>>>
>>>>>> But meanwhile you do program PEBS to sample it. That will happily sample
>>>>>> this garbage.
>>>>>>
>>>>>> Hmm?
>>>>>
>>>>> I'm not quite sure I understand the issue.
>>>>>
>>>>> The HES_STOPPED counter should be a pre-existing counter. Just for some
>>>>> reason, it's stopped, right? So perf doesn't need to re-configure the
>>>>> PEBS__DATA_CFG, since the idx is not changed.
>>>>
>>>> Suppose you have your group {A, B, C} and lets suppose A is the PEBS
>>>> event, further suppose that B is also a sampling event. Lets say they
>>>> get hardware counters 1,2 and 3 respectively.
>>>>
>>>> Then lets say B gets throttled.
>>>>
>>>> While it is throttled, we get a new event D scheduled, and D gets placed
>>>> on counter 2 -- where B lives, which gets moved over to counter 4.
>>>>
>>>> Then our loops will update and remove B from 2, but because
>>>> throttled/HES_STOPPED it will not start it on counter 4.
>>>>>> Meanwhile, we do have the PEBS_DATA_CFG thing updated to sample counter
>>>> 1,3 and 4.
>>>>
>>>> PEBS assist happens, and samples the uninitialized counter 4.
>>>> Also, by skipping x86_pmu_start() we miss the assignment of
>>> cpuc->events[] so PEBS buffer decode can't even find the dodgy event.
>>>
>>
>> Yes, counter 4 includes garbage before the B is started again.
>> But the cpuc->events[counter 4] is NULL either.
>>
>> The current implementation ignores the NULL cpuc->events[]. The stopped
>> B should not be mistakenly updated.
> 
> Ah, indeed. I was so close.
> 
> One question though -- is this value ever exposed otherwise? I had a
> quick look and I don't think we support PERF_SAMPLE_RAW for PEBS, but
> what about PEBS-to-PT ? 
>

The counters snapshotting feature is only available on the latest
platforms, lunar lake/arrow lake, and future platforms with Arch PEBS.

The PEBS-to-PT is not enumerated on lunar lake/arrow lake. (It actually
never works on hybrid.) It will be deprecated on Arch PEBS as well.
So we don't need to worry about the PEBS-to-PT.

> Anywya, let me go find this v10 thing :-)

Thanks.

Kan