Messages in this thread Patch in this message | | | Subject | Re: [PATCH] Fix undefined operation VMXOFF during reboot and crash | From | Randy Dunlap <> | Date | Wed, 10 Jun 2020 12:36:13 -0700 |
| |
Hi David,
If you happen to make a v2 of this patch, there are a few comments that begin with "/**" but they are not kernel-doc comments, so they should instead begin with just "/*". Please see below.
(and you did not introduce this comment style here.)
On 6/10/20 11:12 AM, David P. Reed wrote: > > Signed-off-by: David P. Reed <dpreed@deepplum.com> > --- > arch/x86/include/asm/virtext.h | 24 ++++++++++++---- > arch/x86/kernel/reboot.c | 13 ++------- > arch/x86/kernel/traps.c | 52 ++++++++++++++++++++++++++++++++-- > 3 files changed, 71 insertions(+), 18 deletions(-) > > diff --git a/arch/x86/include/asm/virtext.h b/arch/x86/include/asm/virtext.h > index 9aad0e0876fb..ea2d67191684 100644 > --- a/arch/x86/include/asm/virtext.h > +++ b/arch/x86/include/asm/virtext.h > @@ -13,12 +13,16 @@ > #ifndef _ASM_X86_VIRTEX_H > #define _ASM_X86_VIRTEX_H > > +#include <linux/percpu.h> > + > #include <asm/processor.h> > > #include <asm/vmx.h> > #include <asm/svm.h> > #include <asm/tlbflush.h> > > +DECLARE_PER_CPU_READ_MOSTLY(int, doing_emergency_vmxoff); > + > /* > * VMX functions: > */ > @@ -33,8 +37,8 @@ static inline int cpu_has_vmx(void) > /** Disable VMX on the current CPU
just /* Disable VMX on the current CPU
> * > * vmxoff causes a undefined-opcode exception if vmxon was not run > - * on the CPU previously. Only call this function if you know VMX > - * is enabled. > + * on the CPU previously. Only call this function directly if you know VMX > + * is enabled *and* CPU is in VMX root operation. > */ > static inline void cpu_vmxoff(void) > { > @@ -47,17 +51,25 @@ static inline int cpu_vmx_enabled(void) > return __read_cr4() & X86_CR4_VMXE; > } > > -/** Disable VMX if it is enabled on the current CPU > +/** Force disable VMX if it is enabled on the current CPU.
just /* Force disable VMX if it is enabled on the current CPU.
> + * Note that if CPU is not in VMX root operation this > + * VMXOFF will fault an undefined operation fault. > + * So the 'doing_emergency_vmxoff' percpu flag is set, > + * the trap handler for just restarts execution after > + * the VMXOFF instruction. > * > - * You shouldn't call this if cpu_has_vmx() returns 0. > + * You shouldn't call this directly if cpu_has_vmx() returns 0. > */ > static inline void __cpu_emergency_vmxoff(void) > { > - if (cpu_vmx_enabled()) > + if (cpu_vmx_enabled()) { > + this_cpu_write(doing_emergency_vmxoff, 1); > cpu_vmxoff(); > + this_cpu_write(doing_emergency_vmxoff, 0); > + } > } > > -/** Disable VMX if it is supported and enabled on the current CPU > +/** Force disable VMX if it is supported and enabled on the current CPU
ditto.
> */ > static inline void cpu_emergency_vmxoff(void) > {
thanks. -- ~Randy
| |