Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <daa5e62d-34b4-6a05-584c-ec5baf9c9216@redhat.com>
Date:   Mon, 7 Nov 2022 19:34:49 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH v3 3/3] KVM: x86/pmu: Limit the maximum number of
 supported AMD GP counters
Content-Language: en-US
To:     Like Xu <like.xu.linux@gmail.com>,
        Sean Christopherson <seanjc@google.com>
Cc:     Jim Mattson <jmattson@google.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220919091008.60695-1-likexu@tencent.com>
 <20220919091008.60695-3-likexu@tencent.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
In-Reply-To: <20220919091008.60695-3-likexu@tencent.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/19/22 11:10, Like Xu wrote:
> @@ -506,6 +506,7 @@ struct kvm_pmc {
>   #define MSR_ARCH_PERFMON_PERFCTR_MAX	(MSR_ARCH_PERFMON_PERFCTR0 + KVM_INTEL_PMC_MAX_GENERIC - 1)
>   #define MSR_ARCH_PERFMON_EVENTSEL_MAX	(MSR_ARCH_PERFMON_EVENTSEL0 + KVM_INTEL_PMC_MAX_GENERIC - 1)
>   #define KVM_PMC_MAX_FIXED	3
> +#define KVM_AMD_PMC_MAX_GENERIC	AMD64_NUM_COUNTERS_CORE
>   struct kvm_pmu {

Even though the BUILD_BUG_ON prevents out-of-bounds accesses, this 
should be hardcoded to 6 to avoid mismatches with msrs_to_save_all[].

> +	BUILD_BUG_ON(AMD64_NUM_COUNTERS_CORE > KVM_AMD_PMC_MAX_GENERIC);

This should be KVM_AMD_PMC_MAX_GENERIC > AMD64_NUM_COUNTERS_CORE, not 
the opposite.

Fixed up and changed the commit message to follow:

     The AMD PerfMonV2 specification allows for a maximum of 16 GP
     counters, but currently only 6 pairs of MSRs are accepted by KVM.

     While AMD64_NUM_COUNTERS_CORE is already equal to 6, increasing
     without adjusting msrs_to_save_all[] could result in out-of-bounds
     accesses.  Therefore introduce a macro (named
     KVM_AMD_PMC_MAX_GENERIC) to refer to the number of counters
     supported by KVM.

Paolo