linux-kernel - Re: [PATCH 2/3] KVM: VMX: Use proper type-safe functions for vCPU => LBRs helpers

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: <5f93760c-cb93-2c58-11d7-f9ddef7f640c@gmail.com>
Date:   Fri, 29 Jul 2022 16:39:07 +0800
From:   Like Xu <like.xu.linux@...il.com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] KVM: VMX: Use proper type-safe functions for vCPU =>
 LBRs helpers

On 28/7/2022 7:34 am, Sean Christopherson wrote:
> Turn vcpu_to_lbr_desc() and vcpu_to_lbr_records() into functions in order
> to provide type safety, to document exactly what they return, and to

Considering the prevalence of similar practices, perhaps we (at least me)
need doc more benefits of "type safety" or the risks of not doing so.

> allow consuming the helpers in vmx.h.  Move the definitions as necessary
> (the macros "reference" to_vmx() before its definition).
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>   arch/x86/kvm/vmx/vmx.h | 26 +++++++++++++++++---------
>   1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> index 286c88e285ea..690421b7d26c 100644
> --- a/arch/x86/kvm/vmx/vmx.h
> +++ b/arch/x86/kvm/vmx/vmx.h
> @@ -6,6 +6,7 @@
>   
>   #include <asm/kvm.h>
>   #include <asm/intel_pt.h>
> +#include <asm/perf_event.h>
>   
>   #include "capabilities.h"
>   #include "kvm_cache_regs.h"
> @@ -91,15 +92,6 @@ union vmx_exit_reason {
>   	u32 full;
>   };
>   
> -#define vcpu_to_lbr_desc(vcpu) (&to_vmx(vcpu)->lbr_desc)
> -#define vcpu_to_lbr_records(vcpu) (&to_vmx(vcpu)->lbr_desc.records)

More targets can be found in the arch/x86/kvm/pmu.h:

#define vcpu_to_pmu(vcpu) (&(vcpu)->arch.pmu)
#define pmu_to_vcpu(pmu)  (container_of((pmu), struct kvm_vcpu, arch.pmu))
#define pmc_to_pmu(pmc)   (&(pmc)->vcpu->arch.pmu)

> -
> -void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu);
> -bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu);
> -
> -int intel_pmu_create_guest_lbr_event(struct kvm_vcpu *vcpu);
> -void vmx_passthrough_lbr_msrs(struct kvm_vcpu *vcpu);

Unrelated move, but no reason not to do so opportunistically.

> -
>   struct lbr_desc {
>   	/* Basic info about guest LBR records. */
>   	struct x86_pmu_lbr records;
> @@ -524,6 +516,22 @@ static inline struct vcpu_vmx *to_vmx(struct kvm_vcpu *vcpu)
>   	return container_of(vcpu, struct vcpu_vmx, vcpu);
>   }
>   
> +static inline struct lbr_desc *vcpu_to_lbr_desc(struct kvm_vcpu *vcpu)
> +{
> +	return &to_vmx(vcpu)->lbr_desc;
> +}
> +
> +static inline struct x86_pmu_lbr *vcpu_to_lbr_records(struct kvm_vcpu *vcpu)
> +{
> +	return &vcpu_to_lbr_desc(vcpu)->records;
> +}
> +
> +void intel_pmu_cross_mapped_check(struct kvm_pmu *pmu);
> +bool intel_pmu_lbr_is_enabled(struct kvm_vcpu *vcpu);
> +
> +int intel_pmu_create_guest_lbr_event(struct kvm_vcpu *vcpu);
> +void vmx_passthrough_lbr_msrs(struct kvm_vcpu *vcpu);
> +
>   static inline unsigned long vmx_get_exit_qual(struct kvm_vcpu *vcpu)
>   {
>   	struct vcpu_vmx *vmx = to_vmx(vcpu);