RFC: silencing kvm unimplemented msr spew.

Commit Message

Dave Jones July 19, 2016, 7:58 p.m. UTC

On Mon, Jul 18, 2016 at 11:26:50AM -0400, Paolo Bonzini wrote:
 > 
 > > kvm is pretty noisy when you have guests poking at MSRs that the kernel
 > > doesn't implement.  The conveniently named 'ignore_msrs' option initially
 > > seemed
 > > like it was what I was looking for, but it changes the printk instead
 > > of eliding it.
 > > 
 > > Untested patch below converts ignore_msrs to a bitmask and adds an option to
 > > be
 > > completely silent. The idea being if after testing, things still work and you
 > > don't care about those messages, you can deploy in production with the
 > > silence option.
 > > 
 > > Would something like this be acceptable ?
 > 
 > Indeed, ignore_msrs does a completely different thing.  It suppresses
 > general protection faults in the guest.  It is related to behavior that
 > KVM injects in the guests, not to the things that KVM spews in the host.
 > 
 > What about just downgrading the printf to KERN_DEBUG?  You could simply
 > change from vcpu_unimpl to vcpu_debug, but it's probably a good idea to
 > keep the ratelimiting; there's a kvm_pr_unimpl, so maybe add a new
 > kvm_pr_debug and vcpu_pr_debug.

Hm, we've certainly got a lot of options in terms of print primitives these days.

We could just do this...



Which I think would have the desired effect, and also gets us dynamic debug
support for free.

Thoughts ?

	Dave
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Paolo Bonzini July 19, 2016, 10:12 p.m. UTC | #1

> On Mon, Jul 18, 2016 at 11:26:50AM -0400, Paolo Bonzini wrote:
> > Indeed, ignore_msrs does a completely different thing.  It suppresses
> > general protection faults in the guest.  It is related to behavior that
> > KVM injects in the guests, not to the things that KVM spews in the host.
> > 
> > What about just downgrading the printf to KERN_DEBUG?  You could simply
> > change from vcpu_unimpl to vcpu_debug, but it's probably a good idea to
> > keep the ratelimiting; there's a kvm_pr_unimpl, so maybe add a new
> > kvm_pr_debug and vcpu_pr_debug.
>
> Hm, we've certainly got a lot of options in terms of print primitives these
> days.
> 
> We could just do this...
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 1c9c973a7dd9..a80b9a0a5f8c 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -423,7 +423,7 @@ struct kvm {
>  #define kvm_debug(fmt, ...) \
>         pr_debug("kvm [%i]: " fmt, task_pid_nr(current), ## __VA_ARGS__)
>  #define kvm_pr_unimpl(fmt, ...) \
> -       pr_err_ratelimited("kvm [%i]: " fmt, \
> +       pr_debug_ratelimited("kvm [%i]: " fmt, \
>                            task_tgid_nr(current), ## __VA_ARGS__)
>  
>  /* The guest did something we don't support. */
> 
> Which I think would have the desired effect, and also gets us dynamic debug
> support for free.

There are some "unimplemented" messages that would be affected and really
should be errors (or even WARNs), but I guess this patch is okay.  Can you
submit it with SoB and all that?

Thanks,

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1c9c973a7dd9..a80b9a0a5f8c 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -423,7 +423,7 @@  struct kvm {
 #define kvm_debug(fmt, ...) \
        pr_debug("kvm [%i]: " fmt, task_pid_nr(current), ## __VA_ARGS__)
 #define kvm_pr_unimpl(fmt, ...) \
-       pr_err_ratelimited("kvm [%i]: " fmt, \
+       pr_debug_ratelimited("kvm [%i]: " fmt, \
                           task_tgid_nr(current), ## __VA_ARGS__)
 
 /* The guest did something we don't support. */