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

[Shivansh Dhiman <shivansh.dhiman@amd.com>]

On 25-09-26 23:13, Sean Christopherson wrote:
> 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.

I remember my compiler giving some warning about the unused parameters IIRC.
So, I added it to preserve clean builds between patches. Anyway, I'll check
my build environment once.

>> 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;
>   }

Checked the VMX side of it, and this looks better. Thanks.

-Shivansh

> 
> 
> 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;
>   }