| Message ID | 20240103031409.2504051-3-dapeng1.mi@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pmu test bugs fix and improvements | expand |
On Tue, Jan 2, 2024 at 7:09 PM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote: > > Considering there are already 8 GP counters and 4 fixed counters on > latest Intel processors, like Sapphire Rapids. The original cnt[] array > length 10 is definitely not enough to cover all supported PMU counters on these > new processors even through currently KVM only supports 3 fixed counters > at most. This would cause out of bound memory access and may trigger > false alarm on PMU counter validation > > It's probably more and more GP and fixed counters are introduced in the > future and then directly extends the cnt[] array length to 64 once and > for all. > > Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> > --- > x86/pmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/x86/pmu.c b/x86/pmu.c > index 0def28695c70..a13b8a8398c6 100644 > --- a/x86/pmu.c > +++ b/x86/pmu.c > @@ -254,7 +254,7 @@ static void check_fixed_counters(void) > > static void check_counters_many(void) > { > - pmu_counter_t cnt[10]; > + pmu_counter_t cnt[64]; I think 48 should be sufficient, based on the layout of IA32_PERF_GLOBAL_CTRL and IA32_PERF_GLOBAL_STATUS. In any event, let's bump this up! Reviewed-by: Jim Mattson <jmattson@google.com>
On 3/26/2024 5:41 AM, Jim Mattson wrote: > On Tue, Jan 2, 2024 at 7:09 PM Dapeng Mi <dapeng1.mi@linux.intel.com> wrote: >> Considering there are already 8 GP counters and 4 fixed counters on >> latest Intel processors, like Sapphire Rapids. The original cnt[] array >> length 10 is definitely not enough to cover all supported PMU counters on these >> new processors even through currently KVM only supports 3 fixed counters >> at most. This would cause out of bound memory access and may trigger >> false alarm on PMU counter validation >> >> It's probably more and more GP and fixed counters are introduced in the >> future and then directly extends the cnt[] array length to 64 once and >> for all. >> >> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> >> --- >> x86/pmu.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/x86/pmu.c b/x86/pmu.c >> index 0def28695c70..a13b8a8398c6 100644 >> --- a/x86/pmu.c >> +++ b/x86/pmu.c >> @@ -254,7 +254,7 @@ static void check_fixed_counters(void) >> >> static void check_counters_many(void) >> { >> - pmu_counter_t cnt[10]; >> + pmu_counter_t cnt[64]; > I think 48 should be sufficient, based on the layout of > IA32_PERF_GLOBAL_CTRL and IA32_PERF_GLOBAL_STATUS. > > In any event, let's bump this up! > > Reviewed-by: Jim Mattson <jmattson@google.com> Yeah, would shrink to 48. Thanks Jim. I'm glad you have time to review this patchset. Not sure if you have time to review other patches?
diff --git a/x86/pmu.c b/x86/pmu.c index 0def28695c70..a13b8a8398c6 100644 --- a/x86/pmu.c +++ b/x86/pmu.c @@ -254,7 +254,7 @@ static void check_fixed_counters(void) static void check_counters_many(void) { - pmu_counter_t cnt[10]; + pmu_counter_t cnt[64]; int i, n; for (i = 0, n = 0; n < pmu.nr_gp_counters; i++) {
Considering there are already 8 GP counters and 4 fixed counters on latest Intel processors, like Sapphire Rapids. The original cnt[] array length 10 is definitely not enough to cover all supported PMU counters on these new processors even through currently KVM only supports 3 fixed counters at most. This would cause out of bound memory access and may trigger false alarm on PMU counter validation It's probably more and more GP and fixed counters are introduced in the future and then directly extends the cnt[] array length to 64 once and for all. Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com> --- x86/pmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)