Re: [PATCH 2/3] KVM: x86: Introduce definitions to support static calls for kvm_pmu_ops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/11/2021 11:48 pm, Sean Christopherson wrote:
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?

The comment says:

 * KVM_X86_OP_NULL() can leave a NULL definition for the
 * case where there is no definition or a function name that
 * doesn't match the typical naming convention is supplied.

Does it help ?

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

This transitions will not be included in the next version. Open to you.

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.

Fine to me.


  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.

Thank and applied.


+
  static inline u64 pmc_bitmask(struct kvm_pmc *pmc)
  {
  	struct kvm_pmu *pmu = pmc_to_pmu(pmc);
--
2.33.0





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux