* [PATCH v2 0/3] powerpc/perf: Fix vpa-pmu driver to report correct numbers
@ 2026-08-13 9:45 Gautam Menghani
2026-08-13 9:45 ` [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs Gautam Menghani
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Gautam Menghani @ 2026-08-13 9:45 UTC (permalink / raw)
To: maddy, atrajeev, npiggin, mpe, chleroy
Cc: Gautam Menghani, linuxppc-dev, kvm, linux-kernel
The vpa-pmu driver can sometimes report incorrect context switch latency
numbers:
$ perf stat -e vpa_pmu/l1_to_l2_lat/ --tid=2061 -I 1000
time counts unit events
1.001022517 18,446,744,073,708,177,408 vpa_pmu/l1_to_l2_lat/
2.002120674 345,092 vpa_pmu/l1_to_l2_lat/
3.003171582 406,342 vpa_pmu/l1_to_l2_lat/
4.004234838 340,836 vpa_pmu/l1_to_l2_lat/
These invalid numbers are observed because the vpa-pmu driver does not
handle the following scenarios:
1. The vCPU process gets rescheduled to a different host cpu.
2. The vCPU process is not running, and vpa_pmu_read() is called to read
counter values.
Fix the above cases by maintaining aggregate counter values inside the
vcpu->arch struct and using them to report data.
changes in v2:
1. Fix the output for "perf stat --cpu ..."
2. Add stable and fixes tags
Gautam Menghani (3):
KVM: PPC: Book3S HV: Maintain aggregate context switch values for
vCPUs
powerpc/perf: Use the aggregate context switch values from vcpu struct
powerpc/perf: Update prev_count of event to get accurate values
arch/powerpc/include/asm/kvm_book3s_64.h | 3 --
arch/powerpc/kvm/book3s_hv.c | 51 ++----------------------
arch/powerpc/perf/vpa-pmu.c | 25 +++++++++---
3 files changed, 22 insertions(+), 57 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs
2026-08-13 9:45 [PATCH v2 0/3] powerpc/perf: Fix vpa-pmu driver to report correct numbers Gautam Menghani
@ 2026-08-13 9:45 ` Gautam Menghani
2026-08-13 10:03 ` sashiko-bot
2026-08-13 9:45 ` [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct Gautam Menghani
2026-08-13 9:45 ` [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values Gautam Menghani
2 siblings, 1 reply; 7+ messages in thread
From: Gautam Menghani @ 2026-08-13 9:45 UTC (permalink / raw)
To: maddy, atrajeev, npiggin, mpe, chleroy
Cc: Gautam Menghani, linuxppc-dev, kvm, linux-kernel, stable
The variables in vcpu->arch currently store the latest values read from
PACA when the vCPU exits. Change this to instead store aggregate values
so that every vCPU has its own record of total context switch latencies
it has accumulated. This is essential for the vpa-pmu driver to report
correct numbers.
Fixes: 176cda0619b6 ("powerpc/perf: Add perf interface to expose vpa counters")
Cc: <stable@vger.kernel.org>
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
v1 -> v2:
1. Add stable/fixes tags
arch/powerpc/kvm/book3s_hv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 61dbeea317f3..342168b8bfc8 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4163,12 +4163,12 @@ static void do_trace_nested_cs_time(struct kvm_vcpu *vcpu)
trace_kvmppc_vcpu_stats(vcpu, l1_to_l2_ns - *l1_to_l2_cs_ptr,
l2_to_l1_ns - *l2_to_l1_cs_ptr,
l2_runtime_ns - *l2_runtime_agg_ptr);
+ vcpu->arch.l1_to_l2_cs += (l1_to_l2_ns - *l1_to_l2_cs_ptr);
+ vcpu->arch.l2_to_l1_cs += (l2_to_l1_ns - *l2_to_l1_cs_ptr);
+ vcpu->arch.l2_runtime_agg += (l2_runtime_ns - *l2_runtime_agg_ptr);
*l1_to_l2_cs_ptr = l1_to_l2_ns;
*l2_to_l1_cs_ptr = l2_to_l1_ns;
*l2_runtime_agg_ptr = l2_runtime_ns;
- vcpu->arch.l1_to_l2_cs = l1_to_l2_ns;
- vcpu->arch.l2_to_l1_cs = l2_to_l1_ns;
- vcpu->arch.l2_runtime_agg = l2_runtime_ns;
}
u64 kvmhv_get_l1_to_l2_cs_time(void)
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct
2026-08-13 9:45 [PATCH v2 0/3] powerpc/perf: Fix vpa-pmu driver to report correct numbers Gautam Menghani
2026-08-13 9:45 ` [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs Gautam Menghani
@ 2026-08-13 9:45 ` Gautam Menghani
2026-08-13 9:58 ` sashiko-bot
2026-08-13 9:45 ` [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values Gautam Menghani
2 siblings, 1 reply; 7+ messages in thread
From: Gautam Menghani @ 2026-08-13 9:45 UTC (permalink / raw)
To: maddy, atrajeev, npiggin, mpe, chleroy
Cc: Gautam Menghani, linuxppc-dev, kvm, linux-kernel, stable
The vpa-pmu driver reports incorrect numbers in 2 scenarios:
1. The vCPU process gets rescheduled to a different host cpu
- Incorrect numbers are observed here because the PACA is per-host cpu
resource, and the KVM vCPUs can be rescheduled to different
host cpus. This causes the vpa-pmu driver to subtract wrong values
when vCPUs are rescheduled.
2. The vCPU is not running when vpa_pmu_read() is called.
- In this case get_counter_data() returns 0, and this can result in
negative numbers getting reported.
Fix the above issues by using the aggregate values from the vcpu
structure to capture and report the difference in counter values.
Fixes: 176cda0619b6 ("powerpc/perf: Add perf interface to expose vpa counters")
Cc: <stable@vger.kernel.org>
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
v1 -> v2:
1. Fix the output for "perf stat --cpu ..."
2. Add stable/fixes tags
arch/powerpc/include/asm/kvm_book3s_64.h | 3 --
arch/powerpc/kvm/book3s_hv.c | 45 ------------------------
arch/powerpc/perf/vpa-pmu.c | 24 +++++++++----
3 files changed, 18 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index b936e174eefd..f620e3126d68 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -691,9 +691,6 @@ void kvmhv_set_l2_counters_status(int cpu, bool status);
u64 kvmhv_get_l1_to_l2_cs_time(void);
u64 kvmhv_get_l2_to_l1_cs_time(void);
u64 kvmhv_get_l2_runtime_agg(void);
-u64 kvmhv_get_l1_to_l2_cs_time_vcpu(void);
-u64 kvmhv_get_l2_to_l1_cs_time_vcpu(void);
-u64 kvmhv_get_l2_runtime_agg_vcpu(void);
#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 342168b8bfc8..b9285b7f1fed 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4189,51 +4189,6 @@ u64 kvmhv_get_l2_runtime_agg(void)
}
EXPORT_SYMBOL(kvmhv_get_l2_runtime_agg);
-u64 kvmhv_get_l1_to_l2_cs_time_vcpu(void)
-{
- struct kvm_vcpu *vcpu;
- struct kvm_vcpu_arch *arch;
-
- vcpu = local_paca->kvm_hstate.kvm_vcpu;
- if (vcpu) {
- arch = &vcpu->arch;
- return arch->l1_to_l2_cs;
- } else {
- return 0;
- }
-}
-EXPORT_SYMBOL(kvmhv_get_l1_to_l2_cs_time_vcpu);
-
-u64 kvmhv_get_l2_to_l1_cs_time_vcpu(void)
-{
- struct kvm_vcpu *vcpu;
- struct kvm_vcpu_arch *arch;
-
- vcpu = local_paca->kvm_hstate.kvm_vcpu;
- if (vcpu) {
- arch = &vcpu->arch;
- return arch->l2_to_l1_cs;
- } else {
- return 0;
- }
-}
-EXPORT_SYMBOL(kvmhv_get_l2_to_l1_cs_time_vcpu);
-
-u64 kvmhv_get_l2_runtime_agg_vcpu(void)
-{
- struct kvm_vcpu *vcpu;
- struct kvm_vcpu_arch *arch;
-
- vcpu = local_paca->kvm_hstate.kvm_vcpu;
- if (vcpu) {
- arch = &vcpu->arch;
- return arch->l2_runtime_agg;
- } else {
- return 0;
- }
-}
-EXPORT_SYMBOL(kvmhv_get_l2_runtime_agg_vcpu);
-
#else
int kvmhv_get_l2_counters_status(void)
{
diff --git a/arch/powerpc/perf/vpa-pmu.c b/arch/powerpc/perf/vpa-pmu.c
index bff4cfab7b94..e79d98447c74 100644
--- a/arch/powerpc/perf/vpa-pmu.c
+++ b/arch/powerpc/perf/vpa-pmu.c
@@ -91,7 +91,7 @@ static int vpa_pmu_event_init(struct perf_event *event)
return 0;
}
-static unsigned long get_counter_data(struct perf_event *event)
+static unsigned long get_counter_data(struct kvm_vcpu *vcpu, struct perf_event *event)
{
unsigned int config = event->attr.config;
u64 data;
@@ -99,19 +99,19 @@ static unsigned long get_counter_data(struct perf_event *event)
switch (config) {
case L1_TO_L2_CS_LAT:
if (event->attach_state & PERF_ATTACH_TASK)
- data = kvmhv_get_l1_to_l2_cs_time_vcpu();
+ data = vcpu->arch.l1_to_l2_cs;
else
data = kvmhv_get_l1_to_l2_cs_time();
break;
case L2_TO_L1_CS_LAT:
if (event->attach_state & PERF_ATTACH_TASK)
- data = kvmhv_get_l2_to_l1_cs_time_vcpu();
+ data = vcpu->arch.l2_to_l1_cs;
else
data = kvmhv_get_l2_to_l1_cs_time();
break;
case L2_RUNTIME_AGG:
if (event->attach_state & PERF_ATTACH_TASK)
- data = kvmhv_get_l2_runtime_agg_vcpu();
+ data = vcpu->arch.l2_runtime_agg;
else
data = kvmhv_get_l2_runtime_agg();
break;
@@ -126,21 +126,33 @@ static unsigned long get_counter_data(struct perf_event *event)
static int vpa_pmu_add(struct perf_event *event, int flags)
{
u64 data;
+ struct kvm_vcpu *vcpu;
+ vcpu = local_paca->kvm_hstate.kvm_vcpu;
+ if (!vcpu)
+ goto out;
+
+ event->pmu_private = vcpu;
kvmhv_set_l2_counters_status(smp_processor_id(), true);
- data = get_counter_data(event);
+ data = get_counter_data(vcpu, event);
local64_set(&event->hw.prev_count, data);
+out:
return 0;
}
static void vpa_pmu_read(struct perf_event *event)
{
u64 prev_data, new_data, final_data;
+ struct kvm_vcpu *vcpu;
+
+ vcpu = (struct kvm_vcpu *) event->pmu_private;
+ if (!vcpu)
+ return;
prev_data = local64_read(&event->hw.prev_count);
- new_data = get_counter_data(event);
+ new_data = get_counter_data(vcpu, event);
final_data = new_data - prev_data;
local64_add(final_data, &event->count);
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values
2026-08-13 9:45 [PATCH v2 0/3] powerpc/perf: Fix vpa-pmu driver to report correct numbers Gautam Menghani
2026-08-13 9:45 ` [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs Gautam Menghani
2026-08-13 9:45 ` [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct Gautam Menghani
@ 2026-08-13 9:45 ` Gautam Menghani
2026-08-13 10:01 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Gautam Menghani @ 2026-08-13 9:45 UTC (permalink / raw)
To: maddy, atrajeev, npiggin, mpe, chleroy
Cc: Gautam Menghani, linuxppc-dev, kvm, linux-kernel, stable
When vpa_pmu_read() is called twice while a process is running on a cpu,
the "event->hw.prev_count" variable returns a stale value on the 2nd
read which results in a larger value getting added to event->count.
Fix this by updating the event->hw.prev_count every time vpa_pmu_read()
is called.
Fixes: 176cda0619b6 ("powerpc/perf: Add perf interface to expose vpa counters")
Cc: <stable@vger.kernel.org>
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
v1 -> v2:
1. Add stable/fixes tags.
arch/powerpc/perf/vpa-pmu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/perf/vpa-pmu.c b/arch/powerpc/perf/vpa-pmu.c
index e79d98447c74..efc39f71553b 100644
--- a/arch/powerpc/perf/vpa-pmu.c
+++ b/arch/powerpc/perf/vpa-pmu.c
@@ -156,6 +156,7 @@ static void vpa_pmu_read(struct perf_event *event)
final_data = new_data - prev_data;
local64_add(final_data, &event->count);
+ local64_set(&event->hw.prev_count, new_data);
}
static void vpa_pmu_del(struct perf_event *event, int flags)
--
2.54.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct
2026-08-13 9:45 ` [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct Gautam Menghani
@ 2026-08-13 9:58 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-13 9:58 UTC (permalink / raw)
To: Gautam Menghani; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Task-bound PMU events cache a stale vCPU pointer, resulting in tracking the wrong vCPU.
- [High] CPU-wide and system-wide PMU events are broken and silently report 0 if added when no guest is running.
- [High] Uninitialized event state on task migration leaks stale vCPU pointers.
- [Medium] Unbalanced disablement of the global hardware PMU counters.
Pre-existing issues:
- [Critical] Dereferencing `kvm_hstate.kvm_vcpu` leads to a Use-After-Free if the previously running VM has been destroyed.
--
--- Patch [2]: [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct ---
commit e328e1f6c8baf0140410c1983f559e87d1d0efdc
Author: Gautam Menghani <gautam@linux.ibm.com>
powerpc/perf: Use the aggregate context switch values from vcpu struct
This commit aims to fix inaccurate vpa-pmu counter reporting during vCPU
migrations and when vCPUs are not running by fetching aggregate values
from the vcpu struct instead of relying on the host cpu's PACA.
> diff --git a/arch/powerpc/perf/vpa-pmu.c b/arch/powerpc/perf/vpa-pmu.c
> --- a/arch/powerpc/perf/vpa-pmu.c
> +++ b/arch/powerpc/perf/vpa-pmu.c
[ ... ]
> @@ -99,19 +99,19 @@ static unsigned long get_counter_data(struct perf_event *event)
> switch (config) {
> case L1_TO_L2_CS_LAT:
> if (event->attach_state & PERF_ATTACH_TASK)
> - data = kvmhv_get_l1_to_l2_cs_time_vcpu();
> + data = vcpu->arch.l1_to_l2_cs;
[Severity: Critical]
This is a pre-existing issue, but does dereferencing the vcpu pointer here
lead to a use-after-free if the previously running VM has been destroyed?
When KVM exits a guest, it does not clear local_paca->kvm_hstate.kvm_vcpu,
leaving a dangling pointer in the host's PACA.
The PMU driver reads this dangling pointer in vpa_pmu_add() and permanently
caches it in event->pmu_private. Subsequent calls to vpa_pmu_read() pass
this cached pointer to get_counter_data(), which dereferences the freed
memory here.
> else
> data = kvmhv_get_l1_to_l2_cs_time();
> break;
[ ... ]
> @@ -126,21 +126,33 @@ static int vpa_pmu_add(struct perf_event *event, int flags)
> {
> u64 data;
> + struct kvm_vcpu *vcpu;
>
> + vcpu = local_paca->kvm_hstate.kvm_vcpu;
> + if (!vcpu)
> + goto out;
[Severity: High]
Does this early exit break CPU-wide and system-wide profiling?
If a user initiates a CPU-wide profiling session (e.g. perf stat -a) and no
guest is actively running on that specific CPU at that moment, vcpu is NULL.
The function exits early returning 0 without initializing event->pmu_private
or event->hw.prev_count.
Subsequent calls to vpa_pmu_read() check if (!vcpu) and immediately return,
silently leaving the event counter permanently at 0.
[Severity: High]
Could this early exit also leak state from a previous CPU when a task
migrates?
When a task with a vpa_pmu event migrates to a new CPU, perf core calls
pmu->del on the old CPU and pmu->add on the new CPU. If vcpu is NULL on the
new CPU, it returns 0 (success) but jumps to out without updating or
clearing event->pmu_private.
The event silently retains the old vcpu pointer and prev_count from the
previous CPU, resulting in corrupted profiling data calculated against an
unrelated CPU's context.
[Severity: Medium]
Are the global hardware PMU counters skipped here on early bailout, leading
to an unbalanced disablement later?
If a user creates a PMU event on a CPU where no vCPU is active, this early
exit skips calling kvmhv_set_l2_counters_status(cpu, true).
However, when the event is deleted or scheduled out, vpa_pmu_del()
unconditionally calls kvmhv_set_l2_counters_status(cpu, false). This
unbalanced behavior forcefully disables the hardware counters even when they
were never enabled, which could break legitimate concurrent KVM tracing
sessions.
> +
> + event->pmu_private = vcpu;
[Severity: High]
Is it possible caching the vcpu pointer here results in tracking the wrong
vCPU for task-bound events?
When a user creates a task-bound vpa_pmu perf event, the core perf
subsystem calls vpa_pmu_add() during perf_event_context_sched_in(). At this
point, the task has been scheduled but has not yet invoked the KVM_RUN
ioctl.
Because of this, local_paca->kvm_hstate.kvm_vcpu does not point to the
incoming task's vCPU; it contains a stale pointer to the last vCPU that
executed on this physical host CPU. Caching this pointer permanently means
the event tracks unrelated data.
> kvmhv_set_l2_counters_status(smp_processor_id(), true);
>
> - data = get_counter_data(event);
> + data = get_counter_data(vcpu, event);
> local64_set(&event->hw.prev_count, data);
>
> +out:
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813094535.10083-1-gautam@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values
2026-08-13 9:45 ` [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values Gautam Menghani
@ 2026-08-13 10:01 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-13 10:01 UTC (permalink / raw)
To: Gautam Menghani; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] Missing atomic cmpxchg loop in vpa_pmu_read() when updating prev_count.
Pre-existing issues:
- [High] The VPA PMU `add` callback stores a raw pointer to `struct kvm_vcpu` in `event->pmu_private` without reference counting, leading to a Use-After-Free.
--
--- Patch [3]: [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values ---
commit 7c392d724ba481a5282a90aed95397fac4533493
Author: Gautam Menghani <gautam@linux.ibm.com>
powerpc/perf: Update prev_count of event to get accurate values
This patch updates the VPA PMU read function to keep event->hw.prev_count
current. This prevents stale values from being returned on consecutive reads
while a process is running on a CPU.
> diff --git a/arch/powerpc/perf/vpa-pmu.c b/arch/powerpc/perf/vpa-pmu.c
> index e79d98447c743..efc39f71553bf 100644
> --- a/arch/powerpc/perf/vpa-pmu.c
> +++ b/arch/powerpc/perf/vpa-pmu.c
> @@ -156,6 +156,7 @@ static void vpa_pmu_read(struct perf_event *event)
> final_data = new_data - prev_data;
>
> local64_add(final_data, &event->count);
> + local64_set(&event->hw.prev_count, new_data);
[Severity: Medium]
Is it safe to use local64_set() here without a local64_cmpxchg() loop?
If vpa_pmu_read() is interrupted by an NMI (such as a hardware PMI) and a BPF
program attached to the NMI reads the same event via bpf_perf_event_read(),
the nested read will calculate its delta and update prev_count.
When the outer vpa_pmu_read() resumes, it could calculate its delta using the
old prev_data (resulting in double-counting the inner delta) and then
non-atomically overwrite prev_count.
Should this use a local64_cmpxchg() loop to safely update prev_count?
> }
>
> static void vpa_pmu_del(struct perf_event *event, int flags)
[Severity: High]
This is a pre-existing issue, but does this code leave a stale vcpu pointer
in event->pmu_private that can lead to a use-after-free?
Looking at vpa_pmu_add(), a raw pointer to struct kvm_vcpu is stored without
taking a reference:
arch/powerpc/perf/vpa-pmu.c:vpa_pmu_add() {
...
vcpu = local_paca->kvm_hstate.kvm_vcpu;
if (!vcpu)
goto out;
...
event->pmu_private = vcpu;
}
When the thread context-switches out, vpa_pmu_del() stops the event but
does not clear event->pmu_private to NULL. When the thread is scheduled
back in outside of KVM, vpa_pmu_add() sees the vcpu is NULL and returns
early, leaving the event active with the stale vcpu pointer.
If the VM is then destroyed and the vcpu freed, would a subsequent thread
exit or user read() calling vpa_pmu_read() trigger a use-after-free when
dereferencing vcpu->arch.l1_to_l2_cs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813094535.10083-1-gautam@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs
2026-08-13 9:45 ` [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs Gautam Menghani
@ 2026-08-13 10:03 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-13 10:03 UTC (permalink / raw)
To: Gautam Menghani; +Cc: kvm
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch breaks task-bound VPA PMU events by subtracting independent VCPU aggregates, leading to severe underflows.
Pre-existing issues:
- [Critical] Pre-existing Use-After-Free (UAF) and cross-VM data leak via stale `kvm_hstate.kvm_vcpu` pointer in VPA PMU callbacks.
--
--- Patch [1]: [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs ---
commit b8e49dde6d70a4c423cf40d03275e97149e3916e
Author: Gautam Menghani <gautam@linux.ibm.com>
KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs
This commit changes the vCPU context switch latency variables in vcpu->arch
to store aggregate values rather than the latest snapshots read from PACA.
The goal is to provide each vCPU with its own record of total context
switch latencies for the vpa-pmu driver.
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 61dbeea317f3b..342168b8bfc86 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4163,12 +4163,12 @@ static void do_trace_nested_cs_time(struct kvm_vcpu *vcpu)
> trace_kvmppc_vcpu_stats(vcpu, l1_to_l2_ns - *l1_to_l2_cs_ptr,
> l2_to_l1_ns - *l2_to_l1_cs_ptr,
> l2_runtime_ns - *l2_runtime_agg_ptr);
> + vcpu->arch.l1_to_l2_cs += (l1_to_l2_ns - *l1_to_l2_cs_ptr);
> + vcpu->arch.l2_to_l1_cs += (l2_to_l1_ns - *l2_to_l1_cs_ptr);
> + vcpu->arch.l2_runtime_agg += (l2_runtime_ns - *l2_runtime_agg_ptr);
[Severity: Critical]
This is a pre-existing issue, but does relying on kvm_hstate.kvm_vcpu expose
a use-after-free and cross-VM data leak here?
When a task-bound VPA PMU event schedules in, vpa_pmu_add() calls into KVM
which reads the vCPU from the physical CPU's PACA:
arch/powerpc/kvm/book3s_hv.c:kvmhv_get_l1_to_l2_cs_time_vcpu() {
...
vcpu = local_paca->kvm_hstate.kvm_vcpu;
if (vcpu) {
arch = &vcpu->arch;
return arch->l1_to_l2_cs;
...
}
Since local_paca->kvm_hstate.kvm_vcpu is set during guest entry in
kvmhv_run_single_vcpu() but never cleared on exit, it remains pointing to
the last vCPU that executed on this physical CPU.
If that vCPU belonged to a different VM, its context switch data is leaked.
If the VM was destroyed, wouldn't this dereference a freed vCPU pointer?
[Severity: High]
Will changing these fields to accumulate independent aggregates cause massive
underflows in the PMU driver?
Before this patch, these fields stored a snapshot of the global monotonic
timer. Now they store independent per-vCPU aggregates.
Because of the stale local_paca pointer issue mentioned above, vpa_pmu_add()
records the previous vCPU's aggregate as the starting baseline:
arch/powerpc/perf/vpa-pmu.c:vpa_pmu_add() {
...
data = get_counter_data(event);
local64_set(&event->hw.prev_count, data);
...
}
Later, vpa_pmu_read() subtracts this from the current vCPU's aggregate:
arch/powerpc/perf/vpa-pmu.c:vpa_pmu_read() {
...
final_data = new_data - prev_data;
local64_add(final_data, &event->count);
}
Since new_data and prev_data are now completely unrelated aggregates from
different vCPUs rather than two points on a shared global timeline, won't
this subtraction produce garbage values and severe underflows?
> *l1_to_l2_cs_ptr = l1_to_l2_ns;
> *l2_to_l1_cs_ptr = l2_to_l1_ns;
> *l2_runtime_agg_ptr = l2_runtime_ns;
> - vcpu->arch.l1_to_l2_cs = l1_to_l2_ns;
> - vcpu->arch.l2_to_l1_cs = l2_to_l1_ns;
> - vcpu->arch.l2_runtime_agg = l2_runtime_ns;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260813094535.10083-1-gautam@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-13 10:03 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 9:45 [PATCH v2 0/3] powerpc/perf: Fix vpa-pmu driver to report correct numbers Gautam Menghani
2026-08-13 9:45 ` [PATCH v2 1/3] KVM: PPC: Book3S HV: Maintain aggregate context switch values for vCPUs Gautam Menghani
2026-08-13 10:03 ` sashiko-bot
2026-08-13 9:45 ` [PATCH v2 2/3] powerpc/perf: Use the aggregate context switch values from vcpu struct Gautam Menghani
2026-08-13 9:58 ` sashiko-bot
2026-08-13 9:45 ` [PATCH v2 3/3] powerpc/perf: Update prev_count of event to get accurate values Gautam Menghani
2026-08-13 10:01 ` sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.