From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754109AbcGUUls (ORCPT ); Thu, 21 Jul 2016 16:41:48 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42925 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753877AbcGUUlp (ORCPT ); Thu, 21 Jul 2016 16:41:45 -0400 From: Bandan Das To: Dave Jones Cc: Paolo Bonzini , Linux Kernel , Radim =?utf-8?B?S3LEjW3DocWZ?= , kvm@vger.kernel.org Subject: Re: RFC: silencing kvm unimplemented msr spew. References: <20160715192729.GA4712@fb.com> <110045299.8101686.1468855610053.JavaMail.zimbra@redhat.com> <20160719195856.GA31453@fb.com> <20160721202440.GA10908@fb.com> Date: Thu, 21 Jul 2016 16:41:43 -0400 In-Reply-To: <20160721202440.GA10908@fb.com> (Dave Jones's message of "Thu, 21 Jul 2016 16:24:40 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 21 Jul 2016 20:41:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Dave, Dave Jones writes: > 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). My reasoning was: When debugging guest runs/unimplemented msrs accesses, it makes sense to use the original behavior of printing out the accesses. So, vcpu_unimpl() remains unchanged and is used for that case and vcpu_debug_ratelimited becomes the default. Bandan > Paolo, would you prefer this, or the other approach you already ack'd ? > > Dave > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html