From: Chao Gao <[email protected]>
The CPU STARTING section doesn't allow callbacks to fail. Move KVM's
hotplug callback to ONLINE section so that it can abort onlining a CPU in
certain cases to avoid potentially breaking VMs running on existing CPUs.
For example, when kvm fails to enable hardware virtualization on the
hotplugged CPU.
Place KVM's hotplug state before CPUHP_AP_SCHED_WAIT_EMPTY as it ensures
when offlining a CPU, all user tasks and non-pinned kernel tasks have left
the CPU, i.e. there cannot be a vCPU task around. So, it is safe for KVM's
CPU offline callback to disable hardware virtualization at that point.
Likewise, KVM's online callback can enable hardware virtualization before
any vCPU task gets a chance to run on hotplugged CPUs.
KVM's CPU hotplug callbacks are renamed as well.
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: Chao Gao <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
include/linux/cpuhotplug.h | 2 +-
virt/kvm/kvm_main.c | 30 ++++++++++++++++++++++--------
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index f61447913db9..7972bd63e0cb 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -185,7 +185,6 @@ enum cpuhp_state {
CPUHP_AP_CSKY_TIMER_STARTING,
CPUHP_AP_TI_GP_TIMER_STARTING,
CPUHP_AP_HYPERV_TIMER_STARTING,
- CPUHP_AP_KVM_STARTING,
CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
CPUHP_AP_KVM_ARM_VGIC_STARTING,
CPUHP_AP_KVM_ARM_TIMER_STARTING,
@@ -203,6 +202,7 @@ enum cpuhp_state {
/* Online section invoked on the hotplugged CPU from the hotplug thread */
CPUHP_AP_ONLINE_IDLE,
+ CPUHP_AP_KVM_ONLINE,
CPUHP_AP_SCHED_WAIT_EMPTY,
CPUHP_AP_SMPBOOT_THREADS,
CPUHP_AP_X86_VDSO_VMA_ONLINE,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4243a9541543..6ce6f27f2934 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5010,13 +5010,27 @@ static void hardware_enable_nolock(void *junk)
}
}
-static int kvm_starting_cpu(unsigned int cpu)
+static int kvm_online_cpu(unsigned int cpu)
{
+ int ret = 0;
+
raw_spin_lock(&kvm_count_lock);
- if (kvm_usage_count)
+ /*
+ * Abort the CPU online process if hardware virtualization cannot
+ * be enabled. Otherwise running VMs would encounter unrecoverable
+ * errors when scheduled to this CPU.
+ */
+ if (kvm_usage_count) {
+ WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
+
hardware_enable_nolock(NULL);
+ if (atomic_read(&hardware_enable_failed)) {
+ atomic_set(&hardware_enable_failed, 0);
+ ret = -EIO;
+ }
+ }
raw_spin_unlock(&kvm_count_lock);
- return 0;
+ return ret;
}
static void hardware_disable_nolock(void *junk)
@@ -5029,7 +5043,7 @@ static void hardware_disable_nolock(void *junk)
kvm_arch_hardware_disable();
}
-static int kvm_dying_cpu(unsigned int cpu)
+static int kvm_offline_cpu(unsigned int cpu)
{
raw_spin_lock(&kvm_count_lock);
if (kvm_usage_count)
@@ -5840,8 +5854,8 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
goto out_free_2;
}
- r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_STARTING, "kvm/cpu:starting",
- kvm_starting_cpu, kvm_dying_cpu);
+ r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online",
+ kvm_online_cpu, kvm_offline_cpu);
if (r)
goto out_free_2;
register_reboot_notifier(&kvm_reboot_notifier);
@@ -5902,7 +5916,7 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
kmem_cache_destroy(kvm_vcpu_cache);
out_free_3:
unregister_reboot_notifier(&kvm_reboot_notifier);
- cpuhp_remove_state_nocalls(CPUHP_AP_KVM_STARTING);
+ cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE);
out_free_2:
kvm_arch_hardware_unsetup();
out_free_1:
@@ -5928,7 +5942,7 @@ void kvm_exit(void)
kvm_async_pf_deinit();
unregister_syscore_ops(&kvm_syscore_ops);
unregister_reboot_notifier(&kvm_reboot_notifier);
- cpuhp_remove_state_nocalls(CPUHP_AP_KVM_STARTING);
+ cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE);
on_each_cpu(hardware_disable_nolock, NULL, 1);
kvm_arch_hardware_unsetup();
kvm_arch_exit();
--
2.25.1
On Tue, Aug 30, 2022 at 05:01:20AM -0700, [email protected] wrote:
>From: Chao Gao <[email protected]>
>
>The CPU STARTING section doesn't allow callbacks to fail. Move KVM's
>hotplug callback to ONLINE section so that it can abort onlining a CPU in
>certain cases to avoid potentially breaking VMs running on existing CPUs.
>For example, when kvm fails to enable hardware virtualization on the
>hotplugged CPU.
>
>Place KVM's hotplug state before CPUHP_AP_SCHED_WAIT_EMPTY as it ensures
>when offlining a CPU, all user tasks and non-pinned kernel tasks have left
>the CPU, i.e. there cannot be a vCPU task around. So, it is safe for KVM's
>CPU offline callback to disable hardware virtualization at that point.
>Likewise, KVM's online callback can enable hardware virtualization before
>any vCPU task gets a chance to run on hotplugged CPUs.
>
>KVM's CPU hotplug callbacks are renamed as well.
>
>Suggested-by: Thomas Gleixner <[email protected]>
>Signed-off-by: Chao Gao <[email protected]>
>Link: https://lore.kernel.org/r/[email protected]
Note that Sean gave his Reviewed-by for KVM changes.
https://lore.kernel.org/all/Yg%[email protected]/
On Tue, Aug 30, 2022 at 05:01:20AM -0700, [email protected] wrote:
>From: Chao Gao <[email protected]>
>
>The CPU STARTING section doesn't allow callbacks to fail. Move KVM's
>hotplug callback to ONLINE section so that it can abort onlining a CPU in
>certain cases to avoid potentially breaking VMs running on existing CPUs.
>For example, when kvm fails to enable hardware virtualization on the
>hotplugged CPU.
>
>Place KVM's hotplug state before CPUHP_AP_SCHED_WAIT_EMPTY as it ensures
>when offlining a CPU, all user tasks and non-pinned kernel tasks have left
>the CPU, i.e. there cannot be a vCPU task around. So, it is safe for KVM's
>CPU offline callback to disable hardware virtualization at that point.
>Likewise, KVM's online callback can enable hardware virtualization before
>any vCPU task gets a chance to run on hotplugged CPUs.
>
>KVM's CPU hotplug callbacks are renamed as well.
>
>Suggested-by: Thomas Gleixner <[email protected]>
>Signed-off-by: Chao Gao <[email protected]>
Isaku, your signed-off-by is missing.
>Link: https://lore.kernel.org/r/[email protected]
>---
> include/linux/cpuhotplug.h | 2 +-
> virt/kvm/kvm_main.c | 30 ++++++++++++++++++++++--------
> 2 files changed, 23 insertions(+), 9 deletions(-)
>
>diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
>index f61447913db9..7972bd63e0cb 100644
>--- a/include/linux/cpuhotplug.h
>+++ b/include/linux/cpuhotplug.h
>@@ -185,7 +185,6 @@ enum cpuhp_state {
> CPUHP_AP_CSKY_TIMER_STARTING,
> CPUHP_AP_TI_GP_TIMER_STARTING,
> CPUHP_AP_HYPERV_TIMER_STARTING,
>- CPUHP_AP_KVM_STARTING,
> CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
> CPUHP_AP_KVM_ARM_VGIC_STARTING,
> CPUHP_AP_KVM_ARM_TIMER_STARTING,
The movement of CPUHP_AP_KVM_STARTING changes the ordering between
CPUHP_AP_KVM_STARTING and CPUHP_AP_KVM_ARM_* above [1]. We need
the patch [2] from Marc to avoid breaking ARM.
[1] https://lore.kernel.org/lkml/[email protected]/
[2] https://lore.kernel.org/lkml/[email protected]/
On Thu, 2022-09-01 at 14:18 +0800, Gao, Chao wrote:
> On Tue, Aug 30, 2022 at 05:01:20AM -0700, [email protected] wrote:
> > From: Chao Gao <[email protected]>
> >
> > The CPU STARTING section doesn't allow callbacks to fail. Move KVM's
> > hotplug callback to ONLINE section so that it can abort onlining a CPU in
> > certain cases to avoid potentially breaking VMs running on existing CPUs.
> > For example, when kvm fails to enable hardware virtualization on the
> > hotplugged CPU.
> >
> > Place KVM's hotplug state before CPUHP_AP_SCHED_WAIT_EMPTY as it ensures
> > when offlining a CPU, all user tasks and non-pinned kernel tasks have left
> > the CPU, i.e. there cannot be a vCPU task around. So, it is safe for KVM's
> > CPU offline callback to disable hardware virtualization at that point.
> > Likewise, KVM's online callback can enable hardware virtualization before
> > any vCPU task gets a chance to run on hotplugged CPUs.
> >
> > KVM's CPU hotplug callbacks are renamed as well.
> >
> > Suggested-by: Thomas Gleixner <[email protected]>
> > Signed-off-by: Chao Gao <[email protected]>
>
> Isaku, your signed-off-by is missing.
>
> > Link: https://lore.kernel.org/r/[email protected]
> > ---
> > include/linux/cpuhotplug.h | 2 +-
> > virt/kvm/kvm_main.c | 30 ++++++++++++++++++++++--------
> > 2 files changed, 23 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > index f61447913db9..7972bd63e0cb 100644
> > --- a/include/linux/cpuhotplug.h
> > +++ b/include/linux/cpuhotplug.h
> > @@ -185,7 +185,6 @@ enum cpuhp_state {
> > CPUHP_AP_CSKY_TIMER_STARTING,
> > CPUHP_AP_TI_GP_TIMER_STARTING,
> > CPUHP_AP_HYPERV_TIMER_STARTING,
> > - CPUHP_AP_KVM_STARTING,
> > CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
> > CPUHP_AP_KVM_ARM_VGIC_STARTING,
> > CPUHP_AP_KVM_ARM_TIMER_STARTING,
>
> The movement of CPUHP_AP_KVM_STARTING changes the ordering between
> CPUHP_AP_KVM_STARTING and CPUHP_AP_KVM_ARM_* above [1]. We need
> the patch [2] from Marc to avoid breaking ARM.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
> [2] https://lore.kernel.org/lkml/[email protected]/
How about Isaku just to take your series directly (+his SoB) and add additional
patches?
--
Thanks,
-Kai
On Thu, Sep 01, 2022 at 10:58:04AM +0000,
"Huang, Kai" <[email protected]> wrote:
> On Thu, 2022-09-01 at 14:18 +0800, Gao, Chao wrote:
> > On Tue, Aug 30, 2022 at 05:01:20AM -0700, [email protected] wrote:
> > > From: Chao Gao <[email protected]>
> > >
> > > The CPU STARTING section doesn't allow callbacks to fail. Move KVM's
> > > hotplug callback to ONLINE section so that it can abort onlining a CPU in
> > > certain cases to avoid potentially breaking VMs running on existing CPUs.
> > > For example, when kvm fails to enable hardware virtualization on the
> > > hotplugged CPU.
> > >
> > > Place KVM's hotplug state before CPUHP_AP_SCHED_WAIT_EMPTY as it ensures
> > > when offlining a CPU, all user tasks and non-pinned kernel tasks have left
> > > the CPU, i.e. there cannot be a vCPU task around. So, it is safe for KVM's
> > > CPU offline callback to disable hardware virtualization at that point.
> > > Likewise, KVM's online callback can enable hardware virtualization before
> > > any vCPU task gets a chance to run on hotplugged CPUs.
> > >
> > > KVM's CPU hotplug callbacks are renamed as well.
> > >
> > > Suggested-by: Thomas Gleixner <[email protected]>
> > > Signed-off-by: Chao Gao <[email protected]>
> >
> > Isaku, your signed-off-by is missing.
> >
> > > Link: https://lore.kernel.org/r/[email protected]
> > > ---
> > > include/linux/cpuhotplug.h | 2 +-
> > > virt/kvm/kvm_main.c | 30 ++++++++++++++++++++++--------
> > > 2 files changed, 23 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> > > index f61447913db9..7972bd63e0cb 100644
> > > --- a/include/linux/cpuhotplug.h
> > > +++ b/include/linux/cpuhotplug.h
> > > @@ -185,7 +185,6 @@ enum cpuhp_state {
> > > CPUHP_AP_CSKY_TIMER_STARTING,
> > > CPUHP_AP_TI_GP_TIMER_STARTING,
> > > CPUHP_AP_HYPERV_TIMER_STARTING,
> > > - CPUHP_AP_KVM_STARTING,
> > > CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
> > > CPUHP_AP_KVM_ARM_VGIC_STARTING,
> > > CPUHP_AP_KVM_ARM_TIMER_STARTING,
> >
> > The movement of CPUHP_AP_KVM_STARTING changes the ordering between
> > CPUHP_AP_KVM_STARTING and CPUHP_AP_KVM_ARM_* above [1]. We need
> > the patch [2] from Marc to avoid breaking ARM.
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
> > [2] https://lore.kernel.org/lkml/[email protected]/
>
> How about Isaku just to take your series directly (+his SoB) and add additional
> patches?
Ok will do. Although I hoped to slim it down, I've ended up to take most of it.
four out of six. Now why not two more.
--
Isaku Yamahata <[email protected]>