Re: [PATCH v7 2/8] KVM: VMX: Extend BUILD_CONTROLS_SHADOW macro to support 64-bit variation

[Zeng Guang]

On 4/1/2022 6:27 AM, Sean Christopherson wrote:
> On Fri, Mar 04, 2022, Zeng Guang wrote:
> > +#define BUILD_CONTROLS_SHADOW(lname, uname, bits)			\
> > +static inline								\
> > +void lname##_controls_set(struct vcpu_vmx *vmx, u##bits val)		\
> > +{									\
> > +	if (vmx->loaded_vmcs->controls_shadow.lname != val) {		\
> > +		vmcs_write##bits(uname, val);				\
> > +		vmx->loaded_vmcs->controls_shadow.lname = val;		\
> > +	}								\
> > +}									\
> > +static inline u##bits __##lname##_controls_get(struct loaded_vmcs *vmcs)\
> > +{									\
> > +	return vmcs->controls_shadow.lname;				\
> > +}									\
> > +static inline u##bits lname##_controls_get(struct vcpu_vmx *vmx)	\
> > +{									\
> > +	return __##lname##_controls_get(vmx->loaded_vmcs);		\
> > +}									\
> > +static inline								\
> Drop the newline, there's no need to split this across two lines.  Aligning the
> backslashes will mean they all poke past the 80 char soft limit, but that's totally
> ok.  The whole point of the line limit is to improve readability, and a trivial
> runover is much less painful than a split function declaration.  As a bonus, all
> the backslashes are aligned, have leading whitespace, and still land on a tab stop :-)
>
> #define BUILD_CONTROLS_SHADOW(lname, uname, bits)				\
> static inline void lname##_controls_set(struct vcpu_vmx *vmx, u##bits val)	\
> {										\
> 	if (vmx->loaded_vmcs->controls_shadow.lname != val) {			\
> 		vmcs_write##bits(uname, val);					\
> 		vmx->loaded_vmcs->controls_shadow.lname = val;			\
> 	}									\
> }										\
> static inline u##bits __##lname##_controls_get(struct loaded_vmcs *vmcs)	\
> {										\
> 	return vmcs->controls_shadow.lname;					\
> }										\
> static inline u##bits lname##_controls_get(struct vcpu_vmx *vmx)		\
> {										\
> 	return __##lname##_controls_get(vmx->loaded_vmcs);			\
> }										\
> static inline void lname##_controls_setbit(struct vcpu_vmx *vmx, u##bits val)	\
> {										\
> 	lname##_controls_set(vmx, lname##_controls_get(vmx) | val);		\
> }										\
> static inline void lname##_controls_clearbit(struct vcpu_vmx *vmx, u##bits val)	\
> {										\
> 	lname##_controls_set(vmx, lname##_controls_get(vmx) & ~val);		\
> }
>
> With that fixed,
>
> Reviewed-by: Sean Christopherson <seanjc@xxxxxxxxxx>

OK. I'll revise it.