linux-kernel - Re: [PATCH v3 3/3] KVM: x86/pmu: Add KVM_PMU_CALL() to simplify static calls of kvm_pmu

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]

Message-ID: <ZjQjYiwBg1jGmdUq@google.com>
Date: Thu, 2 May 2024 16:36:02 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Wei Wang <wei.w.wang@...el.com>
Cc: pbonzini@...hat.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/3] KVM: x86/pmu: Add KVM_PMU_CALL() to simplify
 static calls of kvm_pmu_ops

On Thu, Apr 25, 2024, Wei Wang wrote:
>  #define KVM_X86_CALL(func) static_call(kvm_x86_##func)
> +#define KVM_PMU_CALL(func) static_call(kvm_x86_pmu_##func)

..

> @@ -796,7 +796,7 @@ void kvm_pmu_init(struct kvm_vcpu *vcpu)
>  	struct kvm_pmu *pmu = vcpu_to_pmu(vcpu);
>  
>  	memset(pmu, 0, sizeof(*pmu));
> -	static_call(kvm_x86_pmu_init)(vcpu);
> +	KVM_PMU_CALL(init)(vcpu);
>  	kvm_pmu_refresh(vcpu);

I usually like macros to use CAPS so that they're clearly macros, but in this
case I find the code a bit jarring.  Essentially, I *want* my to be fooled into
thinking it's a function call, because that's really what it is.

So rather than all caps, what if we follow function naming style?  E.g.

	memset(pmu, 0, sizeof(*pmu));
	kvm_pmu_call(init)(vcpu);
	kvm_pmu_refresh(vcpu);

and

	if (lapic_in_kernel(vcpu)) {
		kvm_pmu_call(deliver_pmi)(vcpu);
		kvm_apic_local_deliver(vcpu->arch.apic, APIC_LVTPC);
	}

and

	switch (msr) {
	case MSR_CORE_PERF_GLOBAL_STATUS:
	case MSR_CORE_PERF_GLOBAL_CTRL:
	case MSR_CORE_PERF_GLOBAL_OVF_CTRL:
		return kvm_pmu_has_perf_global_ctrl(vcpu_to_pmu(vcpu));
	default:
		break;
	}
	return kvm_pmu_call(msr_idx_to_pmc)(vcpu, msr) ||
	       kvm_pmu_call(is_valid_msr)(vcpu, msr);

all are easier for my brain to parse.