Re: [PATCH] KVM: x86: Allow suppressing prints on RDMSR/WRMSR of unhandled MSRs

[Eyal Moscovici <eyal.moscovici@xxxxxxxxxx>]

On 08/11/17 15:11, Paolo Bonzini wrote:
> On 08/11/2017 13:32, Eyal Moscovici wrote:
> > Some guests use these unhandled MSRs very frequently.
> > This cause dmesg to be populated with lots of aggregated messages on
> > usage of ignored MSRs. As ignore_msrs=true means that the user is
> > well-aware his guest use ignored MSRs, allow to also disable the
> > prints on their usage.
> >
> > An example of such guest is ESXi which tends to access a lot to MSR
> > 0x34 (MSR_SMI_COUNT) very frequently.
> For this particular MSR, it can be a good idea to implement it actually. :)
>
> Paolo
Agreed as this is fairly simple. Have implemented. We will submit a 
patch very soon.

However, I still think this patch of suppressing prints of usage of 
ignored MSRs is useful as it can spam the dmesg when ignored_msrs=true. 
We have seen other guests that also make frequent usage of other MSRs 
such as esoteric CPU perf MSRs and etc.
> > In addition, we have observed this to cause unnecessary delays to
> > guest execution. Such an example is ESXi which experience networking
> > delays in it's guests (L2 guests) because of these prints (even when
> > prints are rate-limited). This can easily be reproduced by pinging
> > from one L2 guest to another.  Once in a while, a peak in ping RTT
> > will be observed. Removing these unhandled MSR prints solves the
> > issue.
> >
> > Because these prints can help diagnose issues with guests,
> > this commit only suppress them by a module parameter instead of
> > removing them from code entirely.
> >
> > Signed-off-by: Eyal Moscovici <eyal.moscovici@xxxxxxxxxx>
> > Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
> > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > ---
> >   arch/x86/kvm/x86.c | 17 +++++++++++++----
> >   1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 03869eb..21c0059 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -107,6 +107,9 @@
> >   static bool __read_mostly ignore_msrs = 0;
> >   module_param(ignore_msrs, bool, S_IRUGO | S_IWUSR);
> >   
> > +static bool __read_mostly suppress_ignore_msrs_prints = false;
> > +module_param(suppress_ignore_msrs_prints, bool, S_IRUGO | S_IWUSR);
> > +
> >   unsigned int min_timer_period_us = 500;
> >   module_param(min_timer_period_us, uint, S_IRUGO | S_IWUSR);
> >   
> > @@ -2317,7 +2320,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >   		/* Drop writes to this legacy MSR -- see rdmsr
> >   		 * counterpart for further detail.
> >   		 */
> > -		vcpu_unimpl(vcpu, "ignored wrmsr: 0x%x data 0x%llx\n", msr, data);
> > +		if (!suppress_ignore_msrs_prints)
> > +			vcpu_unimpl(vcpu, "ignored wrmsr: 0x%x data 0x%llx\n",
> > +				msr, data);
> >   		break;
> >   	case MSR_AMD64_OSVW_ID_LENGTH:
> >   		if (!guest_cpuid_has(vcpu, X86_FEATURE_OSVW))
> > @@ -2354,8 +2359,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >   				    msr, data);
> >   			return 1;
> >   		} else {
> > -			vcpu_unimpl(vcpu, "ignored wrmsr: 0x%x data 0x%llx\n",
> > -				    msr, data);
> > +			if (!suppress_ignore_msrs_prints)
> > +				vcpu_unimpl(vcpu,
> > +					"ignored wrmsr: 0x%x data 0x%llx\n",
> > +					msr, data);
> >   			break;
> >   		}
> >   	}
> > @@ -2573,7 +2580,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >   					       msr_info->index);
> >   			return 1;
> >   		} else {
> > -			vcpu_unimpl(vcpu, "ignored rdmsr: 0x%x\n", msr_info->index);
> > +			if (!suppress_ignore_msrs_prints)
> > +				vcpu_unimpl(vcpu, "ignored rdmsr: 0x%x\n",
> > +					msr_info->index);
> >   			msr_info->data = 0;
> >   		}
> >   		break;