[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <715a9591-a913-4b79-a07b-9599a67e60e7@linux.intel.com>
Date: Wed, 27 Mar 2024 14:40:25 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: Jim Mattson <jmattson@...gle.com>
Cc: Sean Christopherson <seanjc@...gle.com>,
Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, Zhenyu Wang <zhenyuw@...ux.intel.com>,
Zhang Xiong <xiong.y.zhang@...el.com>, Mingwei Zhang <mizhang@...gle.com>,
Like Xu <like.xu.linux@...il.com>, Jinrong Liang <cloudliang@...cent.com>,
Dapeng Mi <dapeng1.mi@...el.com>
Subject: Re: [kvm-unit-tests Patch v3 02/11] x86: pmu: Enlarge cnt[] length to
64 in check_counters_many()
On 3/26/2024 5:41 AM, Jim Mattson wrote:
> On Tue, Jan 2, 2024 at 7:09 PM Dapeng Mi <dapeng1.mi@...ux.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@...ux.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@...gle.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?
Powered by blists - more mailing lists