Re: [PATCH] Ignore DEBUGCTL MSRs

[Alexander Graf <agraf@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

Avi Kivity wrote:
> Alexander Graf wrote:
>>>
>>>> +        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.
>>
>> I was thinking a lot about that. Even though we support data == 0, 
>> usually the kernel log output is useful for people trying to find if 
>> something is cause a problem. If they see that DEBUGCTL gets set, but 
>> won't see it getting unset, they'd get confused IMHO.
>> So the current behavior is on purpose, but if you oppose to that 
>> idea, please tell me.
>>
>
> Once it gets set, you can expect brokenness.  It doesn't matter if it 
> gets unset later.  So IMO not printing on data == 0 is best: quiet on 
> the cases we support, and loud on cases we don't.  So yes, I'd prefer 
> it changed.
>
>

Corrected patch follows...

---

Netware writes to DEBUGCTL and reads from 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.

Writes to DEBUGCTL that are vendor-specific are resembled to behave as 
if the virtual CPU does not know them.

Signed-off-by: Alexander Graf <agraf@suse.de>


[-- Attachment #2: kvm-netware.patch --]
[-- Type: text/x-patch, Size: 1132 bytes --]

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) {
+			/* We support the non-activated case already */
+			break;
+		} else if (data & ~(DEBUGCTLMSR_LBR | DEBUGCTLMSR_BTF)) {
+			/* Values other than LBR and BTF are vendor-specific,
+			   thus reserved and should throw a #GP */
+			return 1;
+		}
+		pr_unimpl(vcpu, "%s: MSR_IA32_DEBUGCTLMSR 0x%llx, nop\n",
+			__func__, data);
+		break;
 	case MSR_IA32_UCODE_REV:
 	case MSR_IA32_UCODE_WRITE:
 		break;
@@ -705,6 +714,11 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_IA32_MC0_MISC+16:
 	case MSR_IA32_UCODE_REV:
 	case MSR_IA32_EBL_CR_POWERON:
+	case MSR_IA32_DEBUGCTLMSR:
+	case MSR_IA32_LASTBRANCHFROMIP:
+	case MSR_IA32_LASTBRANCHTOIP:
+	case MSR_IA32_LASTINTFROMIP:
+	case MSR_IA32_LASTINTTOIP:
 		data = 0;
 		break;
 	case MSR_MTRRcap: