[PATCH v9 1/2] x86/kexec: VMCLEAR VMCSs loaded on all cpus if necessary
This patch provides a way to VMCLEAR VMCSs related to guests
on all cpus before executing the VMXOFF when doing kdump. This
is used to ensure the VMCSs in the vmcore updated and
non-corrupted.
Signed-off-by: Zhang Yanfei
---
arch/x86/include/asm/kexec.h |2 ++
arch/x86/kernel/crash.c | 25 +
2 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
index 317ff17..28feeba 100644
--- a/arch/x86/include/asm/kexec.h
+++ b/arch/x86/include/asm/kexec.h
@@ -163,6 +163,8 @@ struct kimage_arch {
};
#endif
+extern void (*crash_vmclear_loaded_vmcss)(void);
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_X86_KEXEC_H */
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 13ad899..4a2a12f 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
@@ -29,6 +30,20 @@
#include
int in_crash_kexec;
+
+/*
+ * This is used to VMCLEAR all VMCSs loaded on the
+ * processor. And when loading kvm_intel module, the
+ * callback function pointer will be assigned.
+ */
+void (*crash_vmclear_loaded_vmcss)(void) = NULL;
+EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
+
+static inline void cpu_emergency_vmclear_loaded_vmcss(void)
+{
+ if (crash_vmclear_loaded_vmcss)
+ crash_vmclear_loaded_vmcss();
+}
#if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
@@ -46,6 +61,11 @@ static void kdump_nmi_callback(int cpu, struct pt_regs *regs)
#endif
crash_save_cpu(regs, cpu);
+ /*
+* VMCLEAR VMCSs loaded on all cpus if needed.
+*/
+ cpu_emergency_vmclear_loaded_vmcss();
+
/* Disable VMX or SVM if needed.
*
* We need to disable virtualization on all CPUs.
@@ -88,6 +108,11 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
kdump_nmi_shootdown_cpus();
+ /*
+* VMCLEAR VMCSs loaded on this cpu if needed.
+*/
+ cpu_emergency_vmclear_loaded_vmcss();
+
/* Booting kdump kernel with VMX or SVM enabled won't work,
* because (among other limitations) we can't disable paging
* with the virt flags.
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/2] x86/kexec: VMCLEAR VMCSs loaded on all cpus if necessary
Eric, can you ACK it?
On Tue, Nov 27, 2012 at 11:26:02AM +0800, Zhang Yanfei wrote:
> This patch provides a way to VMCLEAR VMCSs related to guests
> on all cpus before executing the VMXOFF when doing kdump. This
> is used to ensure the VMCSs in the vmcore updated and
> non-corrupted.
>
> Signed-off-by: Zhang Yanfei
> ---
> arch/x86/include/asm/kexec.h |2 ++
> arch/x86/kernel/crash.c | 25 +
> 2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index 317ff17..28feeba 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -163,6 +163,8 @@ struct kimage_arch {
> };
> #endif
>
> +extern void (*crash_vmclear_loaded_vmcss)(void);
> +
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_X86_KEXEC_H */
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 13ad899..4a2a12f 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -29,6 +30,20 @@
> #include
>
> int in_crash_kexec;
> +
> +/*
> + * This is used to VMCLEAR all VMCSs loaded on the
> + * processor. And when loading kvm_intel module, the
> + * callback function pointer will be assigned.
> + */
> +void (*crash_vmclear_loaded_vmcss)(void) = NULL;
> +EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
> +
> +static inline void cpu_emergency_vmclear_loaded_vmcss(void)
> +{
> + if (crash_vmclear_loaded_vmcss)
> + crash_vmclear_loaded_vmcss();
> +}
>
> #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>
> @@ -46,6 +61,11 @@ static void kdump_nmi_callback(int cpu, struct pt_regs
> *regs)
> #endif
> crash_save_cpu(regs, cpu);
>
> + /*
> + * VMCLEAR VMCSs loaded on all cpus if needed.
> + */
> + cpu_emergency_vmclear_loaded_vmcss();
> +
> /* Disable VMX or SVM if needed.
>*
>* We need to disable virtualization on all CPUs.
> @@ -88,6 +108,11 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
>
> kdump_nmi_shootdown_cpus();
>
> + /*
> + * VMCLEAR VMCSs loaded on this cpu if needed.
> + */
> + cpu_emergency_vmclear_loaded_vmcss();
> +
> /* Booting kdump kernel with VMX or SVM enabled won't work,
>* because (among other limitations) we can't disable paging
>* with the virt flags.
> --
> 1.7.1
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/2] x86/kexec: VMCLEAR VMCSs loaded on all cpus if necessary
On Tue, Nov 27, 2012 at 02:18:47PM +0200, Gleb Natapov wrote:
> Eric, can you ACK it?
>
Eric, ping.
> On Tue, Nov 27, 2012 at 11:26:02AM +0800, Zhang Yanfei wrote:
> > This patch provides a way to VMCLEAR VMCSs related to guests
> > on all cpus before executing the VMXOFF when doing kdump. This
> > is used to ensure the VMCSs in the vmcore updated and
> > non-corrupted.
> >
> > Signed-off-by: Zhang Yanfei
> > ---
> > arch/x86/include/asm/kexec.h |2 ++
> > arch/x86/kernel/crash.c | 25 +
> > 2 files changed, 27 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> > index 317ff17..28feeba 100644
> > --- a/arch/x86/include/asm/kexec.h
> > +++ b/arch/x86/include/asm/kexec.h
> > @@ -163,6 +163,8 @@ struct kimage_arch {
> > };
> > #endif
> >
> > +extern void (*crash_vmclear_loaded_vmcss)(void);
> > +
> > #endif /* __ASSEMBLY__ */
> >
> > #endif /* _ASM_X86_KEXEC_H */
> > diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> > index 13ad899..4a2a12f 100644
> > --- a/arch/x86/kernel/crash.c
> > +++ b/arch/x86/kernel/crash.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include
> > #include
> > @@ -29,6 +30,20 @@
> > #include
> >
> > int in_crash_kexec;
> > +
> > +/*
> > + * This is used to VMCLEAR all VMCSs loaded on the
> > + * processor. And when loading kvm_intel module, the
> > + * callback function pointer will be assigned.
> > + */
> > +void (*crash_vmclear_loaded_vmcss)(void) = NULL;
> > +EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
> > +
> > +static inline void cpu_emergency_vmclear_loaded_vmcss(void)
> > +{
> > + if (crash_vmclear_loaded_vmcss)
> > + crash_vmclear_loaded_vmcss();
> > +}
> >
> > #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
> >
> > @@ -46,6 +61,11 @@ static void kdump_nmi_callback(int cpu, struct pt_regs
> > *regs)
> > #endif
> > crash_save_cpu(regs, cpu);
> >
> > + /*
> > +* VMCLEAR VMCSs loaded on all cpus if needed.
> > +*/
> > + cpu_emergency_vmclear_loaded_vmcss();
> > +
> > /* Disable VMX or SVM if needed.
> > *
> > * We need to disable virtualization on all CPUs.
> > @@ -88,6 +108,11 @@ void native_machine_crash_shutdown(struct pt_regs *regs)
> >
> > kdump_nmi_shootdown_cpus();
> >
> > + /*
> > +* VMCLEAR VMCSs loaded on this cpu if needed.
> > +*/
> > + cpu_emergency_vmclear_loaded_vmcss();
> > +
> > /* Booting kdump kernel with VMX or SVM enabled won't work,
> > * because (among other limitations) we can't disable paging
> > * with the virt flags.
> > --
> > 1.7.1
>
> --
> Gleb.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/2] x86/kexec: VMCLEAR VMCSs loaded on all cpus if necessary
Zhang Yanfei writes:
> This patch provides a way to VMCLEAR VMCSs related to guests
> on all cpus before executing the VMXOFF when doing kdump. This
> is used to ensure the VMCSs in the vmcore updated and
> non-corrupted.
Apologies for the delay I have been travelling, and I wanted
to at least read through the code.
Overall I think this is good but I have one nit, and I see one real
problem with this code.
> +/*
> + * This is used to VMCLEAR all VMCSs loaded on the
> + * processor. And when loading kvm_intel module, the
> + * callback function pointer will be assigned.
> + */
> +void (*crash_vmclear_loaded_vmcss)(void) = NULL;
> +EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
> +
> +static inline void cpu_emergency_vmclear_loaded_vmcss(void)
> +{
> + if (crash_vmclear_loaded_vmcss)
> + crash_vmclear_loaded_vmcss();
> +}
The nit is the use of emergency instead of crash in the name.
The problem is that this is potentially a NULL pointer dereference if
kvm-intel is removed. The easist fix would be in your second patch to
just make it impossible to unload the kvm-intel module. Otherwise
there the deference of crash_vmclear_loaded_vmcss needs to be rcu
protected, with a syncrhonize_rcu after the pointer is set to NULL in
the unload path.
Otherwise I have no objections to this code.
Eric
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/2] x86/kexec: VMCLEAR VMCSs loaded on all cpus if necessary
于 2012年12月05日 04:14, Eric W. Biederman 写道:
> Zhang Yanfei writes:
>
>> This patch provides a way to VMCLEAR VMCSs related to guests
>> on all cpus before executing the VMXOFF when doing kdump. This
>> is used to ensure the VMCSs in the vmcore updated and
>> non-corrupted.
>
> Apologies for the delay I have been travelling, and I wanted
> to at least read through the code.
>
> Overall I think this is good but I have one nit, and I see one real
> problem with this code.
>
>> +/*
>> + * This is used to VMCLEAR all VMCSs loaded on the
>> + * processor. And when loading kvm_intel module, the
>> + * callback function pointer will be assigned.
>> + */
>> +void (*crash_vmclear_loaded_vmcss)(void) = NULL;
>> +EXPORT_SYMBOL_GPL(crash_vmclear_loaded_vmcss);
>> +
>> +static inline void cpu_emergency_vmclear_loaded_vmcss(void)
>> +{
>> +if (crash_vmclear_loaded_vmcss)
>> +crash_vmclear_loaded_vmcss();
>> +}
>
> The nit is the use of emergency instead of crash in the name.
ok, emergency -> crash
>
> The problem is that this is potentially a NULL pointer dereference if
> kvm-intel is removed. The easist fix would be in your second patch to
> just make it impossible to unload the kvm-intel module. Otherwise
> there the deference of crash_vmclear_loaded_vmcss needs to be rcu
> protected, with a syncrhonize_rcu after the pointer is set to NULL in
> the unload path.
Ah, thanks for this comment.
I think I will use the rcu machanism to solve the problem.
>
> Otherwise I have no objections to this code.
Thanks for your review. I will update the patch and resend it.
Thanks
Zhang Yanfei
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
