* [PATCH v3] KVM/x86: Move definition of __ex to x86.h
@ 2020-12-21 19:48 Uros Bizjak
2020-12-21 22:00 ` Krish Sadhukhan
2020-12-28 23:54 ` Sean Christopherson
0 siblings, 2 replies; 6+ messages in thread
From: Uros Bizjak @ 2020-12-21 19:48 UTC (permalink / raw)
To: kvm, linux-kernel
Cc: Uros Bizjak, Paolo Bonzini, Sean Christopherson, Krish Sadhukhan
Merge __kvm_handle_fault_on_reboot with its sole user
and move the definition of __ex to a common include to be
shared between VMX and SVM.
v2: Rebase to the latest kvm/queue.
v3: Incorporate changes from review comments.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
arch/x86/include/asm/kvm_host.h | 25 -------------------------
arch/x86/kvm/svm/sev.c | 2 --
arch/x86/kvm/svm/svm.c | 2 --
arch/x86/kvm/vmx/vmx.c | 4 +---
arch/x86/kvm/vmx/vmx_ops.h | 4 +---
arch/x86/kvm/x86.h | 24 ++++++++++++++++++++++++
6 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 39707e72b062..a78e4b1a5d77 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1634,31 +1634,6 @@ enum {
#define kvm_arch_vcpu_memslots_id(vcpu) ((vcpu)->arch.hflags & HF_SMM_MASK ? 1 : 0)
#define kvm_memslots_for_spte_role(kvm, role) __kvm_memslots(kvm, (role).smm)
-asmlinkage void kvm_spurious_fault(void);
-
-/*
- * Hardware virtualization extension instructions may fault if a
- * reboot turns off virtualization while processes are running.
- * Usually after catching the fault we just panic; during reboot
- * instead the instruction is ignored.
- */
-#define __kvm_handle_fault_on_reboot(insn) \
- "666: \n\t" \
- insn "\n\t" \
- "jmp 668f \n\t" \
- "667: \n\t" \
- "1: \n\t" \
- ".pushsection .discard.instr_begin \n\t" \
- ".long 1b - . \n\t" \
- ".popsection \n\t" \
- "call kvm_spurious_fault \n\t" \
- "1: \n\t" \
- ".pushsection .discard.instr_end \n\t" \
- ".long 1b - . \n\t" \
- ".popsection \n\t" \
- "668: \n\t" \
- _ASM_EXTABLE(666b, 667b)
-
#define KVM_ARCH_WANT_MMU_NOTIFIER
int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
unsigned flags);
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index e57847ff8bd2..ba492b6d37a0 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -25,8 +25,6 @@
#include "cpuid.h"
#include "trace.h"
-#define __ex(x) __kvm_handle_fault_on_reboot(x)
-
static u8 sev_enc_bit;
static int sev_flush_asids(void);
static DECLARE_RWSEM(sev_deactivate_lock);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 941e5251e13f..733d9f98a121 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -42,8 +42,6 @@
#include "svm.h"
-#define __ex(x) __kvm_handle_fault_on_reboot(x)
-
MODULE_AUTHOR("Qumranet");
MODULE_LICENSE("GPL");
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 75c9c6a0a3a4..b82f2689f2d7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2320,9 +2320,7 @@ static void vmclear_local_loaded_vmcss(void)
}
-/* Just like cpu_vmxoff(), but with the __kvm_handle_fault_on_reboot()
- * tricks.
- */
+/* Just like cpu_vmxoff(), but with the fault handling. */
static void kvm_cpu_vmxoff(void)
{
asm volatile (__ex("vmxoff"));
diff --git a/arch/x86/kvm/vmx/vmx_ops.h b/arch/x86/kvm/vmx/vmx_ops.h
index 692b0c31c9c8..7e3cb53c413f 100644
--- a/arch/x86/kvm/vmx/vmx_ops.h
+++ b/arch/x86/kvm/vmx/vmx_ops.h
@@ -4,13 +4,11 @@
#include <linux/nospec.h>
-#include <asm/kvm_host.h>
#include <asm/vmx.h>
#include "evmcs.h"
#include "vmcs.h"
-
-#define __ex(x) __kvm_handle_fault_on_reboot(x)
+#include "x86.h"
asmlinkage void vmread_error(unsigned long field, bool fault);
__attribute__((regparm(0))) void vmread_error_trampoline(unsigned long field,
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index c5ee0f5ce0f1..5b16d2b5c3bc 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -8,6 +8,30 @@
#include "kvm_cache_regs.h"
#include "kvm_emulate.h"
+asmlinkage void kvm_spurious_fault(void);
+
+/*
+ * Handle a fault on a hardware virtualization (VMX or SVM) instruction.
+ *
+ * Hardware virtualization extension instructions may fault if a reboot turns
+ * off virtualization while processes are running. Usually after catching the
+ * fault we just panic; during reboot instead the instruction is ignored.
+ */
+#define __ex(insn) \
+ "666: " insn "\n" \
+ " jmp 669f\n" \
+ "667:\n" \
+ " .pushsection .discard.instr_begin\n" \
+ " .long 667b - .\n" \
+ " .popsection\n" \
+ " call kvm_spurious_fault\n" \
+ "668:\n" \
+ " .pushsection .discard.instr_end\n" \
+ " .long 668b - .\n" \
+ " .popsection\n" \
+ "669:\n" \
+ _ASM_EXTABLE(666b, 667b)
+
#define KVM_DEFAULT_PLE_GAP 128
#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
#define KVM_DEFAULT_PLE_WINDOW_GROW 2
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] KVM/x86: Move definition of __ex to x86.h
2020-12-21 19:48 [PATCH v3] KVM/x86: Move definition of __ex to x86.h Uros Bizjak
@ 2020-12-21 22:00 ` Krish Sadhukhan
2020-12-21 22:10 ` Uros Bizjak
2020-12-21 23:05 ` Sean Christopherson
2020-12-28 23:54 ` Sean Christopherson
1 sibling, 2 replies; 6+ messages in thread
From: Krish Sadhukhan @ 2020-12-21 22:00 UTC (permalink / raw)
To: Uros Bizjak, kvm, linux-kernel; +Cc: Paolo Bonzini, Sean Christopherson
On 12/21/20 11:48 AM, Uros Bizjak wrote:
> Merge __kvm_handle_fault_on_reboot with its sole user
> and move the definition of __ex to a common include to be
> shared between VMX and SVM.
>
> v2: Rebase to the latest kvm/queue.
>
> v3: Incorporate changes from review comments.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
> arch/x86/include/asm/kvm_host.h | 25 -------------------------
> arch/x86/kvm/svm/sev.c | 2 --
> arch/x86/kvm/svm/svm.c | 2 --
> arch/x86/kvm/vmx/vmx.c | 4 +---
> arch/x86/kvm/vmx/vmx_ops.h | 4 +---
> arch/x86/kvm/x86.h | 24 ++++++++++++++++++++++++
> 6 files changed, 26 insertions(+), 35 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 39707e72b062..a78e4b1a5d77 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1634,31 +1634,6 @@ enum {
> #define kvm_arch_vcpu_memslots_id(vcpu) ((vcpu)->arch.hflags & HF_SMM_MASK ? 1 : 0)
> #define kvm_memslots_for_spte_role(kvm, role) __kvm_memslots(kvm, (role).smm)
>
> -asmlinkage void kvm_spurious_fault(void);
> -
> -/*
> - * Hardware virtualization extension instructions may fault if a
> - * reboot turns off virtualization while processes are running.
> - * Usually after catching the fault we just panic; during reboot
> - * instead the instruction is ignored.
> - */
> -#define __kvm_handle_fault_on_reboot(insn) \
> - "666: \n\t" \
> - insn "\n\t" \
> - "jmp 668f \n\t" \
> - "667: \n\t" \
> - "1: \n\t" \
> - ".pushsection .discard.instr_begin \n\t" \
> - ".long 1b - . \n\t" \
> - ".popsection \n\t" \
> - "call kvm_spurious_fault \n\t" \
> - "1: \n\t" \
> - ".pushsection .discard.instr_end \n\t" \
> - ".long 1b - . \n\t" \
> - ".popsection \n\t" \
> - "668: \n\t" \
> - _ASM_EXTABLE(666b, 667b)
> -
> #define KVM_ARCH_WANT_MMU_NOTIFIER
> int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
> unsigned flags);
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index e57847ff8bd2..ba492b6d37a0 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -25,8 +25,6 @@
> #include "cpuid.h"
> #include "trace.h"
>
> -#define __ex(x) __kvm_handle_fault_on_reboot(x)
> -
> static u8 sev_enc_bit;
> static int sev_flush_asids(void);
> static DECLARE_RWSEM(sev_deactivate_lock);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 941e5251e13f..733d9f98a121 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -42,8 +42,6 @@
>
> #include "svm.h"
>
> -#define __ex(x) __kvm_handle_fault_on_reboot(x)
> -
> MODULE_AUTHOR("Qumranet");
> MODULE_LICENSE("GPL");
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 75c9c6a0a3a4..b82f2689f2d7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2320,9 +2320,7 @@ static void vmclear_local_loaded_vmcss(void)
> }
>
>
> -/* Just like cpu_vmxoff(), but with the __kvm_handle_fault_on_reboot()
> - * tricks.
> - */
> +/* Just like cpu_vmxoff(), but with the fault handling. */
> static void kvm_cpu_vmxoff(void)
> {
> asm volatile (__ex("vmxoff"));
> diff --git a/arch/x86/kvm/vmx/vmx_ops.h b/arch/x86/kvm/vmx/vmx_ops.h
> index 692b0c31c9c8..7e3cb53c413f 100644
> --- a/arch/x86/kvm/vmx/vmx_ops.h
> +++ b/arch/x86/kvm/vmx/vmx_ops.h
> @@ -4,13 +4,11 @@
>
> #include <linux/nospec.h>
>
> -#include <asm/kvm_host.h>
> #include <asm/vmx.h>
>
> #include "evmcs.h"
> #include "vmcs.h"
> -
> -#define __ex(x) __kvm_handle_fault_on_reboot(x)
> +#include "x86.h"
>
> asmlinkage void vmread_error(unsigned long field, bool fault);
> __attribute__((regparm(0))) void vmread_error_trampoline(unsigned long field,
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index c5ee0f5ce0f1..5b16d2b5c3bc 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -8,6 +8,30 @@
> #include "kvm_cache_regs.h"
> #include "kvm_emulate.h"
>
> +asmlinkage void kvm_spurious_fault(void);
> +
> +/*
> + * Handle a fault on a hardware virtualization (VMX or SVM) instruction.
> + *
> + * Hardware virtualization extension instructions may fault if a reboot turns
> + * off virtualization while processes are running. Usually after catching the
> + * fault we just panic; during reboot instead the instruction is ignored.
> + */
> +#define __ex(insn) \
While the previous name was too elaborate, this new name is very
cryptic. Unless we are saving for space, it's better to give a somewhat
descriptive name.
> + "666: " insn "\n" \
> + " jmp 669f\n" \
> + "667:\n" \
> + " .pushsection .discard.instr_begin\n" \
> + " .long 667b - .\n" \
> + " .popsection\n" \
> + " call kvm_spurious_fault\n" \
> + "668:\n" \
> + " .pushsection .discard.instr_end\n" \
> + " .long 668b - .\n" \
> + " .popsection\n" \
> + "669:\n" \
> + _ASM_EXTABLE(666b, 667b)
> +
> #define KVM_DEFAULT_PLE_GAP 128
> #define KVM_VMX_DEFAULT_PLE_WINDOW 4096
> #define KVM_DEFAULT_PLE_WINDOW_GROW 2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] KVM/x86: Move definition of __ex to x86.h
2020-12-21 22:00 ` Krish Sadhukhan
@ 2020-12-21 22:10 ` Uros Bizjak
2020-12-21 23:05 ` Sean Christopherson
1 sibling, 0 replies; 6+ messages in thread
From: Uros Bizjak @ 2020-12-21 22:10 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: kvm, LKML, Paolo Bonzini, Sean Christopherson
On Mon, Dec 21, 2020 at 11:01 PM Krish Sadhukhan
<krish.sadhukhan@oracle.com> wrote:
>
>
> On 12/21/20 11:48 AM, Uros Bizjak wrote:
> > Merge __kvm_handle_fault_on_reboot with its sole user
> > and move the definition of __ex to a common include to be
> > shared between VMX and SVM.
> >
> > v2: Rebase to the latest kvm/queue.
> >
> > v3: Incorporate changes from review comments.
> >
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> > ---
> > arch/x86/include/asm/kvm_host.h | 25 -------------------------
> > arch/x86/kvm/svm/sev.c | 2 --
> > arch/x86/kvm/svm/svm.c | 2 --
> > arch/x86/kvm/vmx/vmx.c | 4 +---
> > arch/x86/kvm/vmx/vmx_ops.h | 4 +---
> > arch/x86/kvm/x86.h | 24 ++++++++++++++++++++++++
> > 6 files changed, 26 insertions(+), 35 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 39707e72b062..a78e4b1a5d77 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1634,31 +1634,6 @@ enum {
> > #define kvm_arch_vcpu_memslots_id(vcpu) ((vcpu)->arch.hflags & HF_SMM_MASK ? 1 : 0)
> > #define kvm_memslots_for_spte_role(kvm, role) __kvm_memslots(kvm, (role).smm)
> >
> > -asmlinkage void kvm_spurious_fault(void);
> > -
> > -/*
> > - * Hardware virtualization extension instructions may fault if a
> > - * reboot turns off virtualization while processes are running.
> > - * Usually after catching the fault we just panic; during reboot
> > - * instead the instruction is ignored.
> > - */
> > -#define __kvm_handle_fault_on_reboot(insn) \
> > - "666: \n\t" \
> > - insn "\n\t" \
> > - "jmp 668f \n\t" \
> > - "667: \n\t" \
> > - "1: \n\t" \
> > - ".pushsection .discard.instr_begin \n\t" \
> > - ".long 1b - . \n\t" \
> > - ".popsection \n\t" \
> > - "call kvm_spurious_fault \n\t" \
> > - "1: \n\t" \
> > - ".pushsection .discard.instr_end \n\t" \
> > - ".long 1b - . \n\t" \
> > - ".popsection \n\t" \
> > - "668: \n\t" \
> > - _ASM_EXTABLE(666b, 667b)
> > -
> > #define KVM_ARCH_WANT_MMU_NOTIFIER
> > int kvm_unmap_hva_range(struct kvm *kvm, unsigned long start, unsigned long end,
> > unsigned flags);
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index e57847ff8bd2..ba492b6d37a0 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -25,8 +25,6 @@
> > #include "cpuid.h"
> > #include "trace.h"
> >
> > -#define __ex(x) __kvm_handle_fault_on_reboot(x)
> > -
> > static u8 sev_enc_bit;
> > static int sev_flush_asids(void);
> > static DECLARE_RWSEM(sev_deactivate_lock);
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index 941e5251e13f..733d9f98a121 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -42,8 +42,6 @@
> >
> > #include "svm.h"
> >
> > -#define __ex(x) __kvm_handle_fault_on_reboot(x)
> > -
> > MODULE_AUTHOR("Qumranet");
> > MODULE_LICENSE("GPL");
> >
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 75c9c6a0a3a4..b82f2689f2d7 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -2320,9 +2320,7 @@ static void vmclear_local_loaded_vmcss(void)
> > }
> >
> >
> > -/* Just like cpu_vmxoff(), but with the __kvm_handle_fault_on_reboot()
> > - * tricks.
> > - */
> > +/* Just like cpu_vmxoff(), but with the fault handling. */
> > static void kvm_cpu_vmxoff(void)
> > {
> > asm volatile (__ex("vmxoff"));
> > diff --git a/arch/x86/kvm/vmx/vmx_ops.h b/arch/x86/kvm/vmx/vmx_ops.h
> > index 692b0c31c9c8..7e3cb53c413f 100644
> > --- a/arch/x86/kvm/vmx/vmx_ops.h
> > +++ b/arch/x86/kvm/vmx/vmx_ops.h
> > @@ -4,13 +4,11 @@
> >
> > #include <linux/nospec.h>
> >
> > -#include <asm/kvm_host.h>
> > #include <asm/vmx.h>
> >
> > #include "evmcs.h"
> > #include "vmcs.h"
> > -
> > -#define __ex(x) __kvm_handle_fault_on_reboot(x)
> > +#include "x86.h"
> >
> > asmlinkage void vmread_error(unsigned long field, bool fault);
> > __attribute__((regparm(0))) void vmread_error_trampoline(unsigned long field,
> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > index c5ee0f5ce0f1..5b16d2b5c3bc 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -8,6 +8,30 @@
> > #include "kvm_cache_regs.h"
> > #include "kvm_emulate.h"
> >
> > +asmlinkage void kvm_spurious_fault(void);
> > +
> > +/*
> > + * Handle a fault on a hardware virtualization (VMX or SVM) instruction.
> > + *
> > + * Hardware virtualization extension instructions may fault if a reboot turns
> > + * off virtualization while processes are running. Usually after catching the
> > + * fault we just panic; during reboot instead the instruction is ignored.
> > + */
> > +#define __ex(insn) \
>
>
> While the previous name was too elaborate, this new name is very
> cryptic. Unless we are saving for space, it's better to give a somewhat
> descriptive name.
Then we will need to update all usage sites to a new name, something I
tried to avoid.
Uros.
> > + "666: " insn "\n" \
> > + " jmp 669f\n" \
> > + "667:\n" \
> > + " .pushsection .discard.instr_begin\n" \
> > + " .long 667b - .\n" \
> > + " .popsection\n" \
> > + " call kvm_spurious_fault\n" \
> > + "668:\n" \
> > + " .pushsection .discard.instr_end\n" \
> > + " .long 668b - .\n" \
> > + " .popsection\n" \
> > + "669:\n" \
> > + _ASM_EXTABLE(666b, 667b)
> > +
> > #define KVM_DEFAULT_PLE_GAP 128
> > #define KVM_VMX_DEFAULT_PLE_WINDOW 4096
> > #define KVM_DEFAULT_PLE_WINDOW_GROW 2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] KVM/x86: Move definition of __ex to x86.h
2020-12-21 22:00 ` Krish Sadhukhan
2020-12-21 22:10 ` Uros Bizjak
@ 2020-12-21 23:05 ` Sean Christopherson
1 sibling, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2020-12-21 23:05 UTC (permalink / raw)
To: Krish Sadhukhan; +Cc: Uros Bizjak, kvm, linux-kernel, Paolo Bonzini
On Mon, Dec 21, 2020, Krish Sadhukhan wrote:
>
> On 12/21/20 11:48 AM, Uros Bizjak wrote:
> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > index c5ee0f5ce0f1..5b16d2b5c3bc 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -8,6 +8,30 @@
> > #include "kvm_cache_regs.h"
> > #include "kvm_emulate.h"
> > +asmlinkage void kvm_spurious_fault(void);
> > +
> > +/*
> > + * Handle a fault on a hardware virtualization (VMX or SVM) instruction.
> > + *
> > + * Hardware virtualization extension instructions may fault if a reboot turns
> > + * off virtualization while processes are running. Usually after catching the
> > + * fault we just panic; during reboot instead the instruction is ignored.
> > + */
> > +#define __ex(insn) \
>
>
> While the previous name was too elaborate, this new name is very cryptic.
> Unless we are saving for space,it's better to give a somewhat descriptive
> name.
We are saving for space in a way. Not so much to actually save lines of code,
but to avoid stealing the focus from the code that matters. __ex() is cryptic
for the completely unfamiliar, but I'm worried that anything more verbose will
harm the readability of the code where it is used, which is usually what's more
important in the long run.
__ex() does have some meaning, as it's connected to the various ex_handler_*()
helpers. ex_handle() is the best semi-verbose alternative that I can think of,
but even that is too long for my liking when reading the inline asm flows. And
it's not like ex_handle() tells the whole story; the reader still has to go to
the definition to understand what it does, or worse, will make incorrect
assumptions about how exceptions are handled.
E.g. with the short version, my eyes gravitate toward vmxoff/vmsave without
getting stuck on the verbose wrapper.
asm volatile (__ex("vmxoff"));
asm volatile(__ex("vmsave") : : "a" (__sme_page_pa(sd->save_area)) : "memory");
vs.
asm volatile (ex_handle("vmxoff"));
asm volatile(ex_handle("vmsave") : : "a" (__sme_page_pa(sd->save_area)) : "memory");
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] KVM/x86: Move definition of __ex to x86.h
2020-12-21 19:48 [PATCH v3] KVM/x86: Move definition of __ex to x86.h Uros Bizjak
2020-12-21 22:00 ` Krish Sadhukhan
@ 2020-12-28 23:54 ` Sean Christopherson
2020-12-29 22:14 ` Sean Christopherson
1 sibling, 1 reply; 6+ messages in thread
From: Sean Christopherson @ 2020-12-28 23:54 UTC (permalink / raw)
To: Uros Bizjak; +Cc: kvm, linux-kernel, Paolo Bonzini, Krish Sadhukhan
On Mon, Dec 21, 2020, Uros Bizjak wrote:
> Merge __kvm_handle_fault_on_reboot with its sole user
> and move the definition of __ex to a common include to be
> shared between VMX and SVM.
>
> v2: Rebase to the latest kvm/queue.
>
> v3: Incorporate changes from review comments.
The v2, v3, ... vN patch history should go below the '---' so that it doesn't
need to be manually stripped when applying.
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
vN stuff down here
> arch/x86/include/asm/kvm_host.h | 25 -------------------------
> arch/x86/kvm/svm/sev.c | 2 --
> arch/x86/kvm/svm/svm.c | 2 --
> arch/x86/kvm/vmx/vmx.c | 4 +---
> arch/x86/kvm/vmx/vmx_ops.h | 4 +---
> arch/x86/kvm/x86.h | 24 ++++++++++++++++++++++++
> 6 files changed, 26 insertions(+), 35 deletions(-)
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] KVM/x86: Move definition of __ex to x86.h
2020-12-28 23:54 ` Sean Christopherson
@ 2020-12-29 22:14 ` Sean Christopherson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2020-12-29 22:14 UTC (permalink / raw)
To: Uros Bizjak; +Cc: kvm, linux-kernel, Paolo Bonzini, Krish Sadhukhan
On Mon, Dec 28, 2020, Sean Christopherson wrote:
> On Mon, Dec 21, 2020, Uros Bizjak wrote:
> > Merge __kvm_handle_fault_on_reboot with its sole user
> > and move the definition of __ex to a common include to be
> > shared between VMX and SVM.
> >
> > v2: Rebase to the latest kvm/queue.
> >
> > v3: Incorporate changes from review comments.
>
> The v2, v3, ... vN patch history should go below the '---' so that it doesn't
> need to be manually stripped when applying.
>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Uros Bizjak <ubizjak@gmail.com>
> > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> > ---
>
> vN stuff down here
>
> > arch/x86/include/asm/kvm_host.h | 25 -------------------------
> > arch/x86/kvm/svm/sev.c | 2 --
> > arch/x86/kvm/svm/svm.c | 2 --
> > arch/x86/kvm/vmx/vmx.c | 4 +---
> > arch/x86/kvm/vmx/vmx_ops.h | 4 +---
> > arch/x86/kvm/x86.h | 24 ++++++++++++++++++++++++
> > 6 files changed, 26 insertions(+), 35 deletions(-)
>
> Reviewed-by: Sean Christopherson <seanjc@google.com>
Paolo, can you hold off on queuing this patch? Long story short, this jogged my
memory for something tangentially related and I ended up with series that kills
off __ex() / ____kvm_handle_fault_on_reboot() completely. It's coded up, I just
need to test. I'm OOO for a few days, will hopefully get it posted next week.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-29 22:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-21 19:48 [PATCH v3] KVM/x86: Move definition of __ex to x86.h Uros Bizjak
2020-12-21 22:00 ` Krish Sadhukhan
2020-12-21 22:10 ` Uros Bizjak
2020-12-21 23:05 ` Sean Christopherson
2020-12-28 23:54 ` Sean Christopherson
2020-12-29 22:14 ` Sean Christopherson
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.