On Fri, Dec 14, 2018 at 11:42:31AM -0800, Jim Mattson wrote: > On Fri, Dec 14, 2018 at 11:31 AM Sean Christopherson > <sean.j.christopherson@xxxxxxxxx> wrote: > > > > On Wed, Dec 12, 2018 at 04:10:26PM -0800, Jim Mattson wrote: > > > If the guest supports RDTSCP, it already has read access to the > > > hardware IA32_TSC_AUX MSR via RDTSCP, so we can allow it read-access > > > via RDMSR as well. If the guest doesn't support RDTSCP, then we > > > should not allow it read access to the hardware IA32_TSC_AUX MSR. > > > > Maybe tweak the last sentence? > > > > Intercept all accesses if the guest doesn't support RDTSCP in order > > to inject #GP, IA32_TSC_AUX exists iff RDTSCP is supported. > > > > > Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx> > > > Reviewed-by: Marc Orr <marcorr@xxxxxxxxxx> > > > Reviewed-by: Peter Shier <pshier@xxxxxxxxxx> > > > > Reviewed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> > > Offline, Krish had a reasonable complaint about where I stuck this bit > of code. Should I do another version with some refactoring, or are > people happy enough with this version? What's the alternative location? It seemed a bit odd to me too, but at the same time there isn't much of a precedent for toggling intercept of a single MSR based on guest CPUID. And looking at the code, maybe it only seems odd because we have vmx_update_msr_bitmap(). What if that and vmx_update_msr_bitmap_x2apic() were renamed to e.g. vmx_update_intercept_for_x2apic_msrs() and __vmx_update_intercept_for_x2apic_msrs() so that there isn't a somewhat bogus implication that updating the bitmaps should only be done in vmx_update_msr_bitmap().
