On 12/14/2018 11:50 AM, Sean Christopherson wrote:
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().
The other possibility is to create a new function, say,
vmx_cpuid_update_msr_bitmap(), which will be called in the CPUID update
path, thereby leaving vmx_update_msr_bitmap() as is.
