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
