lkml.org 
[lkml]   [2021]   [Nov]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/3] KVM: x86: Introduce definitions to support static calls for kvm_pmu_ops
From
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
>>
>

\
 
 \ /
  Last update: 2021-11-08 10:31    [W:0.209 / U:0.024 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site