Re: [PATCH v4 4/5] KVM: SVM: Turn DEBUGCTL_RESERVED_BITS into a helper

[Sean Christopherson <seanjc@google.com>]

On Tue, Jul 21, 2026, Shivansh Dhiman wrote:
> Replace the static DEBUGCTL_RESERVED_BITS macro with a helper,
> svm_get_debugctl_reserved_bits(), and plumb the vCPU into
> svm_copy_vmrun_state() so it can be passed to the helper.
> 
> The vCPU argument is currently unused (marked __maybe_unused).

There's no need to tag parameters __maybe_unused, AFAIK no compiler ever complains
about parameters, only local/global variables.

> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 9b1afa47c54c..dbde186ccb81 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -820,7 +820,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  		 * svm_set_msr's definition of reserved bits.
>  		 */
>  		svm_copy_lbrs(&vmcb02->save, save);
> -		vmcb02->save.dbgctl &= ~DEBUGCTL_RESERVED_BITS;
> +		vmcb02->save.dbgctl &= ~svm_get_debugctl_reserved_bits(vcpu);
>  	} else {
>  		svm_copy_lbrs(&vmcb02->save, &vmcb01->save);
>  	}
> @@ -1203,7 +1203,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
>  }
>  
>  /* Copy state save area fields which are handled by VMRUN */
> -void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
> +void svm_copy_vmrun_state(struct kvm_vcpu *vcpu, struct vmcb_save_area *to_save,
>  			  struct vmcb_save_area *from_save)
>  {
>  	to_save->es = from_save->es;
> @@ -1230,7 +1230,7 @@ void svm_copy_vmrun_state(struct vmcb_save_area *to_save,
>  
>  	if (kvm_cpu_cap_has(X86_FEATURE_LBRV)) {
>  		svm_copy_lbrs(to_save, from_save);
> -		to_save->dbgctl &= ~DEBUGCTL_RESERVED_BITS;
> +		to_save->dbgctl &= ~svm_get_debugctl_reserved_bits(vcpu);
>  	}
>  }

...

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index e9f2456982d4..ccf85ea79d37 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -3164,7 +3164,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
>  			data &= ~DEBUGCTLMSR_BTF;
>  		}
>  
> -		if (data & DEBUGCTL_RESERVED_BITS)
> +		if (data & svm_get_debugctl_reserved_bits(vcpu))
>  			return 1;
>  
>  		if (svm->vmcb->save.dbgctl == data)
> @@ -5037,8 +5037,7 @@ static int svm_enter_smm(struct kvm_vcpu *vcpu, union kvm_smram *smram)
>  
>  	BUILD_BUG_ON(offsetof(struct vmcb, save) != 0x400);
>  
> -	svm_copy_vmrun_state(map_save.hva + 0x400,
> -			     &svm->vmcb01.ptr->save);
> +	svm_copy_vmrun_state(vcpu, map_save.hva + 0x400, &svm->vmcb01.ptr->save);
>  
>  	kvm_vcpu_unmap(vcpu, &map_save);
>  	return 0;
> @@ -5081,7 +5080,7 @@ static int svm_leave_smm(struct kvm_vcpu *vcpu, const union kvm_smram *smram)
>  	 * used during SMM (see svm_enter_smm())
>  	 */
>  
> -	svm_copy_vmrun_state(&svm->vmcb01.ptr->save, map_save.hva + 0x400);
> +	svm_copy_vmrun_state(vcpu, &svm->vmcb01.ptr->save, map_save.hva + 0x400);
>  
>  	/*
>  	 * Enter the nested guest now
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index d52010e4de97..696f1b4b8f8f 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -783,7 +783,10 @@ BUILD_SVM_MSR_BITMAP_HELPERS(bool, test, test)
>  BUILD_SVM_MSR_BITMAP_HELPERS(void, clear, __clear)
>  BUILD_SVM_MSR_BITMAP_HELPERS(void, set, __set)
>  
> -#define DEBUGCTL_RESERVED_BITS (~DEBUGCTLMSR_LBR)
> +static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu __maybe_unused)

The polarity is cumbersome and annoyingly different than VMX, quite silly since
most paths care about supported bits, and the resulting code once BLD comes along
is downright ridiculous.  Invert the polarity so that we end up with

  static inline u64 svm_get_supported_debugctl(struct kvm_vcpu *vcpu)
  {
	u64 debugctl = DEBUGCTLMSR_LBR;

	if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
		debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;

	return debugctl;
  }


instead of:

  static inline u64 svm_get_debugctl_reserved_bits(struct kvm_vcpu *vcpu)
  {
       u64 debugctl = DEBUGCTLMSR_LBR;

       if (guest_cpu_cap_has(vcpu, X86_FEATURE_BUS_LOCK_DETECT))
               debugctl |= DEBUGCTLMSR_BUS_LOCK_DETECT;

       return ~debugctl;
  }