From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753418AbcGVIsq (ORCPT ); Fri, 22 Jul 2016 04:48:46 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:55232 "EHLO mx5-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752171AbcGVIso (ORCPT ); Fri, 22 Jul 2016 04:48:44 -0400 Date: Fri, 22 Jul 2016 04:48:41 -0400 (EDT) From: Paolo Bonzini To: Dave Jones Cc: Bandan Das , Linux Kernel , Radim =?utf-8?B?S3LEjW3DocWZ?= , kvm@vger.kernel.org Message-ID: <844431780.9635789.1469177321917.JavaMail.zimbra@redhat.com> In-Reply-To: <20160721202440.GA10908@fb.com> References: <20160715192729.GA4712@fb.com> <110045299.8101686.1468855610053.JavaMail.zimbra@redhat.com> <20160719195856.GA31453@fb.com> <20160721202440.GA10908@fb.com> Subject: Re: RFC: silencing kvm unimplemented msr spew. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [10.4.164.1, 10.5.100.50] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF43 (Mac)/8.0.6_GA_5922) Thread-Topic: silencing kvm unimplemented msr spew. Thread-Index: F+4qM0wFfAJAj0QRdIOQqUB2LaRyqA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > On Tue, Jul 19, 2016 at 04:24:31PM -0400, Bandan Das wrote: > > > Heh, actually after speaking about this to Paolo a while back, I had this > > sleeping > > in my local branch for a while. Same as what you suggested (without the > > ratelimiting) > > > > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c > > index def97b3..c6e6f64 100644 > > --- a/arch/x86/kvm/mmu.c > > +++ b/arch/x86/kvm/mmu.c > > @@ -4952,7 +4952,7 @@ void kvm_mmu_invalidate_mmio_sptes(struct kvm *kvm, > > struct kvm_memslots *slots) > > * zap all shadow pages. > > */ > > if (unlikely((slots->generation & MMIO_GEN_MASK) == 0)) { > > - printk_ratelimited(KERN_DEBUG "kvm: zapping shadow pages > > for mmio generation wraparound\n"); > > + kvm_debug("zapping shadow pages for mmio generation > > wraparound\n"); > > kvm_mmu_invalidate_zap_all_pages(kvm); > > } > > } > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 7da5dd2..02d09f9 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -2229,7 +2229,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct > > msr_data *msr_info) > > if (kvm_pmu_is_valid_msr(vcpu, msr)) > > return kvm_pmu_set_msr(vcpu, msr_info); > > if (!ignore_msrs) { > > - vcpu_unimpl(vcpu, "unhandled wrmsr: 0x%x data > > %llx\n", > > + vcpu_debug(vcpu, "unhandled wrmsr: 0x%x data > > %llx\n", > > msr, data); > > return 1; > > } else { > > @@ -2441,7 +2441,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct > > msr_data *msr_info) > > if (kvm_pmu_is_valid_msr(vcpu, msr_info->index)) > > return kvm_pmu_get_msr(vcpu, msr_info->index, > > &msr_info->data); > > if (!ignore_msrs) { > > - vcpu_unimpl(vcpu, "unhandled rdmsr: 0x%x\n", msr_info->index); > > + vcpu_debug(vcpu, "unhandled rdmsr: 0x%x\n", msr_info->index); > > return 1; > > } else { > > vcpu_unimpl(vcpu, "ignored rdmsr: 0x%x\n", > > msr_info->index); > > > > I had the same reasoning regarding dynamic debugging which I think is > > enabled by default on most builds anyway. > > Yeah, that's close. Though I would have done the same for the other side of > the if's too. > (Still evaluating which mode is actually more useful for us). For Linux guests, there should be no reason to use ignore_msrs. Linux is pretty resilient to "missing" MSRs (especially because they are already ignored if the kernel is compiled with CONFIG_PARAVIRT=y!). The option is really more for Windows, because it doesn't have anything like CONFIG_PARAVIRT and because drivers are sometimes less vetted (and sometimes do RDMSR themselves for whatever reason). In general we try to look at beta versions of Windows and add any required MSRs well before the final release date, but if you're using old kernels you might be stuck with ignore_msrs. IOW, if there was a really common reason to use ignore_msrs it would be the default. ;) > Paolo, would you prefer this, or the other approach you already ack'd ? I think I prefer the other, because vcpu_debug is not ratelimited. If the guest can trigger a printk it should always be ratelimited. Paolo