On Wed, Nov 03, 2021, Like Xu wrote:
> diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
> index 0db1887137d9..b6f08c719125 100644
> --- a/arch/x86/kvm/pmu.c
> +++ b/arch/x86/kvm/pmu.c
> @@ -50,6 +50,13 @@
> struct kvm_pmu_ops kvm_pmu_ops __read_mostly;
> EXPORT_SYMBOL_GPL(kvm_pmu_ops);
>
> +#define KVM_X86_PMU_OP(func) \
> + DEFINE_STATIC_CALL_NULL(kvm_x86_pmu_##func, \
> + *(((struct kvm_pmu_ops *)0)->func))
> +#define KVM_X86_PMU_OP_NULL KVM_X86_PMU_OP
More of a question for the existing code, what's the point of KVM_X86_OP_NULL?
AFAICT, it always resolves to KVM_X86_OP. Unless there's some magic I'm missing,
I vote we remove KVM_X86_OP_NULL and then not introduce KVM_X86_PMU_OP_NULL.
And I'm pretty sure it's useless, e.g. get_cs_db_l_bits is defined with the NULL
variant, but it's never NULL and its calls aren't guarded with anything. And if
KVM_X86_OP_NULL is intended to aid in documenting behavior, it's doing a pretty
miserable job of that :-)
> +#include <asm/kvm-x86-pmu-ops.h>
> +EXPORT_STATIC_CALL_GPL(kvm_x86_pmu_is_valid_msr);
I'll double down on my nVMX suggestion so that this export can be avoided.
> static void kvm_pmi_trigger_fn(struct irq_work *irq_work)
> {
> struct kvm_pmu *pmu = container_of(irq_work, struct kvm_pmu, irq_work);
> diff --git a/arch/x86/kvm/pmu.h b/arch/x86/kvm/pmu.h
> index b2fe135d395a..e5550d4acf14 100644
> --- a/arch/x86/kvm/pmu.h
> +++ b/arch/x86/kvm/pmu.h
> @@ -3,6 +3,8 @@
> #define __KVM_X86_PMU_H
>
> #include <linux/nospec.h>
> +#include <linux/static_call_types.h>
> +#include <linux/static_call.h>
>
> #define vcpu_to_pmu(vcpu) (&(vcpu)->arch.pmu)
> #define pmu_to_vcpu(pmu) (container_of((pmu), struct kvm_vcpu, arch.pmu))
> @@ -45,6 +47,19 @@ struct kvm_pmu_ops {
> void (*cleanup)(struct kvm_vcpu *vcpu);
> };
>
> +#define KVM_X86_PMU_OP(func) \
> + DECLARE_STATIC_CALL(kvm_x86_pmu_##func, *(((struct kvm_pmu_ops *)0)->func))
> +#define KVM_X86_PMU_OP_NULL KVM_X86_PMU_OP
> +#include <asm/kvm-x86-pmu-ops.h>
> +
> +static inline void kvm_pmu_ops_static_call_update(void)
> +{
> +#define KVM_X86_PMU_OP(func) \
> + static_call_update(kvm_x86_pmu_##func, kvm_pmu_ops.func)
> +#define KVM_X86_PMU_OP_NULL KVM_X86_PMU_OP
> +#include <asm/kvm-x86-pmu-ops.h>
> +}
As alluded to in patch 01, I'd prefer these go in kvm_ops_static_call_update()
to keep the static call magic somewhat contained.
> +
> static inline u64 pmc_bitmask(struct kvm_pmc *pmc)
> {
> struct kvm_pmu *pmu = pmc_to_pmu(pmc);
> --
> 2.33.0
>
