From: Isaku Yamahata <[email protected]>
This patch series is to implement the suggestion by Sean Christopherson [1]
to reorganize enable/disable cpu virtualization feature by replacing
the arch-generic current enable/disable logic with PM related hooks. And
convert kvm/x86 to use new hooks.
- Untable x86 hardware enable logic, snapshot MSRs for user return notifier,
enabling cpu virtualization on cpu online and platform resume. and real
enabling of CPU virtualization feature
- Introduce hooks related to PM.
- Convert kvm/x86 code to user those hooks.
- Split out hardware enabling/disabling logic into a separate file. Compile
it for non-x86 code. Once conversion of other KVM archs is done, this file
can be dropped.
- Delete cpus_hardware_enabled. 17/18 and 18/18
[1] https://lore.kernel.org/kvm/[email protected]/
Changes from v1:
- Add a patch "KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section"
to make online/offline callback to run thread context to use mutex instead
of spin lock
- fixes pointed by Chao Gao
Chao Gao (3):
KVM: x86: Move check_processor_compatibility from init ops to runtime
ops
Partially revert "KVM: Pass kvm_init()'s opaque param to additional
arch funcs"
KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section
Isaku Yamahata (16):
KVM: x86: Drop kvm_user_return_msr_cpu_online()
KVM: x86: Use this_cpu_ptr() instead of
per_cpu_ptr(smp_processor_id())
KVM: Drop kvm_count_lock and instead protect kvm_usage_count with
kvm_lock
KVM: Add arch hooks for PM events with empty stub
KVM: x86: Move TSC fixup logic to KVM arch resume callback
KVM: Add arch hook when VM is added/deleted
KVM: Move out KVM arch PM hooks and hardware enable/disable logic
KVM: kvm_arch.c: Remove _nolock post fix
KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed
KVM: Do processor compatibility check on cpu online and resume
KVM: x86: Duplicate arch callbacks related to pm events
KVM: Eliminate kvm_arch_post_init_vm()
KVM: x86: Delete kvm_arch_hardware_enable/disable()
KVM: Add config to not compile kvm_arch.c
RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check
RFC: KVM: Remove cpus_hardware_enabled and related sanity check
Documentation/virt/kvm/locking.rst | 14 +--
arch/arm64/kvm/arm.c | 2 +-
arch/mips/kvm/mips.c | 2 +-
arch/powerpc/kvm/powerpc.c | 2 +-
arch/riscv/kvm/main.c | 2 +-
arch/s390/kvm/kvm-s390.c | 2 +-
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/Kconfig | 1 +
arch/x86/kvm/svm/svm.c | 4 +-
arch/x86/kvm/vmx/vmx.c | 14 +--
arch/x86/kvm/x86.c | 192 +++++++++++++++++++++-------
include/linux/cpuhotplug.h | 2 +-
include/linux/kvm_host.h | 14 ++-
virt/kvm/Kconfig | 3 +
virt/kvm/Makefile.kvm | 3 +
virt/kvm/kvm_arch.c | 126 +++++++++++++++++++
virt/kvm/kvm_main.c | 193 +++++++++--------------------
18 files changed, 373 insertions(+), 206 deletions(-)
create mode 100644 virt/kvm/kvm_arch.c
--
2.25.1
From: Isaku Yamahata <[email protected]>
cpus_hardware_enabled mask seems incomplete protection against other kernel
component using CPU virtualization feature. Because it's obscure and
incomplete, remove the check.
Signed-off-by: Isaku Yamahata <[email protected]>
---
virt/kvm/kvm_arch.c | 16 ++--------------
1 file changed, 2 insertions(+), 14 deletions(-)
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 8f2d920a2a8f..cbad0181c177 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -12,22 +12,16 @@
#include <linux/kvm_host.h>
-static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
-
static int __hardware_enable(void)
{
- int cpu = raw_smp_processor_id();
int r;
WARN_ON_ONCE(preemptible());
- if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
- return 0;
r = kvm_arch_hardware_enable();
if (r)
- pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu);
- else
- cpumask_set_cpu(cpu, &cpus_hardware_enabled);
+ pr_info("kvm: enabling virtualization on CPU%d failed\n",
+ smp_processor_id());
return r;
}
@@ -41,13 +35,7 @@ static void hardware_enable(void *arg)
static void hardware_disable(void *junk)
{
- int cpu = raw_smp_processor_id();
-
WARN_ON_ONCE(preemptible());
-
- if (!cpumask_test_cpu(cpu, &cpus_hardware_enabled))
- return;
- cpumask_clear_cpu(cpu, &cpus_hardware_enabled);
kvm_arch_hardware_disable();
}
--
2.25.1
From: Isaku Yamahata <[email protected]>
convert per_cpu_ptr(smp_processor_id()) to this_cpu_ptr() as trivial
cleanup.
Signed-off-by: Isaku Yamahata <[email protected]>
---
arch/x86/kvm/x86.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 16104a2f7d8e..7d5fff68befe 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -416,8 +416,7 @@ EXPORT_SYMBOL_GPL(kvm_find_user_return_msr);
int kvm_set_user_return_msr(unsigned slot, u64 value, u64 mask)
{
- unsigned int cpu = smp_processor_id();
- struct kvm_user_return_msrs *msrs = per_cpu_ptr(user_return_msrs, cpu);
+ struct kvm_user_return_msrs *msrs = this_cpu_ptr(user_return_msrs);
struct kvm_user_return_msr_values *values = &msrs->values[slot];
int err;
@@ -449,8 +448,7 @@ EXPORT_SYMBOL_GPL(kvm_set_user_return_msr);
static void drop_user_return_notifiers(void)
{
- unsigned int cpu = smp_processor_id();
- struct kvm_user_return_msrs *msrs = per_cpu_ptr(user_return_msrs, cpu);
+ struct kvm_user_return_msrs *msrs = this_cpu_ptr(user_return_msrs);
if (msrs->registered)
kvm_on_user_return(&msrs->urn);
--
2.25.1
From: Isaku Yamahata <[email protected]>
So that kvm_arch_hardware_enable/disable() aren't defined.
Once the conversion of all KVM archs is done, this config and kvm_arch.c
should be removed.
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Isaku Yamahata <[email protected]>
---
arch/x86/kvm/Kconfig | 1 +
include/linux/kvm_host.h | 2 ++
virt/kvm/Kconfig | 3 +++
virt/kvm/Makefile.kvm | 5 ++++-
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index e3cbd7706136..e2e16205425d 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -25,6 +25,7 @@ config KVM
depends on X86_LOCAL_APIC
select PREEMPT_NOTIFIERS
select MMU_NOTIFIER
+ select HAVE_KVM_OVERRIDE_HARDWARE_ENABLE
select HAVE_KVM_IRQCHIP
select HAVE_KVM_PFNCACHE
select HAVE_KVM_IRQFD
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8abbf7a1773b..74111118db42 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1434,8 +1434,10 @@ void kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu, struct dentry *debugfs_
static inline void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) {}
#endif
+#ifndef CONFIG_HAVE_KVM_OVERRIDE_HARDWARE_ENABLE
int kvm_arch_hardware_enable(void);
void kvm_arch_hardware_disable(void);
+#endif
int kvm_arch_hardware_setup(void *opaque);
void kvm_arch_pre_hardware_unsetup(void);
void kvm_arch_hardware_unsetup(void);
diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
index a8c5c9f06b3c..917314a87696 100644
--- a/virt/kvm/Kconfig
+++ b/virt/kvm/Kconfig
@@ -72,3 +72,6 @@ config KVM_XFER_TO_GUEST_WORK
config HAVE_KVM_PM_NOTIFIER
bool
+
+config HAVE_KVM_OVERRIDE_HARDWARE_ENABLE
+ def_bool n
diff --git a/virt/kvm/Makefile.kvm b/virt/kvm/Makefile.kvm
index c4210acabd35..c0187ec4f83c 100644
--- a/virt/kvm/Makefile.kvm
+++ b/virt/kvm/Makefile.kvm
@@ -5,7 +5,10 @@
KVM ?= ../../../virt/kvm
-kvm-y := $(KVM)/kvm_main.o $(KVM)/kvm_arch.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
+kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
+ifneq ($(CONFIG_HAVE_KVM_OVERRIDE_HARDWARE_ENABLE), y)
+kvm-y += $(KVM)/kvm_arch.o
+endif
kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o
kvm-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o
kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
--
2.25.1
From: Isaku Yamahata <[email protected]>
Because kvm_count_lock unnecessarily complicates the KVM locking convention
Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock for
simplicity.
Opportunistically add some comments on locking.
Suggested-by: Sean Christopherson <[email protected]>
Signed-off-by: Isaku Yamahata <[email protected]>
---
Documentation/virt/kvm/locking.rst | 14 +++++-------
virt/kvm/kvm_main.c | 36 +++++++++++++++++++++---------
2 files changed, 30 insertions(+), 20 deletions(-)
diff --git a/Documentation/virt/kvm/locking.rst b/Documentation/virt/kvm/locking.rst
index 845a561629f1..8957e32aa724 100644
--- a/Documentation/virt/kvm/locking.rst
+++ b/Documentation/virt/kvm/locking.rst
@@ -216,15 +216,11 @@ time it will be set using the Dirty tracking mechanism described above.
:Type: mutex
:Arch: any
:Protects: - vm_list
-
-``kvm_count_lock``
-^^^^^^^^^^^^^^^^^^
-
-:Type: raw_spinlock_t
-:Arch: any
-:Protects: - hardware virtualization enable/disable
-:Comment: 'raw' because hardware enabling/disabling must be atomic /wrt
- migration.
+ - kvm_usage_count
+ - hardware virtualization enable/disable
+:Comment: Use cpus_read_lock() for hardware virtualization enable/disable
+ because hardware enabling/disabling must be atomic /wrt
+ migration. The lock order is cpus lock => kvm_lock.
``kvm->mn_invalidate_lock``
^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6ce6f27f2934..606ac6bb67d0 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -100,7 +100,6 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink);
*/
DEFINE_MUTEX(kvm_lock);
-static DEFINE_RAW_SPINLOCK(kvm_count_lock);
LIST_HEAD(vm_list);
static cpumask_var_t cpus_hardware_enabled;
@@ -4996,6 +4995,8 @@ static void hardware_enable_nolock(void *junk)
int cpu = raw_smp_processor_id();
int r;
+ WARN_ON_ONCE(preemptible());
+
if (cpumask_test_cpu(cpu, cpus_hardware_enabled))
return;
@@ -5014,7 +5015,7 @@ static int kvm_online_cpu(unsigned int cpu)
{
int ret = 0;
- raw_spin_lock(&kvm_count_lock);
+ mutex_lock(&kvm_lock);
/*
* Abort the CPU online process if hardware virtualization cannot
* be enabled. Otherwise running VMs would encounter unrecoverable
@@ -5029,7 +5030,7 @@ static int kvm_online_cpu(unsigned int cpu)
ret = -EIO;
}
}
- raw_spin_unlock(&kvm_count_lock);
+ mutex_unlock(&kvm_lock);
return ret;
}
@@ -5037,6 +5038,8 @@ static void hardware_disable_nolock(void *junk)
{
int cpu = raw_smp_processor_id();
+ WARN_ON_ONCE(preemptible());
+
if (!cpumask_test_cpu(cpu, cpus_hardware_enabled))
return;
cpumask_clear_cpu(cpu, cpus_hardware_enabled);
@@ -5045,10 +5048,10 @@ static void hardware_disable_nolock(void *junk)
static int kvm_offline_cpu(unsigned int cpu)
{
- raw_spin_lock(&kvm_count_lock);
+ mutex_lock(&kvm_lock);
if (kvm_usage_count)
hardware_disable_nolock(NULL);
- raw_spin_unlock(&kvm_count_lock);
+ mutex_unlock(&kvm_lock);
return 0;
}
@@ -5063,16 +5066,19 @@ static void hardware_disable_all_nolock(void)
static void hardware_disable_all(void)
{
- raw_spin_lock(&kvm_count_lock);
+ cpus_read_lock();
+ mutex_lock(&kvm_lock);
hardware_disable_all_nolock();
- raw_spin_unlock(&kvm_count_lock);
+ mutex_unlock(&kvm_lock);
+ cpus_read_unlock();
}
static int hardware_enable_all(void)
{
int r = 0;
- raw_spin_lock(&kvm_count_lock);
+ cpus_read_lock();
+ mutex_lock(&kvm_lock);
kvm_usage_count++;
if (kvm_usage_count == 1) {
@@ -5085,7 +5091,8 @@ static int hardware_enable_all(void)
}
}
- raw_spin_unlock(&kvm_count_lock);
+ mutex_unlock(&kvm_lock);
+ cpus_read_unlock();
return r;
}
@@ -5691,15 +5698,22 @@ static void kvm_init_debug(void)
static int kvm_suspend(void)
{
- if (kvm_usage_count)
+ /*
+ * The caller ensures that CPU hotlug is disabled by
+ * cpu_hotplug_disable() and other CPUs are offlined. No need for
+ * locking.
+ */
+ if (kvm_usage_count) {
+ lockdep_assert_not_held(&kvm_lock);
hardware_disable_nolock(NULL);
+ }
return 0;
}
static void kvm_resume(void)
{
if (kvm_usage_count) {
- lockdep_assert_not_held(&kvm_count_lock);
+ lockdep_assert_not_held(&kvm_lock);
hardware_enable_nolock(NULL);
}
}
--
2.25.1
On Tue, Aug 30, 2022 at 05:01:17AM -0700, [email protected] wrote:
>From: Isaku Yamahata <[email protected]>
>
>convert per_cpu_ptr(smp_processor_id()) to this_cpu_ptr() as trivial
>cleanup.
>
>Signed-off-by: Isaku Yamahata <[email protected]>
Reviewed-by: Chao Gao <[email protected]>
>---
> arch/x86/kvm/x86.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 16104a2f7d8e..7d5fff68befe 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -416,8 +416,7 @@ EXPORT_SYMBOL_GPL(kvm_find_user_return_msr);
>
> int kvm_set_user_return_msr(unsigned slot, u64 value, u64 mask)
> {
>- unsigned int cpu = smp_processor_id();
>- struct kvm_user_return_msrs *msrs = per_cpu_ptr(user_return_msrs, cpu);
>+ struct kvm_user_return_msrs *msrs = this_cpu_ptr(user_return_msrs);
> struct kvm_user_return_msr_values *values = &msrs->values[slot];
> int err;
>
>@@ -449,8 +448,7 @@ EXPORT_SYMBOL_GPL(kvm_set_user_return_msr);
>
> static void drop_user_return_notifiers(void)
> {
>- unsigned int cpu = smp_processor_id();
>- struct kvm_user_return_msrs *msrs = per_cpu_ptr(user_return_msrs, cpu);
>+ struct kvm_user_return_msrs *msrs = this_cpu_ptr(user_return_msrs);
>
> if (msrs->registered)
> kvm_on_user_return(&msrs->urn);
>--
>2.25.1
>