linux-kernel - Re: [PATCH 1/2] KVM: x86/pmu: Define KVM_PMC_MAX

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 21 Jun 2024 06:48:43 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Dapeng Mi <dapeng1.mi@...ux.intel.com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Jim Mattson <jmattson@...gle.com>, Mingwei Zhang <mizhang@...gle.com>, 
	Xiong Zhang <xiong.y.zhang@...el.com>, Zhenyu Wang <zhenyuw@...ux.intel.com>, 
	Like Xu <like.xu.linux@...il.com>, Jinrong Liang <cloudliang@...cent.com>, 
	Dapeng Mi <dapeng1.mi@...el.com>
Subject: Re: [PATCH 1/2] KVM: x86/pmu: Define KVM_PMC_MAX_GENERIC for platform independence

On Fri, Jun 21, 2024, Dapeng Mi wrote:
> >> diff --git a/arch/x86/kvm/svm/pmu.c b/arch/x86/kvm/svm/pmu.c
> >> index 6e908bdc3310..2fca247798eb 100644
> >> --- a/arch/x86/kvm/svm/pmu.c
> >> +++ b/arch/x86/kvm/svm/pmu.c
> >> @@ -218,7 +218,7 @@ static void amd_pmu_init(struct kvm_vcpu *vcpu)
> >>  	int i;
> >>  
> >>  	BUILD_BUG_ON(KVM_AMD_PMC_MAX_GENERIC > AMD64_NUM_COUNTERS_CORE);
> >> -	BUILD_BUG_ON(KVM_AMD_PMC_MAX_GENERIC > INTEL_PMC_MAX_GENERIC);
> >> +	BUILD_BUG_ON(KVM_AMD_PMC_MAX_GENERIC > KVM_PMC_MAX_GENERIC);
> >>  
> >>  	for (i = 0; i < KVM_AMD_PMC_MAX_GENERIC ; i++) {
> >>  		pmu->gp_counters[i].type = KVM_PMC_GP;
> >> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c
> >> index fb5cbd6cbeff..a4b0bee04596 100644
> >> --- a/arch/x86/kvm/vmx/pmu_intel.c
> >> +++ b/arch/x86/kvm/vmx/pmu_intel.c
> >> @@ -570,6 +570,8 @@ static void intel_pmu_init(struct kvm_vcpu *vcpu)
> >>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
> >>  	struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu);
> >>  
> >> +	BUILD_BUG_ON(KVM_INTEL_PMC_MAX_GENERIC > KVM_PMC_MAX_GENERIC);
> > Rather than BUILD_BUG_ON() for both Intel and AMD, can't we just do?
> >
> > #define KVM_MAX_NR_GP_COUNTERS max(KVM_INTEL_PMC_MAX_GENERIC, KVM_AMD_PMC_MAX_GENERIC)
> 
> Actually I tried this, but compiler would report the below error since
> KVM_PMC_MAX_GENERIC would used to define the array
> gp_counters[KVM_PMC_MAX_GENERIC];
> 
> ./include/linux/minmax.h:48:50: error: braced-group within expression
> allowed only inside a function

Oh, right, the min/max macros are super fancy to deal with types.  How about this
(getting there over 2-3 patches)?

I don't love the "#define MAX", but I don't see a better option.  I suppose maybe
we should use __MAX or KVM_MAX since kvm_host.h is likely included outside of KVM?

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5c0415899a07..ce0c9191eb85 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -532,13 +532,18 @@ struct kvm_pmc {
        u64 current_config;
 };
 
+#define MAX(a, b) ((a) >= (b) ? (a) : (b))
+
 /* More counters may conflict with other existing Architectural MSRs */
-#define KVM_INTEL_PMC_MAX_GENERIC      8
-#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 MSR_ARCH_PERFMON_FIXED_CTR_MAX (MSR_ARCH_PERFMON_FIXED_CTR0 + KVM_PMC_MAX_FIXED - 1)
-#define KVM_AMD_PMC_MAX_GENERIC        6
+#define KVM_MAX_NR_INTEL_GP_COUNTERS   8
+#define KVM_MAX_NR_AMD_GP_COUNTERS     6
+#define KVM_MAX_NR_GP_COUNTERS         MAX(KVM_MAX_NR_INTEL_GP_COUNTERS, \
+                                           KVM_MAX_NR_AMD_GP_COUNTERS)
+
+#define KVM_MAX_NR_INTEL_FIXED_COUTNERS        3
+#define KVM_MAX_NR_AMD_FIXED_COUTNERS  0
+#define KVM_MAX_NR_FIXED_COUNTERS      MAX(KVM_MAX_NR_INTEL_FIXED_COUTNERS, \
+                                           KVM_MAX_NR_AMD_FIXED_COUTNERS)
 
 struct kvm_pmu {
        u8 version;
@@ -554,8 +559,8 @@ struct kvm_pmu {
        u64 global_status_rsvd;
        u64 reserved_bits;
        u64 raw_event_mask;
-       struct kvm_pmc gp_counters[KVM_INTEL_PMC_MAX_GENERIC];
-       struct kvm_pmc fixed_counters[KVM_PMC_MAX_FIXED];
+       struct kvm_pmc gp_counters[KVM_MAX_NR_GP_COUNTERS];
+       struct kvm_pmc fixed_counters[KVM_MAX_NR_FIXED_COUNTERS];
 
        /*
         * Overlay the bitmap with a 64-bit atomic so that all bits can be