Re: [PATCH] KVM: nVMX: Use SET_MSR_OR_WARN() to simplify failure logging

[Oliver Upton <oupton@google.com>]

On Thu, Dec 12, 2019 at 01:43:27AM +0100, Paolo Bonzini wrote:
> On 02/12/19 22:21, Sean Christopherson wrote:
> > As for the original code, arguably it *should* do a full WARN and not
> > simply log the error, as kvm_set_msr() should never fail if
> > VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL was exposed to L1, unlike the above two
> > cases where KVM is processing an L1-controlled MSR list, e.g.:
> > 
> > 	if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL)
> > 		WARN_ON_ONCE(kvm_set_msr(vcpu, MSR_CORE_PERF_GLOBAL_CTRL,
> > 					 vmcs12->host_ia32_perf_global_ctrl));
> > 
> > Back to this patch, this isn't simply consolidating code, it's promoting
> > L1-controlled messages from pr_debug() to pr_warn().
> > 
> > What if you add a patch to remove SET_MSR_OR_WARN() and instead manually
> > do the WARN_ON_ONCE() as above, and then introduce a new macro to
> > consolidate the pr_debug_ratelimited() stuff in this patch?

Sean,

Thank you for the detailed review of this patch (as well as the last one
that I snuck past you :-P). I'm in complete agreement with your
sentiments, a follow-up is in order. I'll get that out soon.

> Should go without saying (Sean is a Certified Reviewer according to
> MAINTAINERS :)) but I agree.
> 
> Paolo

Sean has been a great help in providing detailed reviews -- well
deserving of the designation! Thank you for pinging this thread, Paolo.

--
Best,
Oliver