Re: RFC: silencing kvm unimplemented msr spew.

[Bandan Das <bsd@redhat.com>]

Hi Dave,

Dave Jones <dsj@fb.com> 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