Messages in this thread | | | Date | Fri, 26 Jun 2020 13:37:50 -0400 | From | Peter Xu <> | Subject | Re: [PATCH 1/2] KVM: X86: Move ignore_msrs handling upper the stack |
| |
On Fri, Jun 26, 2020 at 08:56:57AM -0700, Sean Christopherson wrote: > Not really? It's solving a problem that doesn't exist in the current code > base (assuming TSC_CTRL is fixed), and IMO solving it in an ugly fashion. > > I would much prefer that, _if_ we want to support blind KVM-internal MSR > accesses, we end up with code like: > > if (msr_info->kvm_internal) { > return 1; > } else if (!ignore_msrs) { > vcpu_debug_ratelimited(vcpu, "unhandled wrmsr: 0x%x data 0x%llx\n", > msr, data); > return 1; > } else { > if (report_ignored_msrs) > vcpu_unimpl(vcpu, > "ignored wrmsr: 0x%x data 0x%llx\n", > msr, data); > break; > } > > But I'm still not convinced that there is a legimiate scenario for setting > kvm_internal=true.
Actually this really looks like my initial version when I was discussing this with Paolo before this version, but Paolo suggested what I implemented last. I think I agree with Paolo that it's an improvement to have a way to get/set real msr value so that we don't need to further think about effects being taken with the two tricky msr knobs (report_ignored_msrs, ignore_msrs). These knobs are even trickier to me when they're hidden deep, because they are not easily expected when seeing the name of the functions (e.g. __kvm_set_msr, rather than __kvm_set_msr_retval_fixed).
Thanks,
-- Peter Xu
| |