[PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Sandipan Das]

The branch snapshot callback used by bpf_get_branch_snapshot() copies
up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
last LBR read kept fewer branches. The entries past lbr_stack.nr are
invalid, but they end up reaching BPF programs.

Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
amd_pmu_lbr_read() skips reading the registers, so that a stale count
is never reused.

Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
Signed-off-by: Sandipan Das <sandipan.das@amd.com>
---
 arch/x86/events/amd/core.c | 2 +-
 arch/x86/events/amd/lbr.c  | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 49b6b8fce566..bf631f60757c 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
 	cpuc = this_cpu_ptr(&cpu_hw_events);
 
 	amd_pmu_lbr_read();
-	cnt = min(cnt, x86_pmu.lbr_nr);
+	cnt = min(cnt, cpuc->lbr_stack.nr);
 	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
 
 	amd_pmu_v2_enable_all(0);
diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
index 1e1b2f08cb51..a00d3850c4a8 100644
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
 	struct branch_entry entry;
 	int out = 0, idx, i;
 
-	if (!cpuc->lbr_users)
+	if (!cpuc->lbr_users) {
+		cpuc->lbr_stack.nr = 0;
 		return;
+	}
 
 	for (i = 0; i < x86_pmu.lbr_nr; i++) {
 		entry.from.full	= amd_pmu_lbr_get_from(i);
-- 
2.53.0

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[sashiko-bot]

> The branch snapshot callback used by bpf_get_branch_snapshot() copies
> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
> last LBR read kept fewer branches. The entries past lbr_stack.nr are
> invalid, but they end up reaching BPF programs.
> 
> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
> amd_pmu_lbr_read() skips reading the registers, so that a stale count
> is never reused.
> 
> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007181002.4005262-1-sandipan.das@amd.com?part=1

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Peter Zijlstra]

On Wed, Oct 07, 2026 at 11:40:02PM +0530, Sandipan Das wrote:
> The branch snapshot callback used by bpf_get_branch_snapshot() copies
> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
> last LBR read kept fewer branches. The entries past lbr_stack.nr are
> invalid, but they end up reaching BPF programs.
> 
> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
> amd_pmu_lbr_read() skips reading the registers, so that a stale count
> is never reused.
> 
> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> ---
>  arch/x86/events/amd/core.c | 2 +-
>  arch/x86/events/amd/lbr.c  | 4 +++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 49b6b8fce566..bf631f60757c 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
>  	cpuc = this_cpu_ptr(&cpu_hw_events);
>  
>  	amd_pmu_lbr_read();
> -	cnt = min(cnt, x86_pmu.lbr_nr);
> +	cnt = min(cnt, cpuc->lbr_stack.nr);
>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>  
>  	amd_pmu_v2_enable_all(0);
> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> index 1e1b2f08cb51..a00d3850c4a8 100644
> --- a/arch/x86/events/amd/lbr.c
> +++ b/arch/x86/events/amd/lbr.c
> @@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
>  	struct branch_entry entry;
>  	int out = 0, idx, i;
>  
> -	if (!cpuc->lbr_users)
> +	if (!cpuc->lbr_users) {
> +		cpuc->lbr_stack.nr = 0;
>  		return;
> +	}
>  
>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
>  		entry.from.full	= amd_pmu_lbr_get_from(i);

You now seem to deviate from what the Intel code does. Please clarify.

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Sandipan Das]

On 12-10-2026 21:00, Peter Zijlstra wrote:
> On Wed, Oct 07, 2026 at 11:40:02PM +0530, Sandipan Das wrote:
>> The branch snapshot callback used by bpf_get_branch_snapshot() copies
>> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
>> last LBR read kept fewer branches. The entries past lbr_stack.nr are
>> invalid, but they end up reaching BPF programs.
>>
>> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
>> amd_pmu_lbr_read() skips reading the registers, so that a stale count
>> is never reused.
>>
>> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
>> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
>> ---
>>  arch/x86/events/amd/core.c | 2 +-
>>  arch/x86/events/amd/lbr.c  | 4 +++-
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
>> index 49b6b8fce566..bf631f60757c 100644
>> --- a/arch/x86/events/amd/core.c
>> +++ b/arch/x86/events/amd/core.c
>> @@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
>>  	cpuc = this_cpu_ptr(&cpu_hw_events);
>>  
>>  	amd_pmu_lbr_read();
>> -	cnt = min(cnt, x86_pmu.lbr_nr);
>> +	cnt = min(cnt, cpuc->lbr_stack.nr);
>>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>>  
>>  	amd_pmu_v2_enable_all(0);
>> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
>> index 1e1b2f08cb51..a00d3850c4a8 100644
>> --- a/arch/x86/events/amd/lbr.c
>> +++ b/arch/x86/events/amd/lbr.c
>> @@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
>>  	struct branch_entry entry;
>>  	int out = 0, idx, i;
>>  
>> -	if (!cpuc->lbr_users)
>> +	if (!cpuc->lbr_users) {
>> +		cpuc->lbr_stack.nr = 0;
>>  		return;
>> +	}
>>  
>>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
>>  		entry.from.full	= amd_pmu_lbr_get_from(i);
> 
> You now seem to deviate from what the Intel code does. Please clarify.

Yes, it is a deviation. I thought of proposing the following but didn't
proceed because I was not sure about the if condition in the second hunk
which considers things other than cpuc->lbr_users for an early return.

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0a34d674df59..5dfb9e9156e8 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2822,7 +2822,7 @@ __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
        struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);

        intel_pmu_lbr_read();
-       cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);
+       cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);

        memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
        intel_pmu_enable_all(0);
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index 0b446d8912e4..6b070490f461 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
         * but this simple approach seems to work for now.
         */
        if (!cpuc->lbr_users || vlbr_exclude_host() ||
-           cpuc->lbr_users == cpuc->lbr_pebs_users)
+           cpuc->lbr_users == cpuc->lbr_pebs_users) {
+               cpuc->lbr_stack.nr = 0;
                return;
+       }

        x86_pmu.lbr_read(cpuc);

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Peter Zijlstra]

On Tue, Oct 13, 2026 at 12:14:58PM +0530, Sandipan Das wrote:
> On 12-10-2026 21:00, Peter Zijlstra wrote:
> > On Wed, Oct 07, 2026 at 11:40:02PM +0530, Sandipan Das wrote:
> >> The branch snapshot callback used by bpf_get_branch_snapshot() copies
> >> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
> >> last LBR read kept fewer branches. The entries past lbr_stack.nr are
> >> invalid, but they end up reaching BPF programs.
> >>
> >> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
> >> amd_pmu_lbr_read() skips reading the registers, so that a stale count
> >> is never reused.
> >>
> >> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
> >> Reported-by: Sashiko <sashiko-bot@kernel.org>
> >> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
> >> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> >> ---
> >>  arch/x86/events/amd/core.c | 2 +-
> >>  arch/x86/events/amd/lbr.c  | 4 +++-
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> >> index 49b6b8fce566..bf631f60757c 100644
> >> --- a/arch/x86/events/amd/core.c
> >> +++ b/arch/x86/events/amd/core.c
> >> @@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
> >>  	cpuc = this_cpu_ptr(&cpu_hw_events);
> >>  
> >>  	amd_pmu_lbr_read();
> >> -	cnt = min(cnt, x86_pmu.lbr_nr);
> >> +	cnt = min(cnt, cpuc->lbr_stack.nr);
> >>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
> >>  
> >>  	amd_pmu_v2_enable_all(0);
> >> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> >> index 1e1b2f08cb51..a00d3850c4a8 100644
> >> --- a/arch/x86/events/amd/lbr.c
> >> +++ b/arch/x86/events/amd/lbr.c
> >> @@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
> >>  	struct branch_entry entry;
> >>  	int out = 0, idx, i;
> >>  
> >> -	if (!cpuc->lbr_users)
> >> +	if (!cpuc->lbr_users) {
> >> +		cpuc->lbr_stack.nr = 0;
> >>  		return;
> >> +	}
> >>  
> >>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
> >>  		entry.from.full	= amd_pmu_lbr_get_from(i);
> > 
> > You now seem to deviate from what the Intel code does. Please clarify.
> 
> Yes, it is a deviation. I thought of proposing the following but didn't
> proceed because I was not sure about the if condition in the second hunk
> which considers things other than cpuc->lbr_users for an early return.

Dapeng, can you look into this? I would really like to keep behaviour
consistent where possible.

> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index 0a34d674df59..5dfb9e9156e8 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -2822,7 +2822,7 @@ __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>         struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
> 
>         intel_pmu_lbr_read();
> -       cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);
> +       cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
> 
>         memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>         intel_pmu_enable_all(0);
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 0b446d8912e4..6b070490f461 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
>          * but this simple approach seems to work for now.
>          */
>         if (!cpuc->lbr_users || vlbr_exclude_host() ||
> -           cpuc->lbr_users == cpuc->lbr_pebs_users)
> +           cpuc->lbr_users == cpuc->lbr_pebs_users) {
> +               cpuc->lbr_stack.nr = 0;
>                 return;
> +       }
> 
>         x86_pmu.lbr_read(cpuc);
> 

Re: [PATCH] perf/x86/amd/lbr: Limit branch snapshots to valid entries

[Mi, Dapeng]

On 10/13/2026 3:46 PM, Peter Zijlstra wrote:
> On Tue, Oct 13, 2026 at 12:14:58PM +0530, Sandipan Das wrote:
>> On 12-10-2026 21:00, Peter Zijlstra wrote:
>>> On Wed, Oct 07, 2026 at 11:40:02PM +0530, Sandipan Das wrote:
>>>> The branch snapshot callback used by bpf_get_branch_snapshot() copies
>>>> up to x86_pmu.lbr_nr entries from cpuc->lbr_entries[], even when the
>>>> last LBR read kept fewer branches. The entries past lbr_stack.nr are
>>>> invalid, but they end up reaching BPF programs.
>>>>
>>>> Limit copying to lbr_stack.nr entries and also reset lbr_stack.nr when
>>>> amd_pmu_lbr_read() skips reading the registers, so that a stale count
>>>> is never reused.
>>>>
>>>> Fixes: a4d18112e531 ("perf/x86/amd: Support capturing LBR from software events")
>>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>>> Closes: https://lore.kernel.org/all/sashiko-outbox-163130@kernel.org/
>>>> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
>>>> ---
>>>>  arch/x86/events/amd/core.c | 2 +-
>>>>  arch/x86/events/amd/lbr.c  | 4 +++-
>>>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
>>>> index 49b6b8fce566..bf631f60757c 100644
>>>> --- a/arch/x86/events/amd/core.c
>>>> +++ b/arch/x86/events/amd/core.c
>>>> @@ -944,7 +944,7 @@ static int amd_pmu_v2_snapshot_branch_stack(struct perf_branch_entry *entries, u
>>>>  	cpuc = this_cpu_ptr(&cpu_hw_events);
>>>>  
>>>>  	amd_pmu_lbr_read();
>>>> -	cnt = min(cnt, x86_pmu.lbr_nr);
>>>> +	cnt = min(cnt, cpuc->lbr_stack.nr);
>>>>  	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>>>>  
>>>>  	amd_pmu_v2_enable_all(0);
>>>> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
>>>> index 1e1b2f08cb51..a00d3850c4a8 100644
>>>> --- a/arch/x86/events/amd/lbr.c
>>>> +++ b/arch/x86/events/amd/lbr.c
>>>> @@ -165,8 +165,10 @@ void amd_pmu_lbr_read(void)
>>>>  	struct branch_entry entry;
>>>>  	int out = 0, idx, i;
>>>>  
>>>> -	if (!cpuc->lbr_users)
>>>> +	if (!cpuc->lbr_users) {
>>>> +		cpuc->lbr_stack.nr = 0;
>>>>  		return;
>>>> +	}
>>>>  
>>>>  	for (i = 0; i < x86_pmu.lbr_nr; i++) {
>>>>  		entry.from.full	= amd_pmu_lbr_get_from(i);
>>> You now seem to deviate from what the Intel code does. Please clarify.
>> Yes, it is a deviation. I thought of proposing the following but didn't
>> proceed because I was not sure about the if condition in the second hunk
>> which considers things other than cpuc->lbr_users for an early return.
> Dapeng, can you look into this? I would really like to keep behaviour
> consistent where possible.

Sure.


>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index 0a34d674df59..5dfb9e9156e8 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -2822,7 +2822,7 @@ __intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
>>         struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
>>
>>         intel_pmu_lbr_read();
>> -       cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);
>> +       cnt = min_t(unsigned int, cnt, cpuc->lbr_stack.nr);
>>
>>         memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
>>         intel_pmu_enable_all(0);
>> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
>> index 0b446d8912e4..6b070490f461 100644
>> --- a/arch/x86/events/intel/lbr.c
>> +++ b/arch/x86/events/intel/lbr.c
>> @@ -1026,8 +1026,10 @@ void intel_pmu_lbr_read(void)
>>          * but this simple approach seems to work for now.
>>          */
>>         if (!cpuc->lbr_users || vlbr_exclude_host() ||
>> -           cpuc->lbr_users == cpuc->lbr_pebs_users)
>> +           cpuc->lbr_users == cpuc->lbr_pebs_users) {
>> +               cpuc->lbr_stack.nr = 0;
I checked the code, and it looks safe to clear cpuc->lbr_stack.nr here. For
PEBS events with the LBR group, the LBR stack is populated again for each
PEBS record, so this should not affect the LBR trace for PEBS events.

But I’m not fully sure if it's intentional for the
bpf_get_branch_snapshot() helper. Clearing lbr_stack.nr to 0 prevents the
BPF helper from reading LBR data from PEBS although the LBR data could be
stale since it comes from the past PEBS record instead of the runtime
hardware LBR state. That seems reasonable and correct, but the side effect
is that the BPF helper can't read the LBR stack when all LBR events are
PEBS events. Not sure if it's intentional and expected. 

I’d like to run some tests for this change tomorrow to confirm the
behavior. Thanks.


>>                 return;
>> +       }
>>
>>         x86_pmu.lbr_read(cpuc);
>>