On 09/11/2017 09:15, Eyal Moscovici wrote:
>
>
> 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. :)
>
> 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.
Yes, sorry---it wasn't meant as a rejection of this patch.
Paolo
>>> 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;
>>>
>
