Re: [PATCH 3/5] KVM: x86: Clean up redundant ROL16(val, n) macro definition

[Paolo Bonzini]

On 09/08/21 11:34, Like Xu wrote:
> From: Like Xu <likexu@xxxxxxxxxxx>
>
> The ROL16(val, n) macro is repeatedly defined in several vmcs-related
> files, and it has never been used outside the KVM context.
>
> Let's move it to vmcs.h without any intended functional changes.
>
> Signed-off-by: Like Xu <likexu@xxxxxxxxxxx>
> ---
>   arch/x86/kvm/vmx/evmcs.c  | 1 -
>   arch/x86/kvm/vmx/evmcs.h  | 4 ----
>   arch/x86/kvm/vmx/vmcs.h   | 2 ++
>   arch/x86/kvm/vmx/vmcs12.c | 1 -
>   arch/x86/kvm/vmx/vmcs12.h | 4 ----
>   5 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/evmcs.c b/arch/x86/kvm/vmx/evmcs.c
> index 896b2a50b4aa..0dab1b7b529f 100644
> --- a/arch/x86/kvm/vmx/evmcs.c
> +++ b/arch/x86/kvm/vmx/evmcs.c
> @@ -14,7 +14,6 @@ DEFINE_STATIC_KEY_FALSE(enable_evmcs);
>   
>   #if IS_ENABLED(CONFIG_HYPERV)
>   
> -#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
>   #define EVMCS1_OFFSET(x) offsetof(struct hv_enlightened_vmcs, x)
>   #define EVMCS1_FIELD(number, name, clean_field)[ROL16(number, 6)] = \
>   		{EVMCS1_OFFSET(name), clean_field}
> diff --git a/arch/x86/kvm/vmx/evmcs.h b/arch/x86/kvm/vmx/evmcs.h
> index 2ec9b46f0d0c..152ab0aa82cf 100644
> --- a/arch/x86/kvm/vmx/evmcs.h
> +++ b/arch/x86/kvm/vmx/evmcs.h
> @@ -73,8 +73,6 @@ struct evmcs_field {
>   extern const struct evmcs_field vmcs_field_to_evmcs_1[];
>   extern const unsigned int nr_evmcs_1_fields;
>   
> -#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
> -
>   static __always_inline int get_evmcs_offset(unsigned long field,
>   					    u16 *clean_field)
>   {
> @@ -95,8 +93,6 @@ static __always_inline int get_evmcs_offset(unsigned long field,
>   	return evmcs_field->offset;
>   }
>   
> -#undef ROL16
> -
>   static inline void evmcs_write64(unsigned long field, u64 value)
>   {
>   	u16 clean_field;
> diff --git a/arch/x86/kvm/vmx/vmcs.h b/arch/x86/kvm/vmx/vmcs.h
> index 4b9957e2bf5b..6e5de2e2b0da 100644
> --- a/arch/x86/kvm/vmx/vmcs.h
> +++ b/arch/x86/kvm/vmx/vmcs.h
> @@ -11,6 +11,8 @@
>   
>   #include "capabilities.h"
>   
> +#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
> +
>   struct vmcs_hdr {
>   	u32 revision_id:31;
>   	u32 shadow_vmcs:1;
> diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
> index d9f5d7c56ae3..cab6ba7a5005 100644
> --- a/arch/x86/kvm/vmx/vmcs12.c
> +++ b/arch/x86/kvm/vmx/vmcs12.c
> @@ -2,7 +2,6 @@
>   
>   #include "vmcs12.h"
>   
> -#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
>   #define VMCS12_OFFSET(x) offsetof(struct vmcs12, x)
>   #define FIELD(number, name)	[ROL16(number, 6)] = VMCS12_OFFSET(name)
>   #define FIELD64(number, name)						\
> diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h
> index 5e0e1b39f495..2a45f026ee11 100644
> --- a/arch/x86/kvm/vmx/vmcs12.h
> +++ b/arch/x86/kvm/vmx/vmcs12.h
> @@ -364,8 +364,6 @@ static inline void vmx_check_vmcs12_offsets(void)
>   extern const unsigned short vmcs_field_to_offset_table[];
>   extern const unsigned int nr_vmcs12_fields;
>   
> -#define ROL16(val, n) ((u16)(((u16)(val) << (n)) | ((u16)(val) >> (16 - (n)))))
> -
>   static inline short vmcs_field_to_offset(unsigned long field)
>   {
>   	unsigned short offset;
> @@ -385,8 +383,6 @@ static inline short vmcs_field_to_offset(unsigned long field)
>   	return offset;
>   }
>   
> -#undef ROL16
> -
>   static inline u64 vmcs12_read_any(struct vmcs12 *vmcs12, unsigned long field,
>   				  u16 offset)
>   {

Queued, thanks.

Paolo