From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] Ignore DEBUGCTL MSRs Date: Wed, 16 Jul 2008 11:26:37 +0200 Message-ID: <487DBECD.7000605@suse.de> References: <4860806D.6000608@suse.de> <486F6904.2040907@qumranet.com> <4874B3B7.7060003@suse.de> <48761651.3030407@qumranet.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------010609070402040100060306" Cc: kvm@vger.kernel.org, joerg.roedel@amd.com To: Avi Kivity Return-path: Received: from cantor2.suse.de ([195.135.220.15]:42202 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753918AbYGPJ0m (ORCPT ); Wed, 16 Jul 2008 05:26:42 -0400 In-Reply-To: <48761651.3030407@qumranet.com> Sender: kvm-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------010609070402040100060306 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Avi Kivity wrote: > 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. ok. > >> + 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. --------------- 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 --------------010609070402040100060306 Content-Type: text/x-patch; name="kvm-netware.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="kvm-netware.patch" 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 & ~(u64)(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: --------------010609070402040100060306--