From: Peter Zijlstra <peterz@infradead.org> Date: 2021-09-29 07:35:52
On Wed, Sep 29, 2021 at 12:04:21AM +0000, Song Liu wrote:
Hi Peter,
We have see the warning below while testing the new bpf_get_branch_snapshot
helper, on a QEMU vm (/usr/bin/qemu-system-x86_64 -enable-kvm -cpu host ...).
This issue doesn't happen on bare metal systems (no QEMU).
We didn't cover this case, as LBR didn't really work in QEMU. But it seems to
work after I upgrade the host kernel to 5.12.
At the moment, we don't have much idea on how to debug and fix the issue. Could
you please share your thoughts on this?
Well, that's virt, afaik stuff not working is like a feature there or
something, who knows. I've Cc'ed Like Xu who might have clue since he
did the patches.
Virt just ain't worth the pain if you ask me.
On Wed, Sep 29, 2021 at 12:04:21AM +0000, Song Liu wrote:
quoted
Hi Peter,
We have see the warning below while testing the new bpf_get_branch_snapshot
helper, on a QEMU vm (/usr/bin/qemu-system-x86_64 -enable-kvm -cpu host ...).
This issue doesn't happen on bare metal systems (no QEMU).
We didn't cover this case, as LBR didn't really work in QEMU. But it seems to
work after I upgrade the host kernel to 5.12.
The guest LBR is enabled since the v5.12.
quoted
At the moment, we don't have much idea on how to debug and fix the issue. Could
you please share your thoughts on this?
Well, that's virt, afaik stuff not working is like a feature there or
something, who knows. I've Cc'ed Like Xu who might have clue since he
did the patches.
Virt just ain't worth the pain if you ask me.
Just cc me for any vPMU/x86 stuff.
quoted
Thanks in advance!
Song
============================== 8< ============================
[ 139.494159] unchecked MSR access error: WRMSR to 0x3f1 (tried to write 0x0000000000000000) at rIP: 0xffffffff81011a8b (intel_pmu_snapshot_branch_stack+0x3b/0xd0)
Uh, it uses a PEBS counter to sample or count, which is not yet upstream but
should be soon.
Song, can you try to fix bpf_get_branch_snapshot on a normal PMC counter,
or where is the src for bpf_get_branch_snapshot? I am more than happy to help.
From: Peter Zijlstra <peterz@infradead.org> Date: 2021-09-29 12:26:16
On Wed, Sep 29, 2021 at 08:05:24PM +0800, Like Xu wrote:
On 29/9/2021 3:35 pm, Peter Zijlstra wrote:
quoted
quoted
[ 139.494159] unchecked MSR access error: WRMSR to 0x3f1 (tried to write 0x0000000000000000) at rIP: 0xffffffff81011a8b (intel_pmu_snapshot_branch_stack+0x3b/0xd0)
Uh, it uses a PEBS counter to sample or count, which is not yet upstream but
should be soon.
Ooh that's PEBS_ENABLE
Song, can you try to fix bpf_get_branch_snapshot on a normal PMC counter,
or where is the src for bpf_get_branch_snapshot? I am more than happy to help.
Nah, all that code wants to do is disable PEBS... and virt being virt,
it's all sorts of weird with f/m/s :/
I so hate all that. So there's two solutions:
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
or
- create another 2 intel_pmu_snapshot_{,arch_}branch_stack() copies
that omit the PEBS thing.
On Sep 29, 2021, at 5:05 AM, Like Xu [off-list ref] wrote:
On 29/9/2021 3:35 pm, Peter Zijlstra wrote:
quoted
On Wed, Sep 29, 2021 at 12:04:21AM +0000, Song Liu wrote:
quoted
Hi Peter,
We have see the warning below while testing the new bpf_get_branch_snapshot
helper, on a QEMU vm (/usr/bin/qemu-system-x86_64 -enable-kvm -cpu host ...).
This issue doesn't happen on bare metal systems (no QEMU).
We didn't cover this case, as LBR didn't really work in QEMU. But it seems to
work after I upgrade the host kernel to 5.12.
The guest LBR is enabled since the v5.12.
quoted
quoted
At the moment, we don't have much idea on how to debug and fix the issue. Could
you please share your thoughts on this?
Well, that's virt, afaik stuff not working is like a feature there or
something, who knows. I've Cc'ed Like Xu who might have clue since he
did the patches.
Virt just ain't worth the pain if you ask me.
Just cc me for any vPMU/x86 stuff.
quoted
quoted
Thanks in advance!
Song
============================== 8< ============================
[ 139.494159] unchecked MSR access error: WRMSR to 0x3f1 (tried to write 0x0000000000000000) at rIP: 0xffffffff81011a8b (intel_pmu_snapshot_branch_stack+0x3b/0xd0)
Uh, it uses a PEBS counter to sample or count, which is not yet upstream but should be soon.
Song, can you try to fix bpf_get_branch_snapshot on a normal PMC counter,
or where is the src for bpf_get_branch_snapshot? I am more than happy to help.
bpf_get_branch_snapshot is available in the bpf-next tree:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
To repro the issue:
1. build the kernel and boot qemu with it;
2. build tools/testing/selftests/bpf
cd tools/testing/selftests/bpf ; make -j
3. copy test_progs and bpf_testmod.ko from tools/testing/selftests/bpf
to the qemu vm;
4. run the test as
./test_progs -t snapshot -v
This should trigger "unchecked MSR access error" in dmesg. Please
let me know if this doesn't work.
Thanks,
Song
On Sep 29, 2021, at 5:26 AM, Peter Zijlstra [off-list ref] wrote:
On Wed, Sep 29, 2021 at 08:05:24PM +0800, Like Xu wrote:
quoted
On 29/9/2021 3:35 pm, Peter Zijlstra wrote:
quoted
quoted
quoted
[ 139.494159] unchecked MSR access error: WRMSR to 0x3f1 (tried to write 0x0000000000000000) at rIP: 0xffffffff81011a8b (intel_pmu_snapshot_branch_stack+0x3b/0xd0)
Uh, it uses a PEBS counter to sample or count, which is not yet upstream but
should be soon.
Ooh that's PEBS_ENABLE
quoted
Song, can you try to fix bpf_get_branch_snapshot on a normal PMC counter,
or where is the src for bpf_get_branch_snapshot? I am more than happy to help.
Nah, all that code wants to do is disable PEBS... and virt being virt,
it's all sorts of weird with f/m/s :/
I so hate all that. So there's two solutions:
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
Thanks,
Song
or
- create another 2 intel_pmu_snapshot_{,arch_}branch_stack() copies
that omit the PEBS thing.
From: Peter Zijlstra <peterz@infradead.org> Date: 2021-09-29 16:07:09
On Wed, Sep 29, 2021 at 02:42:27PM +0000, Song Liu wrote:
Hi Peter,
quoted
On Sep 29, 2021, at 5:26 AM, Peter Zijlstra [off-list ref] wrote:
On Wed, Sep 29, 2021 at 08:05:24PM +0800, Like Xu wrote:
quoted
On 29/9/2021 3:35 pm, Peter Zijlstra wrote:
quoted
quoted
quoted
[ 139.494159] unchecked MSR access error: WRMSR to 0x3f1 (tried to write 0x0000000000000000) at rIP: 0xffffffff81011a8b (intel_pmu_snapshot_branch_stack+0x3b/0xd0)
Uh, it uses a PEBS counter to sample or count, which is not yet upstream but
should be soon.
Ooh that's PEBS_ENABLE
quoted
Song, can you try to fix bpf_get_branch_snapshot on a normal PMC counter,
or where is the src for bpf_get_branch_snapshot? I am more than happy to help.
Nah, all that code wants to do is disable PEBS... and virt being virt,
it's all sorts of weird with f/m/s :/
I so hate all that. So there's two solutions:
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them
next. But maybe Kan or Andi know without asking.
From: Liang, Kan <hidden> Date: 2021-09-29 16:35:39
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks,
Kan
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */-__intel_pmu_pebs_disable_all();__intel_pmu_lbr_disable();/* ... until here */return__intel_pmu_snapshot_branch_stack(entries,cnt,flags);
@@ -2223,7 +2222,6 @@ intel_pmu_snapshot_arch_branch_stack(struct perf_branch_entry *entries, unsigned/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */-__intel_pmu_pebs_disable_all();__intel_pmu_arch_lbr_disable();/* ... until here */return__intel_pmu_snapshot_branch_stack(entries,cnt,flags);
In the test, this does eliminate the warning.
Thanks,
Song
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them
next. But maybe Kan or Andi know without asking.
On modern CPUs it should be sufficient. In fact the perfmon v4 path that
that was recently deleted did it.
I'm not sure about all old CPUs. There was a corner case on older CPUs
(Haswell and Broadwell) with old Microcode where it was needed when entering virtualization
(but that should be fixed now)
-Andi
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
quoted hunk
/* ... until here */
return __intel_pmu_snapshot_branch_stack(entries, cnt, flags);
@@ -2223,7 +2222,6 @@ intel_pmu_snapshot_arch_branch_stack(struct perf_branch_entry *entries, unsigned /* must not have branches... */ local_irq_save(flags); __intel_pmu_disable_all(false); /* we don't care about BTS */- __intel_pmu_pebs_disable_all(); __intel_pmu_arch_lbr_disable(); /* ... until here */ return __intel_pmu_snapshot_branch_stack(entries, cnt, flags);
In the test, this does eliminate the warning.
Thanks,
Song
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
really useful in the VM, because all the interesting entries are flushed
by these. I am not sure how to further optimize these. Do you have some
suggestions on this?
Thanks,
Song
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
Do you use counters for snapshot branch stack?
Can the assumption of "without PMI" be broken sine Intel does have
the hardware mechanism like "freeze LBR on counter overflow
(aka, DEBUGCTLMSR_FREEZE_LBRS_ON_PMI)" ?
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Yes, it makes sense.
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
Is it possible that we have another LBR in-kernel user in addition to the current
BPF-LBR snapshot user, such as another BPF-LBR snapshot user or a LBR perf user ?
In the intel_pmu_snapshot_[arch]_branch_stack(), what if there is a PMI or NMI
handler
to be called before __intel_pmu_lbr_disable(), which means more branch instructions
(assuming we don't use the FREEZE_LBRS_ON_xxx capability)?
How about try to disable LBR at the earliest possible time, before
__intel_pmu_disable_all(false) ?
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
Are you saying that you still need to call
__intel_pmu_pebs_disable_all() to maintain precision ?
really useful in the VM, because all the interesting entries are flushed
by these. I am not sure how to further optimize these. Do you have some
suggestions on this?
On Oct 7, 2021, at 11:36 PM, Like Xu [off-list ref] wrote:
On 8/10/2021 1:46 pm, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
Do you use counters for snapshot branch stack?
Can the assumption of "without PMI" be broken sine Intel does have
the hardware mechanism like "freeze LBR on counter overflow
(aka, DEBUGCTLMSR_FREEZE_LBRS_ON_PMI)" ?
We are capturing LBR on software events. For example, when a complex syscall,
such as sys_bpf() and sys_perf_event_open(), returns -EINVAL, it is not obvious
what wen wrong. The branch stack at the return (on a kretprobe or fexit) could
give us additional information.
quoted
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Yes, it makes sense.
quoted
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
Is it possible that we have another LBR in-kernel user in addition to the current
BPF-LBR snapshot user, such as another BPF-LBR snapshot user or a LBR perf user ?
I think it is OK to have another user. We just need to capture the LBR entries.
In fact, we simply enable LBR by opening a perf_event on each CPU. So from the
kernel's point of view, the LBR is owned used by "another user".
In the intel_pmu_snapshot_[arch]_branch_stack(), what if there is a PMI or NMI handler
to be called before __intel_pmu_lbr_disable(), which means more branch instructions
(assuming we don't use the FREEZE_LBRS_ON_xxx capability)?
If we are unlucky and hit an NMI, we may get garbage data. The user will run the
test again.
How about try to disable LBR at the earliest possible time, before __intel_pmu_disable_all(false) ?
I am not sure which solution is the best here. On bare metal, current version works
fine (available in bpf-next tree).
quoted
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
Are you saying that you still need to call
__intel_pmu_pebs_disable_all() to maintain precision ?
I think we don't need pebs_disable_all. In the VM, pebs_disable_all will trigger
"unchecked MSR access error" warning. After removing it, the warning message is
gone. However, after we remove pebs_disable_all, we still see too many LBR entries
are flushed before LBR is stopped. Most of these new entries are in extable code.
I guess this is because the VM access these MSR differently.
Thanks,
Song
On Oct 7, 2021, at 11:36 PM, Like Xu [off-list ref] wrote:
On 8/10/2021 1:46 pm, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
Do you use counters for snapshot branch stack?
Can the assumption of "without PMI" be broken sine Intel does have
the hardware mechanism like "freeze LBR on counter overflow
(aka, DEBUGCTLMSR_FREEZE_LBRS_ON_PMI)" ?
We are capturing LBR on software events. For example, when a complex syscall,
such as sys_bpf() and sys_perf_event_open(), returns -EINVAL, it is not obvious
what wen wrong. The branch stack at the return (on a kretprobe or fexit) could
give us additional information.
quoted
quoted
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Yes, it makes sense.
quoted
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
Is it possible that we have another LBR in-kernel user in addition to the current
BPF-LBR snapshot user, such as another BPF-LBR snapshot user or a LBR perf user ?
I think it is OK to have another user. We just need to capture the LBR entries.
In fact, we simply enable LBR by opening a perf_event on each CPU. So from the
kernel's point of view, the LBR is owned used by "another user".
quoted
In the intel_pmu_snapshot_[arch]_branch_stack(), what if there is a PMI or NMI handler
to be called before __intel_pmu_lbr_disable(), which means more branch instructions
(assuming we don't use the FREEZE_LBRS_ON_xxx capability)?
If we are unlucky and hit an NMI, we may get garbage data. The user will run the
test again.
quoted
How about try to disable LBR at the earliest possible time, before __intel_pmu_disable_all(false) ?
I am not sure which solution is the best here. On bare metal, current version works
fine (available in bpf-next tree).
quoted
quoted
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
Are you saying that you still need to call
__intel_pmu_pebs_disable_all() to maintain precision ?
I think we don't need pebs_disable_all. In the VM, pebs_disable_all will trigger
"unchecked MSR access error" warning. After removing it, the warning message is
gone. However, after we remove pebs_disable_all, we still see too many LBR entries
are flushed before LBR is stopped. Most of these new entries are in extable code.
I guess this is because the VM access these MSR differently.
Hi Song,
Thanks for your detailed input. I saw your workaround "if (is_hypervisor())" on
the tree.
Even when the guest supports PEBS, this use case fails and the root cause is still
playing hide-and-seek with me. Just check with you to see if you get similar results
when the guest LBR behavior makes the test case fail like this:
serial_test_get_branch_snapshot:FAIL:find_looptest_in_lbr unexpected
find_looptest_in_lbr: actual 0 <= expected 6
serial_test_get_branch_snapshot:FAIL:check_wasted_entries unexpected
check_wasted_entries: actual 32 >= expected 10
#52 get_branch_snapshot:FAIL
Also, do you know or rough guess about how extable code relates to the test case ?
On Oct 9, 2021, at 2:03 AM, Like Xu [off-list ref] wrote:
On 9/10/2021 1:08 am, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 11:36 PM, Like Xu [off-list ref] wrote:
On 8/10/2021 1:46 pm, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
Do you use counters for snapshot branch stack?
Can the assumption of "without PMI" be broken sine Intel does have
the hardware mechanism like "freeze LBR on counter overflow
(aka, DEBUGCTLMSR_FREEZE_LBRS_ON_PMI)" ?
We are capturing LBR on software events. For example, when a complex syscall,
such as sys_bpf() and sys_perf_event_open(), returns -EINVAL, it is not obvious
what wen wrong. The branch stack at the return (on a kretprobe or fexit) could
give us additional information.
quoted
quoted
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Yes, it makes sense.
quoted
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
Is it possible that we have another LBR in-kernel user in addition to the current
BPF-LBR snapshot user, such as another BPF-LBR snapshot user or a LBR perf user ?
I think it is OK to have another user. We just need to capture the LBR entries.
In fact, we simply enable LBR by opening a perf_event on each CPU. So from the
kernel's point of view, the LBR is owned used by "another user".
quoted
In the intel_pmu_snapshot_[arch]_branch_stack(), what if there is a PMI or NMI handler
to be called before __intel_pmu_lbr_disable(), which means more branch instructions
(assuming we don't use the FREEZE_LBRS_ON_xxx capability)?
If we are unlucky and hit an NMI, we may get garbage data. The user will run the
test again.
quoted
How about try to disable LBR at the earliest possible time, before __intel_pmu_disable_all(false) ?
I am not sure which solution is the best here. On bare metal, current version works
fine (available in bpf-next tree).
quoted
quoted
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
Are you saying that you still need to call
__intel_pmu_pebs_disable_all() to maintain precision ?
I think we don't need pebs_disable_all. In the VM, pebs_disable_all will trigger
"unchecked MSR access error" warning. After removing it, the warning message is
gone. However, after we remove pebs_disable_all, we still see too many LBR entries
are flushed before LBR is stopped. Most of these new entries are in extable code.
I guess this is because the VM access these MSR differently.
Hi Song,
Thanks for your detailed input. I saw your workaround "if (is_hypervisor())" on the tree.
Even when the guest supports PEBS, this use case fails and the root cause is still
playing hide-and-seek with me. Just check with you to see if you get similar results
when the guest LBR behavior makes the test case fail like this:
serial_test_get_branch_snapshot:FAIL:find_looptest_in_lbr unexpected find_looptest_in_lbr: actual 0 <= expected 6
serial_test_get_branch_snapshot:FAIL:check_wasted_entries unexpected check_wasted_entries: actual 32 >= expected 10
#52 get_branch_snapshot:FAIL
Also, do you know or rough guess about how extable code relates to the test case ?
Sorry for the delayed response. I finally got some time to look into
this again. After disabling most debug configs, I managed to get it
work in the VM with a simple change as
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */-__intel_pmu_pebs_disable_all();__intel_pmu_lbr_disable();/* ... until here */return__intel_pmu_snapshot_branch_stack(entries,cnt,flags);
(of course we also need to remove the is_hypervisor() check.).
But I am not sure whether this is the best fix.
I pushed all the change and debug code I used to
https://git.kernel.org/pub/scm/linux/kernel/git/song/linux.git/log/?h=get_branch_snapshot_in_vm
Could you please take a look at it and share your feedback on this?
Specifically, can we fix intel_pmu_snapshot_branch_stack in vm with the
change above?
Thanks,
Song
On Oct 9, 2021, at 2:03 AM, Like Xu [off-list ref] wrote:
On 9/10/2021 1:08 am, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 11:36 PM, Like Xu [off-list ref] wrote:
On 8/10/2021 1:46 pm, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
Do you use counters for snapshot branch stack?
Can the assumption of "without PMI" be broken sine Intel does have
the hardware mechanism like "freeze LBR on counter overflow
(aka, DEBUGCTLMSR_FREEZE_LBRS_ON_PMI)" ?
We are capturing LBR on software events. For example, when a complex syscall,
such as sys_bpf() and sys_perf_event_open(), returns -EINVAL, it is not obvious
what wen wrong. The branch stack at the return (on a kretprobe or fexit) could
give us additional information.
quoted
quoted
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Yes, it makes sense.
quoted
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
Is it possible that we have another LBR in-kernel user in addition to the current
BPF-LBR snapshot user, such as another BPF-LBR snapshot user or a LBR perf user ?
I think it is OK to have another user. We just need to capture the LBR entries.
In fact, we simply enable LBR by opening a perf_event on each CPU. So from the
kernel's point of view, the LBR is owned used by "another user".
quoted
In the intel_pmu_snapshot_[arch]_branch_stack(), what if there is a PMI or NMI handler
to be called before __intel_pmu_lbr_disable(), which means more branch instructions
(assuming we don't use the FREEZE_LBRS_ON_xxx capability)?
If we are unlucky and hit an NMI, we may get garbage data. The user will run the
test again.
quoted
How about try to disable LBR at the earliest possible time, before __intel_pmu_disable_all(false) ?
I am not sure which solution is the best here. On bare metal, current version works
fine (available in bpf-next tree).
quoted
quoted
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
Are you saying that you still need to call
__intel_pmu_pebs_disable_all() to maintain precision ?
I think we don't need pebs_disable_all. In the VM, pebs_disable_all will trigger
"unchecked MSR access error" warning. After removing it, the warning message is
gone. However, after we remove pebs_disable_all, we still see too many LBR entries
are flushed before LBR is stopped. Most of these new entries are in extable code.
I guess this is because the VM access these MSR differently.
Hi Song,
Thanks for your detailed input. I saw your workaround "if (is_hypervisor())" on the tree.
Even when the guest supports PEBS, this use case fails and the root cause is still
playing hide-and-seek with me. Just check with you to see if you get similar results
when the guest LBR behavior makes the test case fail like this:
serial_test_get_branch_snapshot:FAIL:find_looptest_in_lbr unexpected find_looptest_in_lbr: actual 0 <= expected 6
serial_test_get_branch_snapshot:FAIL:check_wasted_entries unexpected check_wasted_entries: actual 32 >= expected 10
#52 get_branch_snapshot:FAIL
Also, do you know or rough guess about how extable code relates to the test case ?
Sorry for the delayed response. I finally got some time to look into
this again. After disabling most debug configs, I managed to get it
work in the VM with a simple change as
Yes, most of the contaminated lbr records come from these guest symbols:
intel_pmu_snapshot_branch_stack
native_write_msr
trace_hardirqs_off
lockdep_hardirqs_off
__lock_acquire
mark_lock
migrate_disable
rcu_is_watching
bpf_get_branch_snapshot
__bpf_prog_enter
I think we're fine with the current guest LBR emulation, right?
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */-__intel_pmu_pebs_disable_all();__intel_pmu_lbr_disable();/* ... until here */return__intel_pmu_snapshot_branch_stack(entries,cnt,flags);
How do we inform the user of bpf_get_branch_snapshot in a reasonable way
that the lbr data will be inaccurate when using the debug kernel?
Is it better to check for mutual exclusion in code or to use the user documentation
to specify this part of the restriction? It affects the user experience.
Specifically, can we fix intel_pmu_snapshot_branch_stack in vm with the
change above?
At least it's a valid fix and we can start from this change.
On Oct 27, 2021, at 8:09 PM, Like Xu [off-list ref] wrote:
On 26/10/2021 3:09 pm, Song Liu wrote:
quoted
quoted
On Oct 9, 2021, at 2:03 AM, Like Xu [off-list ref] wrote:
On 9/10/2021 1:08 am, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 11:36 PM, Like Xu [off-list ref] wrote:
On 8/10/2021 1:46 pm, Song Liu wrote:
quoted
quoted
On Oct 7, 2021, at 8:34 PM, Like Xu [off-list ref] wrote:
On 30/9/2021 4:05 am, Song Liu wrote:
quoted
Hi Kan,
quoted
On Sep 29, 2021, at 9:35 AM, Liang, Kan [off-list ref] wrote:
quoted
quoted
quoted
- get confirmation that clearing GLOBAL_CTRL is suffient to supress
PEBS, in which case we can simply remove the PEBS_ENABLE clear.
How should we confirm this? Can we run some tests for this? Or do we
need hardware experts' input for this?
I'll put it on the list to ask the hardware people when I talk to them next. But
maybe Kan or Andi know without asking.
If the GLOBAL_CTRL is explicitly disabled, the counters do not count anymore.
It doesn't matter if PEBS is enabled or not.
See 6c1c07b33eb0 ("perf/x86/intel: Avoid unnecessary PEBS_ENABLE MSR
access in PMI "). We optimized the PMU handler base on it.
Thanks for these information!
IIUC, all we need is the following on top of bpf-next/master:
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */
If the value passed in is true, does it affect your use case?
quoted
- __intel_pmu_pebs_disable_all();
In that case, we can reuse "static __always_inline void intel_pmu_disable_all(void)"
regardless of whether PEBS is supported or enabled inside the guest and the host ?
quoted
__intel_pmu_lbr_disable();
How about using intel_pmu_lbr_disable_all() to cover Arch LBR?
We are using LBR without PMI, so there isn't any hardware mechanism to
stop the LBR, we have to stop it in software. There is always a delay
between the event triggers and the LBR is stopped. In this window,
Do you use counters for snapshot branch stack?
Can the assumption of "without PMI" be broken sine Intel does have
the hardware mechanism like "freeze LBR on counter overflow
(aka, DEBUGCTLMSR_FREEZE_LBRS_ON_PMI)" ?
We are capturing LBR on software events. For example, when a complex syscall,
such as sys_bpf() and sys_perf_event_open(), returns -EINVAL, it is not obvious
what wen wrong. The branch stack at the return (on a kretprobe or fexit) could
give us additional information.
quoted
quoted
the LBR is still running and old entries are being replaced by new entries.
We actually need the old entries before the triggering event, so the key
design goal here is to minimize the number of branch instructions between
the event triggers and the LBR is stopped.
Yes, it makes sense.
quoted
Here, both __intel_pmu_disable_all(false) and __intel_pmu_lbr_disable()
are used to optimize for this goal: the fewer branch instructions the
better.
Is it possible that we have another LBR in-kernel user in addition to the current
BPF-LBR snapshot user, such as another BPF-LBR snapshot user or a LBR perf user ?
I think it is OK to have another user. We just need to capture the LBR entries.
In fact, we simply enable LBR by opening a perf_event on each CPU. So from the
kernel's point of view, the LBR is owned used by "another user".
quoted
In the intel_pmu_snapshot_[arch]_branch_stack(), what if there is a PMI or NMI handler
to be called before __intel_pmu_lbr_disable(), which means more branch instructions
(assuming we don't use the FREEZE_LBRS_ON_xxx capability)?
If we are unlucky and hit an NMI, we may get garbage data. The user will run the
test again.
quoted
How about try to disable LBR at the earliest possible time, before __intel_pmu_disable_all(false) ?
I am not sure which solution is the best here. On bare metal, current version works
fine (available in bpf-next tree).
quoted
quoted
After removing __intel_pmu_pebs_disable_all() from
intel_pmu_snapshot_branch_stack(), we found quite a few LBR entries in
extable related code. With these entries, snapshot branch stack is not
Are you saying that you still need to call
__intel_pmu_pebs_disable_all() to maintain precision ?
I think we don't need pebs_disable_all. In the VM, pebs_disable_all will trigger
"unchecked MSR access error" warning. After removing it, the warning message is
gone. However, after we remove pebs_disable_all, we still see too many LBR entries
are flushed before LBR is stopped. Most of these new entries are in extable code.
I guess this is because the VM access these MSR differently.
Hi Song,
Thanks for your detailed input. I saw your workaround "if (is_hypervisor())" on the tree.
Even when the guest supports PEBS, this use case fails and the root cause is still
playing hide-and-seek with me. Just check with you to see if you get similar results
when the guest LBR behavior makes the test case fail like this:
serial_test_get_branch_snapshot:FAIL:find_looptest_in_lbr unexpected find_looptest_in_lbr: actual 0 <= expected 6
serial_test_get_branch_snapshot:FAIL:check_wasted_entries unexpected check_wasted_entries: actual 32 >= expected 10
#52 get_branch_snapshot:FAIL
Also, do you know or rough guess about how extable code relates to the test case ?
Sorry for the delayed response. I finally got some time to look into
this again. After disabling most debug configs, I managed to get it
work in the VM with a simple change as
Yes, most of the contaminated lbr records come from these guest symbols:
intel_pmu_snapshot_branch_stack
native_write_msr
trace_hardirqs_off
lockdep_hardirqs_off
__lock_acquire
mark_lock
migrate_disable
rcu_is_watching
bpf_get_branch_snapshot
__bpf_prog_enter
I think we're fine with the current guest LBR emulation, right?
Yes, current result looks good to me. But it does rely on the .config.
@@ -2209,7 +2209,6 @@ intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int/* must not have branches... */local_irq_save(flags);__intel_pmu_disable_all(false);/* we don't care about BTS */-__intel_pmu_pebs_disable_all();__intel_pmu_lbr_disable();/* ... until here */return__intel_pmu_snapshot_branch_stack(entries,cnt,flags);
How do we inform the user of bpf_get_branch_snapshot in a reasonable way
that the lbr data will be inaccurate when using the debug kernel?
Is it better to check for mutual exclusion in code or to use the user documentation
to specify this part of the restriction? It affects the user experience.
In uapi/linux/bpf.h, we have
* long bpf_get_branch_snapshot(void *entries, u32 size, u64 flags)
* Description
* Get branch trace from hardware engines like Intel LBR. The
* hardware engine is stopped shortly after the helper is
* called. Therefore, the user need to filter branch entries
* based on the actual use case. To capture branch trace
* before the trigger point of the BPF program, the helper
* should be called at the beginning of the BPF program.
I guess this is enough.
quoted
Specifically, can we fix intel_pmu_snapshot_branch_stack in vm with the
change above?
At least it's a valid fix and we can start from this change.