[v2,2/4] kvm: vmx: Omit MSR_STAR from vmx_msr_index[] for i386 builds
diff mbox series

Message ID 20181109222131.60885-1-jmattson@google.com
State New
Headers show
Series
  • [v2,1/4] kvm: vmx: Set IA32_TSC_AUX for legacy mode guests
Related show

Commit Message

Jim Mattson Nov. 9, 2018, 10:21 p.m. UTC
The SYSCALL instruction is only supported in 64-bit mode on Intel
CPUs. With VT-x, A legacy-mode hypervisor can't launch a long-mode
guest. Therefore, we can omit MSR_STAR support for i386 builds.

Note that the elided comment has not been relevant since move_msr_up()
was introduced in commit a75beee6e4f5d ("KVM: VMX: Avoid saving and
restoring msrs on lightweight vmexit").

Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
---
 arch/x86/kvm/vmx.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Jim Mattson Nov. 12, 2018, 5:22 p.m. UTC | #1
On Fri, Nov 9, 2018 at 2:21 PM, Jim Mattson <jmattson@google.com> wrote:
> The SYSCALL instruction is only supported in 64-bit mode on Intel
> CPUs. With VT-x, A legacy-mode hypervisor can't launch a long-mode
> guest. Therefore, we can omit MSR_STAR support for i386 builds.
>
> Note that the elided comment has not been relevant since move_msr_up()
> was introduced in commit a75beee6e4f5d ("KVM: VMX: Avoid saving and
> restoring msrs on lightweight vmexit").

I realize now that eliminating the storage for MSR_STAR from i386
builds breaks syscall emulation on Intel hardware when the guest CPUID
claims "AuthenticAMD" or "AMDisbetter!" (?)

I'll send out a v3 that leaves the storage for MSR_STAR in i386 builds
and clearly documents the reason for it.
Liran Alon Nov. 13, 2018, 12:08 a.m. UTC | #2
> On 12 Nov 2018, at 19:22, Jim Mattson <jmattson@google.com> wrote:
> 
> On Fri, Nov 9, 2018 at 2:21 PM, Jim Mattson <jmattson@google.com> wrote:
>> The SYSCALL instruction is only supported in 64-bit mode on Intel
>> CPUs. With VT-x, A legacy-mode hypervisor can't launch a long-mode
>> guest. Therefore, we can omit MSR_STAR support for i386 builds.
>> 
>> Note that the elided comment has not been relevant since move_msr_up()
>> was introduced in commit a75beee6e4f5d ("KVM: VMX: Avoid saving and
>> restoring msrs on lightweight vmexit").
> 
> I realize now that eliminating the storage for MSR_STAR from i386
> builds breaks syscall emulation on Intel hardware when the guest CPUID
> claims "AuthenticAMD" or "AMDisbetter!" (?)

Heh.
“AMDisbetter!” seems to be a joke that was reported from early engineering samples of AMD K5 processor.
Go figure ;)

> 
> I'll send out a v3 that leaves the storage for MSR_STAR in i386 builds
> and clearly documents the reason for it.

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 120fc97a63fc..da7f43457d49 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1387,15 +1387,11 @@  static u64 host_efer;
 
 static void ept_save_pdptrs(struct kvm_vcpu *vcpu);
 
-/*
- * Keep MSR_STAR at the end, as setup_msrs() will try to optimize it
- * away by decrementing the array size.
- */
 static const u32 vmx_msr_index[] = {
 #ifdef CONFIG_X86_64
-	MSR_SYSCALL_MASK, MSR_LSTAR, MSR_CSTAR,
+	MSR_SYSCALL_MASK, MSR_STAR, MSR_LSTAR, MSR_CSTAR,
 #endif
-	MSR_EFER, MSR_TSC_AUX, MSR_STAR,
+	MSR_EFER, MSR_TSC_AUX
 };
 
 DEFINE_STATIC_KEY_FALSE(enable_evmcs);