Re: [PATCH v7 04/12] KVM: arm64: Make vcpu flag macros more generic

[Marc Zyngier <maz@kernel.org>]

On Tue, 12 Nov 2024 10:37:03 +0000,
James Clark <james.clark@linaro.org> wrote:
> 
> Rename vcpu_* to kvm_* so that the same flags mechanism can be used in
> places other than vcpu without being confusing. Wherever macros are
> still related to vcpu like vcpu_get_flag() with hard coded v->arch, keep
> the vcpu_* name, otherwise change it.
> 
> Also move the "v->arch" access one macro higher for the same reason.
> 
> This will be used for moving flags to host_data in a later commit.
> 
> Signed-off-by: James Clark <james.clark@linaro.org>
> ---
>  arch/arm64/include/asm/kvm_host.h | 88 +++++++++++++++----------------
>  arch/arm64/kvm/hyp/exception.c    | 12 ++---
>  arch/arm64/kvm/inject_fault.c     |  4 +-
>  arch/arm64/kvm/mmio.c             | 10 ++--
>  4 files changed, 57 insertions(+), 57 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index f333b189fb43..34aa59f498c4 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -790,22 +790,22 @@ struct kvm_vcpu_arch {
>  /*
>   * Each 'flag' is composed of a comma-separated triplet:
>   *
> - * - the flag-set it belongs to in the vcpu->arch structure
> + * - the flag-set it belongs to in the structure pointed to by 'v'
>   * - the value for that flag
>   * - the mask for that flag
>   *
> - *  __vcpu_single_flag() builds such a triplet for a single-bit flag.
> - * unpack_vcpu_flag() extract the flag value from the triplet for
> + *  __kvm_single_flag() builds such a triplet for a single-bit flag.
> + * unpack_kvm_flag() extract the flag value from the triplet for
>   * direct use outside of the flag accessors.
>   */
> -#define __vcpu_single_flag(_set, _f)	_set, (_f), (_f)
> +#define __kvm_single_flag(_set, _f)	_set, (_f), (_f)
>  
>  #define __unpack_flag(_set, _f, _m)	_f
> -#define unpack_vcpu_flag(...)		__unpack_flag(__VA_ARGS__)
> +#define unpack_kvm_flag(...)		__unpack_flag(__VA_ARGS__)
>  
>  #define __build_check_flag(v, flagset, f, m)			\
>  	do {							\
> -		typeof(v->arch.flagset) *_fset;			\
> +		typeof(v.flagset) *_fset;			\
>  								\
>  		/* Check that the flags fit in the mask */	\
>  		BUILD_BUG_ON(HWEIGHT(m) != HWEIGHT((f) | (m)));	\
> @@ -813,11 +813,11 @@ struct kvm_vcpu_arch {
>  		BUILD_BUG_ON((sizeof(*_fset) * 8) <= __fls(m));	\
>  	} while (0)
>  
> -#define __vcpu_get_flag(v, flagset, f, m)			\
> +#define __kvm_get_flag(v, flagset, f, m)			\
>  	({							\
>  		__build_check_flag(v, flagset, f, m);		\
>  								\
> -		READ_ONCE(v->arch.flagset) & (m);		\
> +		READ_ONCE(v.flagset) & (m);		\
>  	})
>  
>  /*
> @@ -826,64 +826,64 @@ struct kvm_vcpu_arch {
>   */
>  #ifdef __KVM_NVHE_HYPERVISOR__
>  /* the nVHE hypervisor is always non-preemptible */
> -#define __vcpu_flags_preempt_disable()
> -#define __vcpu_flags_preempt_enable()
> +#define __kvm_flags_preempt_disable()
> +#define __kvm_flags_preempt_enable()
>  #else
> -#define __vcpu_flags_preempt_disable()	preempt_disable()
> -#define __vcpu_flags_preempt_enable()	preempt_enable()
> +#define __kvm_flags_preempt_disable()	preempt_disable()
> +#define __kvm_flags_preempt_enable()	preempt_enable()
>  #endif
>  
> -#define __vcpu_set_flag(v, flagset, f, m)			\
> +#define __kvm_set_flag(v, flagset, f, m)			\

Hell no. Never. The whole point of this naming is that we know what
this applies to. Here, you might as well have replaced 'vcpu' with
'carrot', and the result would be the same.

Not to mention the insane churn this generates.

So no, not happening.

	M.

-- 
Without deviation from the norm, progress is not possible.