Re: [kvm-unit-tests patch v6 05/18] x86: pmu: Enlarge cnt[] length to 48 in check_counters_many()

Mi, Dapeng
2025-02-18T09:24:47Z

On 2/15/2025 5:06 AM, Sean Christopherson wrote:
> On Sat, Sep 14, 2024, Dapeng Mi 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 48 once and
>> for all. Base on the layout of IA32_PERF_GLOBAL_CTRL and
>> IA32_PERF_GLOBAL_STATUS, 48 looks enough in near feature.
>>
>> Reviewed-by: Jim Mattson <jmattson@google.com>
>> 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 a0268db8..b4de2680 100644
>> --- a/x86/pmu.c
>> +++ b/x86/pmu.c
>> @@ -255,7 +255,7 @@ static void check_fixed_counters(void)
>>
>> static void check_counters_many(void)
>> {
>> - pmu_counter_t cnt[10];
>> + pmu_counter_t cnt[48];
> ARGH. Since the *entire* purpose of increasing the size is to guard against
> buffer overflow, add an assert that the loop doesn't overflow.

This is not only for ensuring no buffer overflow. As the commit message
says,  the counter number has already exceeded 10, such as SPR has 12
counters (8 GP + 4 fixed), and there would be more counters in later
platfroms. The aim of enlarging the array size is to ensure these counters
can be enabled and verified simultaneously.  48 may be too large and 32
should be fair enough. Thanks.

>
>> int i, n;
>>
>> for (i = 0, n = 0; n < pmu.nr_gp_counters; i++) {
>> --
>> 2.40.1
>>