Re: [PATCH] Ignore DEBUGCTL MSRs

[Avi Kivity <avi@qumranet.com>]

Alexander Graf wrote:
> Avi Kivity wrote:
>> Alexander Graf wrote:
>>> Netware writes and reads to the DEBUGCTL and LAST*IP MSRs without 
>>> further checks and is really confused to receive a #GP during that. 
>>> To make it happy we should just make them stubs, which is exactly 
>>> what SVM already does.
>>>
>>> To support VMX too, I put these in the generic code. Maybe the SVM 
>>> code could be cleaned up to use generic code too.
>>>
>>
>> Please add a pr_unimpl() when bits that cause a real processor to do 
>> something are set.
>
> Like this? I also removed the set handlers for the *IP MSRs, as these 
> are read only and made it only handle debug bits, no perfmon bits.
>

With a changelog entry.

> Signed-off-by: Alexander Graf <agraf@suse.de>
>
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index fc0721e..10f5e95 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -609,6 +609,15 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
>  		pr_unimpl(vcpu, "%s: MSR_IA32_MCG_CTL 0x%llx, nop\n",
>  			__func__, data);
>  		break;
> +	case MSR_IA32_DEBUGCTLMSR:
> +		if (data > (DEBUGCTLMSR_LBR | DEBUGCTLMSR_BTF)) {
> +			/* Values other than LBR and BTF are vendor-specific,
> +			   thus reserved and should throw a #GP */
> +			return 1;
> +		}
>   
'>' is too clever. '& ~(u64)(...)' is clearer.

Arithmetic and logical ops don't mix well.


> +		pr_unimpl(vcpu, "%s: MSR_IA32_DEBUGCTLMSR 0x%llx, nop\n",
> +			__func__, data);
>   

We can avoid the printout if data == 0, since we support that case fully.

-- 
error compiling committee.c: too many arguments to function