From: Nathan Chancellor <nathan@kernel.org>
To: Sean Christopherson <seanjc@google.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>,
kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
Anirudh Rayabharam <anrayabh@linux.microsoft.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>,
Maxim Levitsky <mlevitsk@redhat.com>,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
llvm@lists.linux.dev
Subject: Re: [PATCH v4 15/25] KVM: VMX: Extend VMX controls macro shenanigans
Date: Fri, 22 Jul 2022 14:04:37 -0700 [thread overview]
Message-ID: <YtsQ5SkCJXQIuKGS@dev-arch.thelio-3990X> (raw)
In-Reply-To: <YtrtdylmyolAHToz@google.com>
On Fri, Jul 22, 2022 at 06:33:27PM +0000, Sean Christopherson wrote:
> On Thu, Jul 14, 2022, Vitaly Kuznetsov wrote:
> > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> > index 286c88e285ea..89eaab3495a6 100644
> > --- a/arch/x86/kvm/vmx/vmx.h
> > +++ b/arch/x86/kvm/vmx/vmx.h
> > @@ -467,6 +467,113 @@ static inline u8 vmx_get_rvi(void)
> > return vmcs_read16(GUEST_INTR_STATUS) & 0xff;
> > }
> >
> > +#define __KVM_REQ_VMX_VM_ENTRY_CONTROLS \
> > + (VM_ENTRY_LOAD_DEBUG_CONTROLS)
> > +#ifdef CONFIG_X86_64
> > + #define KVM_REQ_VMX_VM_ENTRY_CONTROLS \
> > + (__KVM_REQ_VMX_VM_ENTRY_CONTROLS | \
> > + VM_ENTRY_IA32E_MODE)
>
> This breaks 32-bit builds, but at least we know the assert works!
>
> vmx_set_efer() toggles VM_ENTRY_IA32E_MODE without a CONFIG_X86_64 guard. That
> should be easy enough to fix since KVM should never allow EFER_LMA. Compile
> tested patch at the bottom.
>
> More problematic is that clang-13 doesn't like the new asserts, and even worse gives
> a very cryptic error. I don't have bandwidth to look into this at the moment, and
> probably won't next week either.
>
> ERROR: modpost: "__compiletime_assert_533" [arch/x86/kvm/kvm-intel.ko] undefined!
> ERROR: modpost: "__compiletime_assert_531" [arch/x86/kvm/kvm-intel.ko] undefined!
> ERROR: modpost: "__compiletime_assert_532" [arch/x86/kvm/kvm-intel.ko] undefined!
> ERROR: modpost: "__compiletime_assert_530" [arch/x86/kvm/kvm-intel.ko] undefined!
> make[2]: *** [scripts/Makefile.modpost:128: modules-only.symvers] Error 1
> make[1]: *** [Makefile:1753: modules] Error 2
> make[1]: *** Waiting for unfinished jobs....
clang-14 added support for the error and warning attributes, which makes
the BUILD_BUG_ON failures look like GCC. With allmodconfig, this
becomes:
In file included from ../arch/x86/kvm/vmx/vmx.c:61:
In file included from ../arch/x86/kvm/vmx/nested.h:7:
../arch/x86/kvm/vmx/vmx.h:610:1: error: call to __compiletime_assert_1135 declared with 'error' attribute: BUILD_BUG_ON failed: !(val & (KVM_REQ_VMX_VM_ENTRY_CONTROLS | KVM_OPT_VMX_VM_ENTRY_CONTROLS))
BUILD_CONTROLS_SHADOW(vm_entry, VM_ENTRY_CONTROLS, 32)
^
../arch/x86/kvm/vmx/vmx.h:602:2: note: expanded from macro 'BUILD_CONTROLS_SHADOW'
BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname | KVM_OPT_VMX_##uname))); \
^
../include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
../include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
../include/linux/compiler_types.h:340:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
../include/linux/compiler_types.h:333:4: note: expanded from macro '__compiletime_assert'
prefix ## suffix(); \
^
<scratch space>:259:1: note: expanded from here
__compiletime_assert_1135
^
In file included from ../arch/x86/kvm/vmx/vmx.c:61:
In file included from ../arch/x86/kvm/vmx/nested.h:7:
../arch/x86/kvm/vmx/vmx.h:610:1: error: call to __compiletime_assert_1136 declared with 'error' attribute: BUILD_BUG_ON failed: !(val & (KVM_REQ_VMX_VM_ENTRY_CONTROLS | KVM_OPT_VMX_VM_ENTRY_CONTROLS))
../arch/x86/kvm/vmx/vmx.h:607:2: note: expanded from macro 'BUILD_CONTROLS_SHADOW'
BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname | KVM_OPT_VMX_##uname))); \
^
../include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
../include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
../include/linux/compiler_types.h:340:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
../include/linux/compiler_types.h:333:4: note: expanded from macro '__compiletime_assert'
prefix ## suffix(); \
^
<scratch space>:10:1: note: expanded from here
__compiletime_assert_1136
^
In file included from ../arch/x86/kvm/vmx/vmx.c:61:
In file included from ../arch/x86/kvm/vmx/nested.h:7:
../arch/x86/kvm/vmx/vmx.h:611:1: error: call to __compiletime_assert_1137 declared with 'error' attribute: BUILD_BUG_ON failed: !(val & (KVM_REQ_VMX_VM_EXIT_CONTROLS | KVM_OPT_VMX_VM_EXIT_CONTROLS))
BUILD_CONTROLS_SHADOW(vm_exit, VM_EXIT_CONTROLS, 32)
^
../arch/x86/kvm/vmx/vmx.h:602:2: note: expanded from macro 'BUILD_CONTROLS_SHADOW'
BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname | KVM_OPT_VMX_##uname))); \
^
../include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
../include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
../include/linux/compiler_types.h:340:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
../include/linux/compiler_types.h:333:4: note: expanded from macro '__compiletime_assert'
prefix ## suffix(); \
^
<scratch space>:30:1: note: expanded from here
__compiletime_assert_1137
^
In file included from ../arch/x86/kvm/vmx/vmx.c:61:
In file included from ../arch/x86/kvm/vmx/nested.h:7:
../arch/x86/kvm/vmx/vmx.h:611:1: error: call to __compiletime_assert_1138 declared with 'error' attribute: BUILD_BUG_ON failed: !(val & (KVM_REQ_VMX_VM_EXIT_CONTROLS | KVM_OPT_VMX_VM_EXIT_CONTROLS))
../arch/x86/kvm/vmx/vmx.h:607:2: note: expanded from macro 'BUILD_CONTROLS_SHADOW'
BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname | KVM_OPT_VMX_##uname))); \
^
../include/linux/build_bug.h:50:2: note: expanded from macro 'BUILD_BUG_ON'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^
../include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
note: (skipping 1 expansions in backtrace; use -fmacro-backtrace-limit=0 to see all)
../include/linux/compiler_types.h:340:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
../include/linux/compiler_types.h:333:4: note: expanded from macro '__compiletime_assert'
prefix ## suffix(); \
^
<scratch space>:40:1: note: expanded from here
__compiletime_assert_1138
^
4 errors generated.
As you mentioned in the other comment on this patch, the 'inline'
keyword should be '__always_inline' in the BUILD_CONTROLS_SHADOW macro
and a couple of other functions need it for BUILD_BUG_ON to see the
value all the way through the call chain. The following diff resolves
those errors for me, hopefully it is useful!
Cheers,
Nathan
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4ce7ed835e06..b97ed63ece56 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -790,7 +790,7 @@ static bool msr_write_intercepted(struct vcpu_vmx *vmx, u32 msr)
MSR_IA32_SPEC_CTRL);
}
-static void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
+static __always_inline void clear_atomic_switch_msr_special(struct vcpu_vmx *vmx,
unsigned long entry, unsigned long exit)
{
vm_entry_controls_clearbit(vmx, entry);
@@ -848,7 +848,7 @@ static void clear_atomic_switch_msr(struct vcpu_vmx *vmx, unsigned msr)
vmcs_write32(VM_EXIT_MSR_LOAD_COUNT, m->host.nr);
}
-static void add_atomic_switch_msr_special(struct vcpu_vmx *vmx,
+static __always_inline void add_atomic_switch_msr_special(struct vcpu_vmx *vmx,
unsigned long entry, unsigned long exit,
unsigned long guest_val_vmcs, unsigned long host_val_vmcs,
u64 guest_val, u64 host_val)
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 758f80c41beb..acefa5b5e1b9 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -597,12 +597,12 @@ static inline u##bits lname##_controls_get(struct vcpu_vmx *vmx) \
{ \
return __##lname##_controls_get(vmx->loaded_vmcs); \
} \
-static inline void lname##_controls_setbit(struct vcpu_vmx *vmx, u##bits val) \
+static __always_inline void lname##_controls_setbit(struct vcpu_vmx *vmx, u##bits val) \
{ \
BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname | KVM_OPT_VMX_##uname))); \
lname##_controls_set(vmx, lname##_controls_get(vmx) | val); \
} \
-static inline void lname##_controls_clearbit(struct vcpu_vmx *vmx, u##bits val) \
+static __always_inline void lname##_controls_clearbit(struct vcpu_vmx *vmx, u##bits val) \
{ \
BUILD_BUG_ON(!(val & (KVM_REQ_VMX_##uname | KVM_OPT_VMX_##uname))); \
lname##_controls_set(vmx, lname##_controls_get(vmx) & ~val); \
> > +#else
> > + #define KVM_REQ_VMX_VM_ENTRY_CONTROLS \
> > + __KVM_REQ_VMX_VM_ENTRY_CONTROLS
> > +#endif
>
> EFER.LMA patch, compile tested only.
>
> ---
> From: Sean Christopherson <seanjc@google.com>
> Date: Fri, 22 Jul 2022 18:26:21 +0000
> Subject: [PATCH] KVM: VMX: Don't toggle VM_ENTRY_IA32E_MODE for 32-bit
> kernels/KVM
>
> Don't toggle VM_ENTRY_IA32E_MODE in 32-bit kernels/KVM and instead bug
> the VM if KVM attempts to run the guest with EFER.LMA=1. KVM doesn't
> support running 64-bit guests with 32-bit hosts.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/vmx/vmx.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index bff97babf381..8623607e596d 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2894,10 +2894,15 @@ int vmx_set_efer(struct kvm_vcpu *vcpu, u64 efer)
> return 0;
>
> vcpu->arch.efer = efer;
> +#ifdef CONFIG_X86_64
> if (efer & EFER_LMA)
> vm_entry_controls_setbit(vmx, VM_ENTRY_IA32E_MODE);
> else
> vm_entry_controls_clearbit(vmx, VM_ENTRY_IA32E_MODE);
> +#else
> + if (KVM_BUG_ON(efer & EFER_LMA, vcpu->kvm))
> + return 1;
> +#endif
>
> vmx_setup_uret_msrs(vmx);
> return 0;
>
> base-commit: e22e2665637151a321433b2bb705f5c3b8da40bc
> --
>
next prev parent reply other threads:[~2022-07-22 21:04 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-14 9:13 [PATCH v4 00/25] KVM: VMX: Support updated eVMCSv1 revision + use vmcs_config for L1 VMX MSRs Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 01/25] KVM: x86: hyper-v: Expose access to debug MSRs in the partition privilege flags Vitaly Kuznetsov
2022-07-21 21:43 ` Sean Christopherson
2022-07-22 17:22 ` Paolo Bonzini
2022-08-01 8:16 ` Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 02/25] x86/hyperv: Fix 'struct hv_enlightened_vmcs' definition Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 03/25] x86/hyperv: Update " Vitaly Kuznetsov
2022-07-14 9:57 ` Maxim Levitsky
2022-07-14 9:13 ` [PATCH v4 04/25] KVM: VMX: Define VMCS-to-EVMCS conversion for the new fields Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 05/25] KVM: nVMX: Support several new fields in eVMCSv1 Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 06/25] KVM: x86: hyper-v: Cache HYPERV_CPUID_NESTED_FEATURES CPUID leaf Vitaly Kuznetsov
2022-07-14 9:59 ` Maxim Levitsky
2022-07-14 9:13 ` [PATCH v4 07/25] KVM: selftests: Add ENCLS_EXITING_BITMAP{,HIGH} VMCS fields Vitaly Kuznetsov
2022-07-14 9:20 ` Kai Huang
2022-07-14 9:13 ` [PATCH v4 08/25] KVM: selftests: Switch to updated eVMCSv1 definition Vitaly Kuznetsov
2022-07-14 10:07 ` Maxim Levitsky
2022-07-14 9:13 ` [PATCH v4 09/25] KVM: VMX: nVMX: Support TSC scaling and PERF_GLOBAL_CTRL with enlightened VMCS Vitaly Kuznetsov
2022-07-21 21:58 ` Sean Christopherson
2022-07-25 17:09 ` Paolo Bonzini
2022-07-25 18:18 ` Sean Christopherson
2022-07-28 21:52 ` Paolo Bonzini
2022-07-28 22:13 ` Sean Christopherson
2022-07-28 22:24 ` Paolo Bonzini
2022-07-28 22:35 ` Sean Christopherson
2022-08-01 8:54 ` Vitaly Kuznetsov
2022-08-02 13:03 ` Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 10/25] KVM: selftests: Enable TSC scaling in evmcs selftest Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 11/25] KVM: VMX: Get rid of eVMCS specific VMX controls sanitization Vitaly Kuznetsov
2022-07-14 10:04 ` Maxim Levitsky
2022-07-14 9:13 ` [PATCH v4 12/25] KVM: VMX: Check VM_ENTRY_IA32E_MODE in setup_vmcs_config() Vitaly Kuznetsov
2022-07-21 22:00 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 13/25] KVM: VMX: Check CPU_BASED_{INTR,NMI}_WINDOW_EXITING " Vitaly Kuznetsov
2022-07-21 22:01 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 14/25] KVM: VMX: Tweak the special handling of SECONDARY_EXEC_ENCLS_EXITING " Vitaly Kuznetsov
2022-07-21 22:11 ` Sean Christopherson
2022-08-02 12:52 ` Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 15/25] KVM: VMX: Extend VMX controls macro shenanigans Vitaly Kuznetsov
2022-07-21 22:28 ` Sean Christopherson
2022-07-22 18:33 ` Sean Christopherson
2022-07-22 21:04 ` Nathan Chancellor [this message]
2022-07-22 21:38 ` Sean Christopherson
2022-07-23 1:06 ` Nathan Chancellor
2022-07-28 16:27 ` Paolo Bonzini
2022-07-14 9:13 ` [PATCH v4 16/25] KVM: VMX: Move CPU_BASED_CR8_{LOAD,STORE}_EXITING filtering out of setup_vmcs_config() Vitaly Kuznetsov
2022-07-21 22:30 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 17/25] KVM: VMX: Add missing VMEXIT controls to vmcs_config Vitaly Kuznetsov
2022-07-21 22:34 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 18/25] KVM: VMX: Add missing CPU based VM execution " Vitaly Kuznetsov
2022-07-21 22:39 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 19/25] KVM: VMX: Adjust CR3/INVPLG interception for EPT=y at runtime, not setup Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 20/25] KVM: x86: VMX: Replace some Intel model numbers with mnemonics Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 21/25] KVM: VMX: Move LOAD_IA32_PERF_GLOBAL_CTRL errata handling out of setup_vmcs_config() Vitaly Kuznetsov
2022-07-21 22:56 ` Sean Christopherson
2022-07-28 22:25 ` Paolo Bonzini
2022-07-28 22:34 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 22/25] KVM: nVMX: Always set required-1 bits of pinbased_ctls to PIN_BASED_ALWAYSON_WITHOUT_TRUE_MSR Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 23/25] KVM: nVMX: Use sanitized allowed-1 bits for VMX control MSRs Vitaly Kuznetsov
2022-07-14 9:13 ` [PATCH v4 24/25] KVM: VMX: Cache MSR_IA32_VMX_MISC in vmcs_config Vitaly Kuznetsov
2022-07-21 23:06 ` Sean Christopherson
2022-08-02 16:11 ` Vitaly Kuznetsov
2022-08-02 16:28 ` Sean Christopherson
2022-07-14 9:13 ` [PATCH v4 25/25] KVM: nVMX: Use cached host MSR_IA32_VMX_MISC value for setting up nested MSR Vitaly Kuznetsov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YtsQ5SkCJXQIuKGS@dev-arch.thelio-3990X \
--to=nathan@kernel.org \
--cc=anrayabh@linux.microsoft.com \
--cc=jmattson@google.com \
--cc=kvm@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=mlevitsk@redhat.com \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.