* [PATCH v4 00/26] KVM: hardware enable/disable reorganize
@ 2022-09-08 23:25 isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 01/26] KVM: x86: Drop kvm_user_return_msr_cpu_online() isaku.yamahata
` (25 more replies)
0 siblings, 26 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
I've updated the patch series based on Marc's request and Yuan review and
now eliminated kvm_arch_check_processor_compat().
Those two patch are compile only tested.
- "KVM: arm64: Simplify the CPUHP logic"
- "RFC: KVM: powerpc: Move processor compatibility check to hardware setup"
- "RFC: KVM: Remove cpus_hardware_enabled and related sanity check"
Changes from v3:
- Updated "KVM: arm64: Simplify the CPUHP logic".
- add preempt_disable/enable() around hardware_enable/disable() to keep the
assumption of the arch callback.
- Eliminated arch compat check callback, kvm_arch_check_processor_compat().
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 kvm_arch_check_processor_compat()
- Delete cpus_hardware_enabled. 17/18 and 18/18
[1] https://lore.kernel.org/kvm/YvU+6fdkHaqQiKxp@google.com/
[2] https://lore.kernel.org/all/20220216031528.92558-1-chao.gao@intel.com/
Changes from v2:
- Replace the first patch("KVM: x86: Drop kvm_user_return_msr_cpu_online()")
with Sean's implementation
- Included all patches of "Improve KVM's interaction with CPU hotplug" [2]
Until v2, Tried to cherry-pick the least patches of it. It turned out that
all the patches are desirable.
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 (4):
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
KVM: Do compatibility checks on hotplugged CPUs
Isaku Yamahata (19):
KVM: x86: Use this_cpu_ptr() instead of
per_cpu_ptr(smp_processor_id())
KVM: Do processor compatibility check on resume
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: Introduce a arch wrapper to check all processor compatibility
KVM: x86: Duplicate arch callbacks related to pm events and compat
check
KVM: Eliminate kvm_arch_post_init_vm()
KVM: Add config to not compile kvm_arch.c
KVM: x86: Delete kvm_arch_hardware_enable/disable()
KVM: x86: Make x86 processor compat check callback empty
RFC: KVM: powerpc: Move processor compatibility check to hardware
setup
KVM: Eliminate kvm_arch_check_processor_compat()
RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check
RFC: KVM: Remove cpus_hardware_enabled and related sanity check
Marc Zyngier (1):
KVM: arm64: Simplify the CPUHP logic
Sean Christopherson (2):
KVM: x86: Drop kvm_user_return_msr_cpu_online()
KVM: Provide more information in kernel log if hardware enabling fails
Documentation/virt/kvm/locking.rst | 14 +-
arch/arm64/kvm/arch_timer.c | 27 ++--
arch/arm64/kvm/arm.c | 18 ++-
arch/arm64/kvm/vgic/vgic-init.c | 19 +--
arch/mips/kvm/mips.c | 5 -
arch/powerpc/kvm/powerpc.c | 14 +-
arch/riscv/kvm/main.c | 5 -
arch/s390/kvm/kvm-s390.c | 5 -
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 | 24 ++--
arch/x86/kvm/x86.c | 221 ++++++++++++++++++++++++-----
include/kvm/arm_arch_timer.h | 4 +
include/kvm/arm_vgic.h | 4 +
include/linux/cpuhotplug.h | 5 +-
include/linux/kvm_host.h | 16 ++-
virt/kvm/Kconfig | 3 +
virt/kvm/Makefile.kvm | 3 +
virt/kvm/kvm_arch.c | 140 ++++++++++++++++++
virt/kvm/kvm_main.c | 208 +++++++++------------------
22 files changed, 481 insertions(+), 262 deletions(-)
create mode 100644 virt/kvm/kvm_arch.c
base-commit: b90cb1053190353cc30f0fef0ef1f378ccc063c5
--
2.25.1
^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH v4 01/26] KVM: x86: Drop kvm_user_return_msr_cpu_online()
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 02/26] KVM: x86: Use this_cpu_ptr() instead of per_cpu_ptr(smp_processor_id()) isaku.yamahata
` (24 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Yuan Yao
From: Sean Christopherson <seanjc@google.com>
KVM/X86 uses user return notifier to switch MSR for guest or user space.
Snapshot host values on CPU online, change MSR values for guest, and
restore them on returning to user space. The current code abuses
kvm_arch_hardware_enable() which is called on kvm module initialization or
CPU online.
Remove such the abuse of kvm_arch_hardware_enable() by capturing the host
value on the first change of the MSR value to guest VM instead of CPU
online.
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
arch/x86/kvm/x86.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 205ebdc2b11b..73dccc952dd1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -196,6 +196,7 @@ module_param(eager_page_split, bool, 0644);
struct kvm_user_return_msrs {
struct user_return_notifier urn;
+ bool initialized;
bool registered;
struct kvm_user_return_msr_values {
u64 host;
@@ -409,18 +410,20 @@ int kvm_find_user_return_msr(u32 msr)
}
EXPORT_SYMBOL_GPL(kvm_find_user_return_msr);
-static void kvm_user_return_msr_cpu_online(void)
+static void kvm_user_return_msr_init_cpu(struct kvm_user_return_msrs *msrs)
{
- unsigned int cpu = smp_processor_id();
- struct kvm_user_return_msrs *msrs = per_cpu_ptr(user_return_msrs, cpu);
u64 value;
int i;
+ if (msrs->initialized)
+ return;
+
for (i = 0; i < kvm_nr_uret_msrs; ++i) {
rdmsrl_safe(kvm_uret_msrs_list[i], &value);
msrs->values[i].host = value;
msrs->values[i].curr = value;
}
+ msrs->initialized = true;
}
int kvm_set_user_return_msr(unsigned slot, u64 value, u64 mask)
@@ -429,6 +432,8 @@ int kvm_set_user_return_msr(unsigned slot, u64 value, u64 mask)
struct kvm_user_return_msrs *msrs = per_cpu_ptr(user_return_msrs, cpu);
int err;
+ kvm_user_return_msr_init_cpu(msrs);
+
value = (value & mask) | (msrs->values[slot].host & ~mask);
if (value == msrs->values[slot].curr)
return 0;
@@ -9212,7 +9217,12 @@ int kvm_arch_init(void *opaque)
return -ENOMEM;
}
- user_return_msrs = alloc_percpu(struct kvm_user_return_msrs);
+ /*
+ * __GFP_ZERO to ensure user_return_msrs.initialized = false.
+ * See kvm_user_return_msr_init_cpu().
+ */
+ user_return_msrs = alloc_percpu_gfp(struct kvm_user_return_msrs,
+ GFP_KERNEL | __GFP_ZERO);
if (!user_return_msrs) {
printk(KERN_ERR "kvm: failed to allocate percpu kvm_user_return_msrs\n");
r = -ENOMEM;
@@ -11836,7 +11846,6 @@ int kvm_arch_hardware_enable(void)
u64 max_tsc = 0;
bool stable, backwards_tsc = false;
- kvm_user_return_msr_cpu_online();
ret = static_call(kvm_x86_hardware_enable)();
if (ret != 0)
return ret;
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 02/26] KVM: x86: Use this_cpu_ptr() instead of per_cpu_ptr(smp_processor_id())
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 01/26] KVM: x86: Drop kvm_user_return_msr_cpu_online() isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 03/26] KVM: x86: Move check_processor_compatibility from init ops to runtime ops isaku.yamahata
` (23 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Yuan Yao
From: Isaku Yamahata <isaku.yamahata@intel.com>
convert per_cpu_ptr(smp_processor_id()) to this_cpu_ptr() as trivial
cleanup.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
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 73dccc952dd1..0368eab6a7b5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -428,8 +428,7 @@ static void kvm_user_return_msr_init_cpu(struct kvm_user_return_msrs *msrs)
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);
int err;
kvm_user_return_msr_init_cpu(msrs);
@@ -453,8 +452,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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 03/26] KVM: x86: Move check_processor_compatibility from init ops to runtime ops
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 01/26] KVM: x86: Drop kvm_user_return_msr_cpu_online() isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 02/26] KVM: x86: Use this_cpu_ptr() instead of per_cpu_ptr(smp_processor_id()) isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 04/26] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs" isaku.yamahata
` (22 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Yuan Yao
From: Chao Gao <chao.gao@intel.com>
so that KVM can do compatibility checks on hotplugged CPUs. Drop __init
from check_processor_compatibility() and its callees.
use a static_call() to invoke .check_processor_compatibility.
Opportunistically rename {svm,vmx}_check_processor_compat to conform
to the naming convention of fields of kvm_x86_ops.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/r/20220216031528.92558-2-chao.gao@intel.com
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
arch/x86/include/asm/kvm-x86-ops.h | 1 +
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/svm/svm.c | 4 ++--
arch/x86/kvm/vmx/vmx.c | 14 +++++++-------
arch/x86/kvm/x86.c | 3 +--
5 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
index 51f777071584..3bc45932e2d1 100644
--- a/arch/x86/include/asm/kvm-x86-ops.h
+++ b/arch/x86/include/asm/kvm-x86-ops.h
@@ -129,6 +129,7 @@ KVM_X86_OP(msr_filter_changed)
KVM_X86_OP(complete_emulated_msr)
KVM_X86_OP(vcpu_deliver_sipi_vector)
KVM_X86_OP_OPTIONAL_RET0(vcpu_get_apicv_inhibit_reasons);
+KVM_X86_OP(check_processor_compatibility)
#undef KVM_X86_OP
#undef KVM_X86_OP_OPTIONAL
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2c96c43c313a..5df5d88d345f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1445,6 +1445,7 @@ static inline u16 kvm_lapic_irq_dest_mode(bool dest_mode_logical)
struct kvm_x86_ops {
const char *name;
+ int (*check_processor_compatibility)(void);
int (*hardware_enable)(void);
void (*hardware_disable)(void);
void (*hardware_unsetup)(void);
@@ -1655,7 +1656,6 @@ struct kvm_x86_nested_ops {
struct kvm_x86_init_ops {
int (*cpu_has_kvm_support)(void);
int (*disabled_by_bios)(void);
- int (*check_processor_compatibility)(void);
int (*hardware_setup)(void);
unsigned int (*handle_intel_pt_intr)(void);
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f3813dbacb9f..371300f03f55 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4134,7 +4134,7 @@ svm_patch_hypercall(struct kvm_vcpu *vcpu, unsigned char *hypercall)
hypercall[2] = 0xd9;
}
-static int __init svm_check_processor_compat(void)
+static int svm_check_processor_compatibility(void)
{
return 0;
}
@@ -4740,6 +4740,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.name = "kvm_amd",
.hardware_unsetup = svm_hardware_unsetup,
+ .check_processor_compatibility = svm_check_processor_compatibility,
.hardware_enable = svm_hardware_enable,
.hardware_disable = svm_hardware_disable,
.has_emulated_msr = svm_has_emulated_msr,
@@ -5122,7 +5123,6 @@ static struct kvm_x86_init_ops svm_init_ops __initdata = {
.cpu_has_kvm_support = has_svm,
.disabled_by_bios = is_disabled,
.hardware_setup = svm_hardware_setup,
- .check_processor_compatibility = svm_check_processor_compat,
.runtime_ops = &svm_x86_ops,
.pmu_ops = &amd_pmu_ops,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index d7f8331d6f7e..3cf7f18a4115 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2495,8 +2495,8 @@ static bool cpu_has_sgx(void)
return cpuid_eax(0) >= 0x12 && (cpuid_eax(0x12) & BIT(0));
}
-static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
- u32 msr, u32 *result)
+static int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
+ u32 msr, u32 *result)
{
u32 vmx_msr_low, vmx_msr_high;
u32 ctl = ctl_min | ctl_opt;
@@ -2514,7 +2514,7 @@ static __init int adjust_vmx_controls(u32 ctl_min, u32 ctl_opt,
return 0;
}
-static __init u64 adjust_vmx_controls64(u64 ctl_opt, u32 msr)
+static u64 adjust_vmx_controls64(u64 ctl_opt, u32 msr)
{
u64 allowed;
@@ -2523,8 +2523,8 @@ static __init u64 adjust_vmx_controls64(u64 ctl_opt, u32 msr)
return ctl_opt & allowed;
}
-static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
- struct vmx_capability *vmx_cap)
+static int setup_vmcs_config(struct vmcs_config *vmcs_conf,
+ struct vmx_capability *vmx_cap)
{
u32 vmx_msr_low, vmx_msr_high;
u32 min, opt, min2, opt2;
@@ -7417,7 +7417,7 @@ static int vmx_vm_init(struct kvm *kvm)
return 0;
}
-static int __init vmx_check_processor_compat(void)
+static int vmx_check_processor_compatibility(void)
{
struct vmcs_config vmcs_conf;
struct vmx_capability vmx_cap;
@@ -8015,6 +8015,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
.hardware_unsetup = vmx_hardware_unsetup,
+ .check_processor_compatibility = vmx_check_processor_compatibility,
.hardware_enable = vmx_hardware_enable,
.hardware_disable = vmx_hardware_disable,
.has_emulated_msr = vmx_has_emulated_msr,
@@ -8404,7 +8405,6 @@ static __init int hardware_setup(void)
static struct kvm_x86_init_ops vmx_init_ops __initdata = {
.cpu_has_kvm_support = cpu_has_kvm_support,
.disabled_by_bios = vmx_disabled_by_bios,
- .check_processor_compatibility = vmx_check_processor_compat,
.hardware_setup = hardware_setup,
.handle_intel_pt_intr = NULL,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0368eab6a7b5..a7fa55ba344d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12001,7 +12001,6 @@ void kvm_arch_hardware_unsetup(void)
int kvm_arch_check_processor_compat(void *opaque)
{
struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
- struct kvm_x86_init_ops *ops = opaque;
WARN_ON(!irqs_disabled());
@@ -12009,7 +12008,7 @@ int kvm_arch_check_processor_compat(void *opaque)
__cr4_reserved_bits(cpu_has, &boot_cpu_data))
return -EIO;
- return ops->check_processor_compatibility();
+ return static_call(kvm_x86_check_processor_compatibility)();
}
bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu)
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 04/26] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs"
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (2 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 03/26] KVM: x86: Move check_processor_compatibility from init ops to runtime ops isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 05/26] KVM: Provide more information in kernel log if hardware enabling fails isaku.yamahata
` (21 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Suzuki K Poulose, Anup Patel, Claudio Imbrenda,
Yuan Yao
From: Chao Gao <chao.gao@intel.com>
This partially reverts commit b99040853738 ("KVM: Pass kvm_init()'s opaque
param to additional arch funcs") remove opaque from
kvm_arch_check_processor_compat because no one uses this opaque now.
Address conflicts for ARM (due to file movement) and manually handle RISC-V
which comes after the commit.
And changes about kvm_arch_hardware_setup() in original commit are still
needed so they are not reverted.
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Acked-by: Anup Patel <anup@brainfault.org>
Acked-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Link: https://lore.kernel.org/r/20220216031528.92558-3-chao.gao@intel.com
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
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/kvm/x86.c | 2 +-
include/linux/kvm_host.h | 2 +-
virt/kvm/kvm_main.c | 16 +++-------------
8 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 2ff0ef62abad..3385fb57c11a 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -68,7 +68,7 @@ int kvm_arch_hardware_setup(void *opaque)
return 0;
}
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
{
return 0;
}
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index a25e0b73ee70..092d09fb6a7e 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -140,7 +140,7 @@ int kvm_arch_hardware_setup(void *opaque)
return 0;
}
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
{
return 0;
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index fb1490761c87..7b56d6ccfdfb 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -447,7 +447,7 @@ int kvm_arch_hardware_setup(void *opaque)
return 0;
}
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
{
return kvmppc_core_check_processor_compat();
}
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index 1549205fe5fe..f8d6372d208f 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -20,7 +20,7 @@ long kvm_arch_dev_ioctl(struct file *filp,
return -EINVAL;
}
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
{
return 0;
}
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index edfd4bbd0cba..e26d4dd85668 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -254,7 +254,7 @@ int kvm_arch_hardware_enable(void)
return 0;
}
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
{
return 0;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a7fa55ba344d..f6f014e78ab9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11998,7 +11998,7 @@ void kvm_arch_hardware_unsetup(void)
static_call(kvm_x86_hardware_unsetup)();
}
-int kvm_arch_check_processor_compat(void *opaque)
+int kvm_arch_check_processor_compat(void)
{
struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f4519d3689e1..eab352902de7 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1438,7 +1438,7 @@ int kvm_arch_hardware_enable(void);
void kvm_arch_hardware_disable(void);
int kvm_arch_hardware_setup(void *opaque);
void kvm_arch_hardware_unsetup(void);
-int kvm_arch_check_processor_compat(void *opaque);
+int kvm_arch_check_processor_compat(void);
int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 584a5bab3af3..4243a9541543 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5799,22 +5799,14 @@ void kvm_unregister_perf_callbacks(void)
}
#endif
-struct kvm_cpu_compat_check {
- void *opaque;
- int *ret;
-};
-
-static void check_processor_compat(void *data)
+static void check_processor_compat(void *rtn)
{
- struct kvm_cpu_compat_check *c = data;
-
- *c->ret = kvm_arch_check_processor_compat(c->opaque);
+ *(int *)rtn = kvm_arch_check_processor_compat();
}
int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
struct module *module)
{
- struct kvm_cpu_compat_check c;
int r;
int cpu;
@@ -5842,10 +5834,8 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
if (r < 0)
goto out_free_1;
- c.ret = &r;
- c.opaque = opaque;
for_each_online_cpu(cpu) {
- smp_call_function_single(cpu, check_processor_compat, &c, 1);
+ smp_call_function_single(cpu, check_processor_compat, &r, 1);
if (r < 0)
goto out_free_2;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 05/26] KVM: Provide more information in kernel log if hardware enabling fails
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (3 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 04/26] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs" isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 06/26] KVM: arm64: Simplify the CPUHP logic isaku.yamahata
` (20 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Yuan Yao
From: Sean Christopherson <seanjc@google.com>
Provide the name of the calling function to hardware_enable_nolock() and
include it in the error message to provide additional information on
exactly what path failed.
Opportunistically bump the pr_info() to pr_warn(), failure to enable
virtualization support is warn-worthy as _something_ is wrong with the
system.
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/r/20220216031528.92558-4-chao.gao@intel.com
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
virt/kvm/kvm_main.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4243a9541543..278eb6cc7cbe 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4991,7 +4991,7 @@ static struct miscdevice kvm_dev = {
&kvm_chardev_ops,
};
-static void hardware_enable_nolock(void *junk)
+static void hardware_enable_nolock(void *caller_name)
{
int cpu = raw_smp_processor_id();
int r;
@@ -5006,7 +5006,8 @@ static void hardware_enable_nolock(void *junk)
if (r) {
cpumask_clear_cpu(cpu, cpus_hardware_enabled);
atomic_inc(&hardware_enable_failed);
- pr_info("kvm: enabling virtualization on CPU%d failed\n", cpu);
+ pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
+ cpu, (const char *)caller_name);
}
}
@@ -5014,7 +5015,7 @@ static int kvm_starting_cpu(unsigned int cpu)
{
raw_spin_lock(&kvm_count_lock);
if (kvm_usage_count)
- hardware_enable_nolock(NULL);
+ hardware_enable_nolock((void *)__func__);
raw_spin_unlock(&kvm_count_lock);
return 0;
}
@@ -5063,7 +5064,7 @@ static int hardware_enable_all(void)
kvm_usage_count++;
if (kvm_usage_count == 1) {
atomic_set(&hardware_enable_failed, 0);
- on_each_cpu(hardware_enable_nolock, NULL, 1);
+ on_each_cpu(hardware_enable_nolock, (void *)__func__, 1);
if (atomic_read(&hardware_enable_failed)) {
hardware_disable_all_nolock();
@@ -5686,7 +5687,7 @@ static void kvm_resume(void)
{
if (kvm_usage_count) {
lockdep_assert_not_held(&kvm_count_lock);
- hardware_enable_nolock(NULL);
+ hardware_enable_nolock((void *)__func__);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 06/26] KVM: arm64: Simplify the CPUHP logic
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (4 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 05/26] KVM: Provide more information in kernel log if hardware enabling fails isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-12 15:07 ` Marc Zyngier
2022-09-08 23:25 ` [PATCH v4 07/26] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section isaku.yamahata
` (19 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Marc Zyngier <maz@kernel.org>
For a number of historical reasons, the KVM/arm64 hotplug setup is pretty
complicated, and we have two extra CPUHP notifiers for vGIC and timers.
It looks pretty pointless, and gets in the way of further changes.
So let's just expose some helpers that can be called from the core
CPUHP callback, and get rid of everything else.
This gives us the opportunity to drop a useless notifier entry,
as well as tidy-up the timer enable/disable, which was a bit odd.
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/r/20220216031528.92558-5-chao.gao@intel.com
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/arm64/kvm/arch_timer.c | 27 ++++++++++-----------------
arch/arm64/kvm/arm.c | 13 +++++++++++++
arch/arm64/kvm/vgic/vgic-init.c | 19 ++-----------------
include/kvm/arm_arch_timer.h | 4 ++++
include/kvm/arm_vgic.h | 4 ++++
include/linux/cpuhotplug.h | 3 ---
6 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/arch/arm64/kvm/arch_timer.c b/arch/arm64/kvm/arch_timer.c
index bb24a76b4224..33fca1a691a5 100644
--- a/arch/arm64/kvm/arch_timer.c
+++ b/arch/arm64/kvm/arch_timer.c
@@ -811,10 +811,18 @@ void kvm_timer_vcpu_init(struct kvm_vcpu *vcpu)
ptimer->host_timer_irq_flags = host_ptimer_irq_flags;
}
-static void kvm_timer_init_interrupt(void *info)
+void kvm_timer_cpu_up(void)
{
enable_percpu_irq(host_vtimer_irq, host_vtimer_irq_flags);
- enable_percpu_irq(host_ptimer_irq, host_ptimer_irq_flags);
+ if (host_ptimer_irq)
+ enable_percpu_irq(host_ptimer_irq, host_ptimer_irq_flags);
+}
+
+void kvm_timer_cpu_down(void)
+{
+ disable_percpu_irq(host_vtimer_irq);
+ if (host_ptimer_irq)
+ disable_percpu_irq(host_ptimer_irq);
}
int kvm_arm_timer_set_reg(struct kvm_vcpu *vcpu, u64 regid, u64 value)
@@ -976,18 +984,6 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
preempt_enable();
}
-static int kvm_timer_starting_cpu(unsigned int cpu)
-{
- kvm_timer_init_interrupt(NULL);
- return 0;
-}
-
-static int kvm_timer_dying_cpu(unsigned int cpu)
-{
- disable_percpu_irq(host_vtimer_irq);
- return 0;
-}
-
static int timer_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
{
if (vcpu)
@@ -1185,9 +1181,6 @@ int kvm_timer_hyp_init(bool has_gic)
goto out_free_irq;
}
- cpuhp_setup_state(CPUHP_AP_KVM_ARM_TIMER_STARTING,
- "kvm/arm/timer:starting", kvm_timer_starting_cpu,
- kvm_timer_dying_cpu);
return 0;
out_free_irq:
free_percpu_irq(host_vtimer_irq, kvm_get_running_vcpus());
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 3385fb57c11a..7e83498b83aa 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1676,7 +1676,15 @@ static void _kvm_arch_hardware_enable(void *discard)
int kvm_arch_hardware_enable(void)
{
+ int was_enabled = __this_cpu_read(kvm_arm_hardware_enabled);
+
_kvm_arch_hardware_enable(NULL);
+
+ if (!was_enabled) {
+ kvm_vgic_cpu_up();
+ kvm_timer_cpu_up();
+ }
+
return 0;
}
@@ -1690,6 +1698,11 @@ static void _kvm_arch_hardware_disable(void *discard)
void kvm_arch_hardware_disable(void)
{
+ if (__this_cpu_read(kvm_arm_hardware_enabled)) {
+ kvm_timer_cpu_down();
+ kvm_vgic_cpu_down();
+ }
+
if (!is_protected_kvm_enabled())
_kvm_arch_hardware_disable(NULL);
}
diff --git a/arch/arm64/kvm/vgic/vgic-init.c b/arch/arm64/kvm/vgic/vgic-init.c
index f6d4f4052555..6c7f6ae21ec0 100644
--- a/arch/arm64/kvm/vgic/vgic-init.c
+++ b/arch/arm64/kvm/vgic/vgic-init.c
@@ -465,17 +465,15 @@ int kvm_vgic_map_resources(struct kvm *kvm)
/* GENERIC PROBE */
-static int vgic_init_cpu_starting(unsigned int cpu)
+void kvm_vgic_cpu_up(void)
{
enable_percpu_irq(kvm_vgic_global_state.maint_irq, 0);
- return 0;
}
-static int vgic_init_cpu_dying(unsigned int cpu)
+void kvm_vgic_cpu_down(void)
{
disable_percpu_irq(kvm_vgic_global_state.maint_irq);
- return 0;
}
static irqreturn_t vgic_maintenance_handler(int irq, void *data)
@@ -584,19 +582,6 @@ int kvm_vgic_hyp_init(void)
return ret;
}
- ret = cpuhp_setup_state(CPUHP_AP_KVM_ARM_VGIC_INIT_STARTING,
- "kvm/arm/vgic:starting",
- vgic_init_cpu_starting, vgic_init_cpu_dying);
- if (ret) {
- kvm_err("Cannot register vgic CPU notifier\n");
- goto out_free_irq;
- }
-
kvm_info("vgic interrupt IRQ%d\n", kvm_vgic_global_state.maint_irq);
return 0;
-
-out_free_irq:
- free_percpu_irq(kvm_vgic_global_state.maint_irq,
- kvm_get_running_vcpus());
- return ret;
}
diff --git a/include/kvm/arm_arch_timer.h b/include/kvm/arm_arch_timer.h
index cd6d8f260eab..1638418f72dd 100644
--- a/include/kvm/arm_arch_timer.h
+++ b/include/kvm/arm_arch_timer.h
@@ -104,4 +104,8 @@ void kvm_arm_timer_write_sysreg(struct kvm_vcpu *vcpu,
u32 timer_get_ctl(struct arch_timer_context *ctxt);
u64 timer_get_cval(struct arch_timer_context *ctxt);
+/* CPU HP callbacks */
+void kvm_timer_cpu_up(void);
+void kvm_timer_cpu_down(void);
+
#endif
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 4df9e73a8bb5..fc4acc91ba06 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -431,4 +431,8 @@ int vgic_v4_load(struct kvm_vcpu *vcpu);
void vgic_v4_commit(struct kvm_vcpu *vcpu);
int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db);
+/* CPU HP callbacks */
+void kvm_vgic_cpu_up(void);
+void kvm_vgic_cpu_down(void);
+
#endif /* __KVM_ARM_VGIC_H */
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index f61447913db9..7337414e4947 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -186,9 +186,6 @@ enum cpuhp_state {
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,
/* Must be the last timer callback */
CPUHP_AP_DUMMY_TIMER_STARTING,
CPUHP_AP_ARM_XEN_STARTING,
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 07/26] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (5 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 06/26] KVM: arm64: Simplify the CPUHP logic isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 08/26] KVM: Do compatibility checks on hotplugged CPUs isaku.yamahata
` (18 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Thomas Gleixner, Yuan Yao
From: Chao Gao <chao.gao@intel.com>
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 <tglx@linutronix.de>
Signed-off-by: Chao Gao <chao.gao@intel.com>
Link: https://lore.kernel.org/r/20220216031528.92558-6-chao.gao@intel.com
Reviewed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
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 7337414e4947..de45be38dd27 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,
/* Must be the last timer callback */
CPUHP_AP_DUMMY_TIMER_STARTING,
CPUHP_AP_ARM_XEN_STARTING,
@@ -200,6 +199,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 278eb6cc7cbe..db1303e2abc9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5011,13 +5011,27 @@ static void hardware_enable_nolock(void *caller_name)
}
}
-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((void *)__func__);
+ 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)
@@ -5030,7 +5044,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)
@@ -5841,8 +5855,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);
@@ -5903,7 +5917,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:
@@ -5929,7 +5943,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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 08/26] KVM: Do compatibility checks on hotplugged CPUs
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (6 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 07/26] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 09/26] KVM: Do processor compatibility check on resume isaku.yamahata
` (17 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, Yuan Yao
From: Chao Gao <chao.gao@intel.com>
At init time, KVM does compatibility checks to ensure that all online
CPUs support hardware virtualization and a common set of features. But
KVM uses hotplugged CPUs without such compatibility checks. On Intel
CPUs, this leads to #GP if the hotplugged CPU doesn't support VMX or
vmentry failure if the hotplugged CPU doesn't meet minimal feature
requirements.
Do compatibility checks when onlining a CPU and abort the online process
if the hotplugged CPU is incompatible with online CPUs.
CPU hotplug is disabled during hardware_enable_all() to prevent the corner
case as shown below. A hotplugged CPU marks itself online in
cpu_online_mask (1) and enables interrupt (2) before invoking callbacks
registered in ONLINE section (3). So, if hardware_enable_all() is invoked
on another CPU right after (2), then on_each_cpu() in hardware_enable_all()
invokes hardware_enable_nolock() on the hotplugged CPU before
kvm_online_cpu() is called. This makes the CPU escape from compatibility
checks, which is risky.
start_secondary { ...
set_cpu_online(smp_processor_id(), true); <- 1
...
local_irq_enable(); <- 2
...
cpu_startup_entry(CPUHP_AP_ONLINE_IDLE); <- 3
}
Keep compatibility checks at KVM init time. It can help to find
incompatibility issues earlier and refuse to load arch KVM module
(e.g., kvm-intel).
Loosen the WARN_ON in kvm_arch_check_processor_compat so that it
can be invoked from KVM's CPU hotplug callback (i.e., kvm_online_cpu).
Other arch doesn't depends on prohibiting of preemption because powerpc
has "strcmp(cur_cpu_spec->cpu_name, "model name")" and other arch has
"return 0". Only x86 kvm_arch_check_processor_compat() has issue.
Opportunistically, add a pr_err() for setup_vmcs_config() path in
vmx_check_processor_compatibility() so that each possible error path has
its own error message. Convert printk(KERN_ERR ... to pr_err to please
checkpatch.pl
Signed-off-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/r/20220216031528.92558-7-chao.gao@intel.com
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Yuan Yao <yuan.yao@intel.com>
---
arch/x86/kvm/vmx/vmx.c | 10 ++++++----
arch/x86/kvm/x86.c | 11 +++++++++--
virt/kvm/kvm_main.c | 18 +++++++++++++++++-
3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 3cf7f18a4115..2a1ab6495299 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7421,20 +7421,22 @@ static int vmx_check_processor_compatibility(void)
{
struct vmcs_config vmcs_conf;
struct vmx_capability vmx_cap;
+ int cpu = smp_processor_id();
if (!this_cpu_has(X86_FEATURE_MSR_IA32_FEAT_CTL) ||
!this_cpu_has(X86_FEATURE_VMX)) {
- pr_err("kvm: VMX is disabled on CPU %d\n", smp_processor_id());
+ pr_err("kvm: VMX is disabled on CPU %d\n", cpu);
return -EIO;
}
- if (setup_vmcs_config(&vmcs_conf, &vmx_cap) < 0)
+ if (setup_vmcs_config(&vmcs_conf, &vmx_cap) < 0) {
+ pr_err("kvm: failed to setup vmcs config on CPU %d\n", cpu);
return -EIO;
+ }
if (nested)
nested_vmx_setup_ctls_msrs(&vmcs_conf.nested, vmx_cap.ept);
if (memcmp(&vmcs_config, &vmcs_conf, sizeof(struct vmcs_config)) != 0) {
- printk(KERN_ERR "kvm: CPU %d feature inconsistency!\n",
- smp_processor_id());
+ pr_err("kvm: CPU %d feature inconsistency!\n", cpu);
return -EIO;
}
return 0;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f6f014e78ab9..0ab8db07fa0e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -12000,9 +12000,16 @@ void kvm_arch_hardware_unsetup(void)
int kvm_arch_check_processor_compat(void)
{
- struct cpuinfo_x86 *c = &cpu_data(smp_processor_id());
+ int cpu = smp_processor_id();
+ struct cpuinfo_x86 *c = &cpu_data(cpu);
- WARN_ON(!irqs_disabled());
+ /*
+ * Compatibility checks are done when loading KVM or in KVM's CPU
+ * hotplug callback. It ensures all online CPUs are compatible to run
+ * vCPUs. For other cases, compatibility checks are unnecessary or
+ * even problematic. Try to detect improper usages here.
+ */
+ WARN_ON(!irqs_disabled() && cpu_active(cpu));
if (__cr4_reserved_bits(cpu_has, c) !=
__cr4_reserved_bits(cpu_has, &boot_cpu_data))
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index db1303e2abc9..0ac00c711384 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5013,7 +5013,11 @@ static void hardware_enable_nolock(void *caller_name)
static int kvm_online_cpu(unsigned int cpu)
{
- int ret = 0;
+ int ret;
+
+ ret = kvm_arch_check_processor_compat();
+ if (ret)
+ return ret;
raw_spin_lock(&kvm_count_lock);
/*
@@ -5073,6 +5077,17 @@ static int hardware_enable_all(void)
{
int r = 0;
+ /*
+ * During onlining a CPU, cpu_online_mask is set before kvm_online_cpu()
+ * is called. on_each_cpu() between them includes the CPU. As a result,
+ * hardware_enable_nolock() may get invoked before kvm_online_cpu().
+ * This would enable hardware virtualization on that cpu without
+ * compatibility checks, which can potentially crash system or break
+ * running VMs.
+ *
+ * Disable CPU hotplug to prevent this case from happening.
+ */
+ cpus_read_lock();
raw_spin_lock(&kvm_count_lock);
kvm_usage_count++;
@@ -5087,6 +5102,7 @@ static int hardware_enable_all(void)
}
raw_spin_unlock(&kvm_count_lock);
+ cpus_read_unlock();
return r;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 09/26] KVM: Do processor compatibility check on resume
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (7 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 08/26] KVM: Do compatibility checks on hotplugged CPUs isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 2:15 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock isaku.yamahata
` (16 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
So far the processor compatibility check is not done on resume. It should
be done. The resume is called for resuming from S3/S4. CPUs can be
replaced or the kernel can resume from S4 on a different machine. So
compatibility check is desirable.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
virt/kvm/kvm_main.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0ac00c711384..fc55447c4dba 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5715,6 +5715,13 @@ static int kvm_suspend(void)
static void kvm_resume(void)
{
+ if (kvm_arch_check_processor_compat())
+ /*
+ * No warning here because kvm_arch_check_processor_compat()
+ * would have warned with more information.
+ */
+ return; /* FIXME: disable KVM */
+
if (kvm_usage_count) {
lockdep_assert_not_held(&kvm_count_lock);
hardware_enable_nolock((void *)__func__);
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (8 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 09/26] KVM: Do processor compatibility check on resume isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 3:05 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub isaku.yamahata
` (15 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
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. kvm_arch_hardware_enable/disable() callbacks depend on
non-preemptiblity with the spin lock. Add preempt_disable/enable()
around hardware enable/disable callback to keep the assumption.
Opportunistically add some comments on locking.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
Documentation/virt/kvm/locking.rst | 14 +++-----
virt/kvm/kvm_main.c | 56 +++++++++++++++++++++++-------
2 files changed, 49 insertions(+), 21 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 fc55447c4dba..05ede37edc31 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 *caller_name)
int cpu = raw_smp_processor_id();
int r;
+ WARN_ON_ONCE(preemptible());
+
if (cpumask_test_cpu(cpu, cpus_hardware_enabled))
return;
@@ -5019,7 +5020,7 @@ static int kvm_online_cpu(unsigned int cpu)
if (ret)
return ret;
- 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
@@ -5028,13 +5029,20 @@ static int kvm_online_cpu(unsigned int cpu)
if (kvm_usage_count) {
WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
+ /*
+ * arch callback kvm_arch_hardware_eanble() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
hardware_enable_nolock((void *)__func__);
+ preempt_enable();
if (atomic_read(&hardware_enable_failed)) {
atomic_set(&hardware_enable_failed, 0);
ret = -EIO;
}
}
- raw_spin_unlock(&kvm_count_lock);
+ mutex_unlock(&kvm_lock);
return ret;
}
@@ -5042,6 +5050,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);
@@ -5050,10 +5060,18 @@ static void hardware_disable_nolock(void *junk)
static int kvm_offline_cpu(unsigned int cpu)
{
- raw_spin_lock(&kvm_count_lock);
- if (kvm_usage_count)
+ mutex_lock(&kvm_lock);
+ if (kvm_usage_count) {
+ /*
+ * arch callback kvm_arch_hardware_disable() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
hardware_disable_nolock(NULL);
- raw_spin_unlock(&kvm_count_lock);
+ preempt_enable();
+ }
+ mutex_unlock(&kvm_lock);
return 0;
}
@@ -5068,9 +5086,11 @@ 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)
@@ -5088,7 +5108,7 @@ static int hardware_enable_all(void)
* Disable CPU hotplug to prevent this case from happening.
*/
cpus_read_lock();
- raw_spin_lock(&kvm_count_lock);
+ mutex_lock(&kvm_lock);
kvm_usage_count++;
if (kvm_usage_count == 1) {
@@ -5101,7 +5121,7 @@ static int hardware_enable_all(void)
}
}
- raw_spin_unlock(&kvm_count_lock);
+ mutex_unlock(&kvm_lock);
cpus_read_unlock();
return r;
@@ -5708,8 +5728,18 @@ 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.
+ */
+ lockdep_assert_not_held(&kvm_lock);
+
+ if (kvm_usage_count) {
+ preempt_disable();
hardware_disable_nolock(NULL);
+ preempt_enable();
+ }
return 0;
}
@@ -5723,8 +5753,10 @@ static void kvm_resume(void)
return; /* FIXME: disable KVM */
if (kvm_usage_count) {
- lockdep_assert_not_held(&kvm_count_lock);
+ lockdep_assert_not_held(&kvm_lock);
+ preempt_disable();
hardware_enable_nolock((void *)__func__);
+ preempt_enable();
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (9 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 4:34 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback isaku.yamahata
` (14 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Add arch hooks for reboot, suspend, resume, and CPU-online/offline events
with empty stub functions.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
include/linux/kvm_host.h | 6 +++++
virt/kvm/Makefile.kvm | 2 +-
virt/kvm/kvm_arch.c | 44 ++++++++++++++++++++++++++++++
virt/kvm/kvm_main.c | 58 +++++++++++++++++++++++++---------------
4 files changed, 88 insertions(+), 22 deletions(-)
create mode 100644 virt/kvm/kvm_arch.c
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index eab352902de7..dd2a6d98d4de 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1448,6 +1448,12 @@ int kvm_arch_post_init_vm(struct kvm *kvm);
void kvm_arch_pre_destroy_vm(struct kvm *kvm);
int kvm_arch_create_vm_debugfs(struct kvm *kvm);
+int kvm_arch_suspend(int usage_count);
+void kvm_arch_resume(int usage_count);
+int kvm_arch_reboot(int val);
+int kvm_arch_online_cpu(unsigned int cpu, int usage_count);
+int kvm_arch_offline_cpu(unsigned int cpu, int usage_count);
+
#ifndef __KVM_HAVE_ARCH_VM_ALLOC
/*
* All architectures that want to use vzalloc currently also
diff --git a/virt/kvm/Makefile.kvm b/virt/kvm/Makefile.kvm
index 2c27d5d0c367..c4210acabd35 100644
--- a/virt/kvm/Makefile.kvm
+++ b/virt/kvm/Makefile.kvm
@@ -5,7 +5,7 @@
KVM ?= ../../../virt/kvm
-kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
+kvm-y := $(KVM)/kvm_main.o $(KVM)/kvm_arch.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o
kvm-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o
kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
new file mode 100644
index 000000000000..4748a76bcb03
--- /dev/null
+++ b/virt/kvm/kvm_arch.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * kvm_arch.c: kvm default arch hooks for hardware enabling/disabling
+ * Copyright (c) 2022 Intel Corporation.
+ *
+ * Author:
+ * Isaku Yamahata <isaku.yamahata@intel.com>
+ * <isaku.yamahata@gmail.com>
+ */
+
+#include <linux/kvm_host.h>
+
+/*
+ * Called after the VM is otherwise initialized, but just before adding it to
+ * the vm_list.
+ */
+__weak int kvm_arch_post_init_vm(struct kvm *kvm)
+{
+ return 0;
+}
+
+__weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
+{
+ return 0;
+}
+
+__weak int kvm_arch_offline_cpu(unsigned int cpu, int usage_count)
+{
+ return 0;
+}
+
+__weak int kvm_arch_reboot(int val)
+{
+ return NOTIFY_OK;
+}
+
+__weak int kvm_arch_suspend(int usage_count)
+{
+ return 0;
+}
+
+__weak void kvm_arch_resume(int usage_count)
+{
+}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 05ede37edc31..951f853f6ac9 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -144,6 +144,7 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file)
#endif
static int hardware_enable_all(void);
static void hardware_disable_all(void);
+static void hardware_disable_nolock(void *junk);
static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
@@ -1097,15 +1098,6 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, const char *fdname)
return ret;
}
-/*
- * Called after the VM is otherwise initialized, but just before adding it to
- * the vm_list.
- */
-int __weak kvm_arch_post_init_vm(struct kvm *kvm)
-{
- return 0;
-}
-
/*
* Called just after removing the VM from the vm_list, but before doing any
* other destruction.
@@ -5040,6 +5032,10 @@ static int kvm_online_cpu(unsigned int cpu)
if (atomic_read(&hardware_enable_failed)) {
atomic_set(&hardware_enable_failed, 0);
ret = -EIO;
+ } else {
+ ret = kvm_arch_online_cpu(cpu, kvm_usage_count);
+ if (ret)
+ hardware_disable_nolock(NULL);
}
}
mutex_unlock(&kvm_lock);
@@ -5060,6 +5056,8 @@ static void hardware_disable_nolock(void *junk)
static int kvm_offline_cpu(unsigned int cpu)
{
+ int ret = 0;
+
mutex_lock(&kvm_lock);
if (kvm_usage_count) {
/*
@@ -5069,10 +5067,15 @@ static int kvm_offline_cpu(unsigned int cpu)
*/
preempt_disable();
hardware_disable_nolock(NULL);
+ ret = kvm_arch_offline_cpu(cpu, kvm_usage_count);
+ if (ret) {
+ (void)hardware_enable_nolock(NULL);
+ atomic_set(&hardware_enable_failed, 0);
+ }
preempt_enable();
}
mutex_unlock(&kvm_lock);
- return 0;
+ return ret;
}
static void hardware_disable_all_nolock(void)
@@ -5130,6 +5133,8 @@ static int hardware_enable_all(void)
static int kvm_reboot(struct notifier_block *notifier, unsigned long val,
void *v)
{
+ int r;
+
/*
* Some (well, at least mine) BIOSes hang on reboot if
* in vmx root mode.
@@ -5138,8 +5143,15 @@ static int kvm_reboot(struct notifier_block *notifier, unsigned long val,
*/
pr_info("kvm: exiting hardware virtualization\n");
kvm_rebooting = true;
+
+ /* This hook is called without cpuhotplug disabled. */
+ cpus_read_lock();
+ mutex_lock(&kvm_lock);
on_each_cpu(hardware_disable_nolock, NULL, 1);
- return NOTIFY_OK;
+ r = kvm_arch_reboot(val);
+ mutex_unlock(&kvm_lock);
+ cpus_read_unlock();
+ return r;
}
static struct notifier_block kvm_reboot_notifier = {
@@ -5728,6 +5740,8 @@ static void kvm_init_debug(void)
static int kvm_suspend(void)
{
+ int ret;
+
/*
* The caller ensures that CPU hotlug is disabled by
* cpu_hotplug_disable() and other CPUs are offlined. No need for
@@ -5735,16 +5749,19 @@ static int kvm_suspend(void)
*/
lockdep_assert_not_held(&kvm_lock);
- if (kvm_usage_count) {
- preempt_disable();
+ preempt_disable();
+ if (kvm_usage_count)
hardware_disable_nolock(NULL);
- preempt_enable();
- }
- return 0;
+ ret = kvm_arch_suspend(kvm_usage_count);
+ preempt_enable();
+
+ return ret;
}
static void kvm_resume(void)
{
+ lockdep_assert_not_held(&kvm_lock);
+
if (kvm_arch_check_processor_compat())
/*
* No warning here because kvm_arch_check_processor_compat()
@@ -5752,12 +5769,11 @@ static void kvm_resume(void)
*/
return; /* FIXME: disable KVM */
- if (kvm_usage_count) {
- lockdep_assert_not_held(&kvm_lock);
- preempt_disable();
+ preempt_disable();
+ if (kvm_usage_count)
hardware_enable_nolock((void *)__func__);
- preempt_enable();
- }
+ kvm_arch_resume(kvm_usage_count);
+ preempt_enable();
}
static struct syscore_ops kvm_syscore_ops = {
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (10 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 5:48 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted isaku.yamahata
` (13 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
commit 0dd6a6edb012 ("KVM: Dont mark TSC unstable due to S4 suspend") made
use of kvm_arch_hardware_enable() callback to detect that TSC goes backward
due to S4 suspend. It has to check it only when resuming from S4. Not
every time virtualization hardware ennoblement. Move the logic to
kvm_arch_resume() callback.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0ab8db07fa0e..c733f65aaf3c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11835,18 +11835,27 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
EXPORT_SYMBOL_GPL(kvm_vcpu_deliver_sipi_vector);
int kvm_arch_hardware_enable(void)
+{
+ return static_call(kvm_x86_hardware_enable)();
+}
+
+void kvm_arch_hardware_disable(void)
+{
+ static_call(kvm_x86_hardware_disable)();
+ drop_user_return_notifiers();
+}
+
+void kvm_arch_resume(int usage_count)
{
struct kvm *kvm;
struct kvm_vcpu *vcpu;
unsigned long i;
- int ret;
u64 local_tsc;
u64 max_tsc = 0;
bool stable, backwards_tsc = false;
- ret = static_call(kvm_x86_hardware_enable)();
- if (ret != 0)
- return ret;
+ if (!usage_count)
+ return;
local_tsc = rdtsc();
stable = !kvm_check_tsc_unstable();
@@ -11921,13 +11930,6 @@ int kvm_arch_hardware_enable(void)
}
}
- return 0;
-}
-
-void kvm_arch_hardware_disable(void)
-{
- static_call(kvm_x86_hardware_disable)();
- drop_user_return_notifiers();
}
static inline void kvm_ops_update(struct kvm_x86_init_ops *ops)
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (11 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 6:08 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 14/26] KVM: Move out KVM arch PM hooks and hardware enable/disable logic isaku.yamahata
` (12 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
and pass kvm_usage_count with kvm_lock. Move kvm_arch_post_init_vm() under
kvm_arch_add_vm(). Later kvm_arch_post_init_vm() is deleted once x86
overrides kvm_arch_add_vm().
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
include/linux/kvm_host.h | 2 ++
virt/kvm/kvm_arch.c | 12 +++++++++++-
virt/kvm/kvm_main.c | 21 +++++++++++++++++----
3 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index dd2a6d98d4de..f78364e01ca9 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1445,6 +1445,8 @@ int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
bool kvm_arch_dy_runnable(struct kvm_vcpu *vcpu);
bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
int kvm_arch_post_init_vm(struct kvm *kvm);
+int kvm_arch_add_vm(struct kvm *kvm, int usage_count);
+int kvm_arch_del_vm(int usage_count);
void kvm_arch_pre_destroy_vm(struct kvm *kvm);
int kvm_arch_create_vm_debugfs(struct kvm *kvm);
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 4748a76bcb03..0eac996f4981 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -10,11 +10,21 @@
#include <linux/kvm_host.h>
+__weak int kvm_arch_post_init_vm(struct kvm *kvm)
+{
+ return 0;
+}
+
/*
* Called after the VM is otherwise initialized, but just before adding it to
* the vm_list.
*/
-__weak int kvm_arch_post_init_vm(struct kvm *kvm)
+__weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
+{
+ return kvm_arch_post_init_vm(kvm);
+}
+
+__weak int kvm_arch_del_vm(int usage_count)
{
return 0;
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 951f853f6ac9..7acc35e279ec 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -145,6 +145,7 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file)
static int hardware_enable_all(void);
static void hardware_disable_all(void);
static void hardware_disable_nolock(void *junk);
+static void kvm_del_vm(void);
static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
@@ -1215,11 +1216,12 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
if (r)
goto out_err_no_debugfs;
- r = kvm_arch_post_init_vm(kvm);
- if (r)
- goto out_err;
-
mutex_lock(&kvm_lock);
+ r = kvm_arch_add_vm(kvm, kvm_usage_count);
+ if (r) {
+ mutex_unlock(&kvm_lock);
+ goto out_err;
+ }
list_add(&kvm->vm_list, &vm_list);
mutex_unlock(&kvm_lock);
@@ -1239,6 +1241,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
#endif
out_err_no_mmu_notifier:
hardware_disable_all();
+ kvm_del_vm();
out_err_no_disable:
kvm_arch_destroy_vm(kvm);
out_err_no_arch_destroy_vm:
@@ -1319,6 +1322,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
kvm_arch_free_vm(kvm);
preempt_notifier_dec();
hardware_disable_all();
+ kvm_del_vm();
mmdrop(mm);
module_put(kvm_chardev_ops.owner);
}
@@ -5096,6 +5100,15 @@ static void hardware_disable_all(void)
cpus_read_unlock();
}
+static void kvm_del_vm(void)
+{
+ cpus_read_lock();
+ mutex_lock(&kvm_lock);
+ kvm_arch_del_vm(kvm_usage_count);
+ mutex_unlock(&kvm_lock);
+ cpus_read_unlock();
+}
+
static int hardware_enable_all(void)
{
int r = 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 14/26] KVM: Move out KVM arch PM hooks and hardware enable/disable logic
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (12 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 15/26] KVM: kvm_arch.c: Remove _nolock post fix isaku.yamahata
` (11 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
To make clear that those files are default implementation that KVM/x86 (and
other KVM arch in future) will override them, split out those into a single
file. Once conversions for all kvm archs are done, the file will be
deleted. kvm_arch_pre_hardware_unsetup() is introduced to avoid cross-arch
code churn for now. Once it's settled down,
kvm_arch_pre_hardware_unsetup() can be merged into
kvm_arch_hardware_unsetup() in each arch code. x86 kvm_arch_resume() needs
to be adjusted to do CPU compatibility check and virtualization hardware
enabling.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 10 ++
include/linux/kvm_host.h | 1 +
virt/kvm/kvm_arch.c | 124 +++++++++++++++++++++++-
virt/kvm/kvm_main.c | 198 +++++----------------------------------
4 files changed, 156 insertions(+), 177 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c733f65aaf3c..23623b6a789b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11854,9 +11854,19 @@ void kvm_arch_resume(int usage_count)
u64 max_tsc = 0;
bool stable, backwards_tsc = false;
+ if (kvm_arch_check_processor_compat())
+ return; /* FIXME: disable KVM. */
+
if (!usage_count)
return;
+ preempt_disable();
+ if (kvm_arch_hardware_enable()) {
+ preempt_enable();
+ return;
+ }
+ preempt_enable();
+
local_tsc = rdtsc();
stable = !kvm_check_tsc_unstable();
list_for_each_entry(kvm, &vm_list, vm_list) {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f78364e01ca9..60f4ae9d6f48 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1437,6 +1437,7 @@ static inline void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu) {}
int kvm_arch_hardware_enable(void);
void kvm_arch_hardware_disable(void);
int kvm_arch_hardware_setup(void *opaque);
+void kvm_arch_pre_hardware_unsetup(void);
void kvm_arch_hardware_unsetup(void);
int kvm_arch_check_processor_compat(void);
int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 0eac996f4981..32befdbf7d6e 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -6,49 +6,169 @@
* Author:
* Isaku Yamahata <isaku.yamahata@intel.com>
* <isaku.yamahata@gmail.com>
+ *
+ * TODO: Delete this file once the conversion of all KVM arch is done.
*/
#include <linux/kvm_host.h>
+static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
+static atomic_t hardware_enable_failed;
+
__weak int kvm_arch_post_init_vm(struct kvm *kvm)
{
return 0;
}
+static void hardware_enable_nolock(void *caller_name)
+{
+ int cpu = raw_smp_processor_id();
+ int r;
+
+ WARN_ON_ONCE(preemptible());
+
+ if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
+ return;
+
+ cpumask_set_cpu(cpu, &cpus_hardware_enabled);
+
+ r = kvm_arch_hardware_enable();
+
+ if (r) {
+ cpumask_clear_cpu(cpu, &cpus_hardware_enabled);
+ atomic_inc(&hardware_enable_failed);
+ pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
+ cpu, (const char *)caller_name);
+ }
+}
+
+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);
+ kvm_arch_hardware_disable();
+}
+
+__weak void kvm_arch_pre_hardware_unsetup(void)
+{
+ on_each_cpu(hardware_disable_nolock, NULL, 1);
+}
+
/*
* Called after the VM is otherwise initialized, but just before adding it to
* the vm_list.
*/
__weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
{
- return kvm_arch_post_init_vm(kvm);
+ int r = 0;
+
+ if (usage_count != 1)
+ return 0;
+
+ atomic_set(&hardware_enable_failed, 0);
+ on_each_cpu(hardware_enable_nolock, (void *)__func__, 1);
+
+ if (atomic_read(&hardware_enable_failed)) {
+ r = -EBUSY;
+ goto err;
+ }
+
+ r = kvm_arch_post_init_vm(kvm);
+err:
+ if (r)
+ on_each_cpu(hardware_disable_nolock, NULL, 1);
+ return r;
}
__weak int kvm_arch_del_vm(int usage_count)
{
+ if (usage_count)
+ return 0;
+
+ on_each_cpu(hardware_disable_nolock, NULL, 1);
return 0;
}
__weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
{
- return 0;
+ int ret = 0;
+
+ ret = kvm_arch_check_processor_compat();
+ if (ret)
+ return ret;
+
+ /*
+ * Abort the CPU online process if hardware virtualization cannot
+ * be enabled. Otherwise running VMs would encounter unrecoverable
+ * errors when scheduled to this CPU.
+ */
+ if (usage_count) {
+ WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
+
+ /*
+ * arch callback kvm_arch_hardware_eanble() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
+ hardware_enable_nolock((void *)__func__);
+ preempt_enable();
+ if (atomic_read(&hardware_enable_failed)) {
+ atomic_set(&hardware_enable_failed, 0);
+ ret = -EIO;
+ }
+ }
+ return ret;
}
__weak int kvm_arch_offline_cpu(unsigned int cpu, int usage_count)
{
+ if (usage_count) {
+ /*
+ * arch callback kvm_arch_hardware_disable() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
+ hardware_disable_nolock(NULL);
+ preempt_enable();
+ }
return 0;
}
__weak int kvm_arch_reboot(int val)
{
+ on_each_cpu(hardware_disable_nolock, NULL, 1);
return NOTIFY_OK;
}
__weak int kvm_arch_suspend(int usage_count)
{
+ if (usage_count) {
+ preempt_disable();
+ hardware_disable_nolock(NULL);
+ preempt_enable();
+ }
return 0;
}
__weak void kvm_arch_resume(int usage_count)
{
+ if (kvm_arch_check_processor_compat())
+ /*
+ * No warning here because kvm_arch_check_processor_compat()
+ * would have warned with more information.
+ */
+ return; /* FIXME: disable KVM */
+
+ if (usage_count) {
+ preempt_disable();
+ hardware_enable_nolock((void *)__func__);
+ preempt_enable();
+ }
}
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7acc35e279ec..5373127dcdb6 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -102,9 +102,7 @@ EXPORT_SYMBOL_GPL(halt_poll_ns_shrink);
DEFINE_MUTEX(kvm_lock);
LIST_HEAD(vm_list);
-static cpumask_var_t cpus_hardware_enabled;
static int kvm_usage_count;
-static atomic_t hardware_enable_failed;
static struct kmem_cache *kvm_vcpu_cache;
@@ -142,9 +140,6 @@ static int kvm_no_compat_open(struct inode *inode, struct file *file)
#define KVM_COMPAT(c) .compat_ioctl = kvm_no_compat_ioctl, \
.open = kvm_no_compat_open
#endif
-static int hardware_enable_all(void);
-static void hardware_disable_all(void);
-static void hardware_disable_nolock(void *junk);
static void kvm_del_vm(void);
static void kvm_io_bus_destroy(struct kvm_io_bus *bus);
@@ -1196,10 +1191,6 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
if (r)
goto out_err_no_arch_destroy_vm;
- r = hardware_enable_all();
- if (r)
- goto out_err_no_disable;
-
#ifdef CONFIG_HAVE_KVM_IRQFD
INIT_HLIST_HEAD(&kvm->irq_ack_notifier_list);
#endif
@@ -1216,14 +1207,28 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
if (r)
goto out_err_no_debugfs;
+ /*
+ * During onlining a CPU, cpu_online_mask is set before kvm_online_cpu()
+ * is called. on_each_cpu() between them includes the CPU. As a result,
+ * hardware_enable_nolock() may get invoked before kvm_online_cpu().
+ * This would enable hardware virtualization on that cpu without
+ * compatibility checks, which can potentially crash system or break
+ * running VMs.
+ *
+ * Disable CPU hotplug to prevent this case from happening.
+ */
+ cpus_read_lock();
mutex_lock(&kvm_lock);
+ kvm_usage_count++;
r = kvm_arch_add_vm(kvm, kvm_usage_count);
if (r) {
+ /* the following kvm_del_vm() decrements kvm_usage_count. */
mutex_unlock(&kvm_lock);
goto out_err;
}
list_add(&kvm->vm_list, &vm_list);
mutex_unlock(&kvm_lock);
+ cpus_read_unlock();
preempt_notifier_inc();
kvm_init_pm_notifier(kvm);
@@ -1240,9 +1245,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
mmu_notifier_unregister(&kvm->mmu_notifier, current->mm);
#endif
out_err_no_mmu_notifier:
- hardware_disable_all();
kvm_del_vm();
-out_err_no_disable:
kvm_arch_destroy_vm(kvm);
out_err_no_arch_destroy_vm:
WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
@@ -1321,7 +1324,6 @@ static void kvm_destroy_vm(struct kvm *kvm)
cleanup_srcu_struct(&kvm->srcu);
kvm_arch_free_vm(kvm);
preempt_notifier_dec();
- hardware_disable_all();
kvm_del_vm();
mmdrop(mm);
module_put(kvm_chardev_ops.owner);
@@ -4986,163 +4988,37 @@ static struct miscdevice kvm_dev = {
&kvm_chardev_ops,
};
-static void hardware_enable_nolock(void *caller_name)
-{
- int cpu = raw_smp_processor_id();
- int r;
-
- WARN_ON_ONCE(preemptible());
-
- if (cpumask_test_cpu(cpu, cpus_hardware_enabled))
- return;
-
- cpumask_set_cpu(cpu, cpus_hardware_enabled);
-
- r = kvm_arch_hardware_enable();
-
- if (r) {
- cpumask_clear_cpu(cpu, cpus_hardware_enabled);
- atomic_inc(&hardware_enable_failed);
- pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
- cpu, (const char *)caller_name);
- }
-}
-
static int kvm_online_cpu(unsigned int cpu)
{
int ret;
- ret = kvm_arch_check_processor_compat();
- if (ret)
- return ret;
-
mutex_lock(&kvm_lock);
- /*
- * 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));
-
- /*
- * arch callback kvm_arch_hardware_eanble() assumes that
- * preemption is disabled for historical reason. Disable
- * preemption until all arch callbacks are fixed.
- */
- preempt_disable();
- hardware_enable_nolock((void *)__func__);
- preempt_enable();
- if (atomic_read(&hardware_enable_failed)) {
- atomic_set(&hardware_enable_failed, 0);
- ret = -EIO;
- } else {
- ret = kvm_arch_online_cpu(cpu, kvm_usage_count);
- if (ret)
- hardware_disable_nolock(NULL);
- }
- }
+ ret = kvm_arch_online_cpu(cpu, kvm_usage_count);
mutex_unlock(&kvm_lock);
return ret;
}
-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);
- kvm_arch_hardware_disable();
-}
-
static int kvm_offline_cpu(unsigned int cpu)
{
- int ret = 0;
+ int ret;
mutex_lock(&kvm_lock);
- if (kvm_usage_count) {
- /*
- * arch callback kvm_arch_hardware_disable() assumes that
- * preemption is disabled for historical reason. Disable
- * preemption until all arch callbacks are fixed.
- */
- preempt_disable();
- hardware_disable_nolock(NULL);
- ret = kvm_arch_offline_cpu(cpu, kvm_usage_count);
- if (ret) {
- (void)hardware_enable_nolock(NULL);
- atomic_set(&hardware_enable_failed, 0);
- }
- preempt_enable();
- }
+ ret = kvm_arch_offline_cpu(cpu, kvm_usage_count);
mutex_unlock(&kvm_lock);
return ret;
}
-static void hardware_disable_all_nolock(void)
-{
- BUG_ON(!kvm_usage_count);
-
- kvm_usage_count--;
- if (!kvm_usage_count)
- on_each_cpu(hardware_disable_nolock, NULL, 1);
-}
-
-static void hardware_disable_all(void)
-{
- cpus_read_lock();
- mutex_lock(&kvm_lock);
- hardware_disable_all_nolock();
- mutex_unlock(&kvm_lock);
- cpus_read_unlock();
-}
-
static void kvm_del_vm(void)
{
cpus_read_lock();
mutex_lock(&kvm_lock);
+ WARN_ON_ONCE(!kvm_usage_count);
+ kvm_usage_count--;
kvm_arch_del_vm(kvm_usage_count);
mutex_unlock(&kvm_lock);
cpus_read_unlock();
}
-static int hardware_enable_all(void)
-{
- int r = 0;
-
- /*
- * During onlining a CPU, cpu_online_mask is set before kvm_online_cpu()
- * is called. on_each_cpu() between them includes the CPU. As a result,
- * hardware_enable_nolock() may get invoked before kvm_online_cpu().
- * This would enable hardware virtualization on that cpu without
- * compatibility checks, which can potentially crash system or break
- * running VMs.
- *
- * Disable CPU hotplug to prevent this case from happening.
- */
- cpus_read_lock();
- mutex_lock(&kvm_lock);
-
- kvm_usage_count++;
- if (kvm_usage_count == 1) {
- atomic_set(&hardware_enable_failed, 0);
- on_each_cpu(hardware_enable_nolock, (void *)__func__, 1);
-
- if (atomic_read(&hardware_enable_failed)) {
- hardware_disable_all_nolock();
- r = -EBUSY;
- }
- }
-
- mutex_unlock(&kvm_lock);
- cpus_read_unlock();
-
- return r;
-}
-
static int kvm_reboot(struct notifier_block *notifier, unsigned long val,
void *v)
{
@@ -5160,7 +5036,6 @@ static int kvm_reboot(struct notifier_block *notifier, unsigned long val,
/* This hook is called without cpuhotplug disabled. */
cpus_read_lock();
mutex_lock(&kvm_lock);
- on_each_cpu(hardware_disable_nolock, NULL, 1);
r = kvm_arch_reboot(val);
mutex_unlock(&kvm_lock);
cpus_read_unlock();
@@ -5753,40 +5628,19 @@ static void kvm_init_debug(void)
static int kvm_suspend(void)
{
- int ret;
-
/*
* The caller ensures that CPU hotlug is disabled by
* cpu_hotplug_disable() and other CPUs are offlined. No need for
* locking.
*/
lockdep_assert_not_held(&kvm_lock);
-
- preempt_disable();
- if (kvm_usage_count)
- hardware_disable_nolock(NULL);
- ret = kvm_arch_suspend(kvm_usage_count);
- preempt_enable();
-
- return ret;
+ return kvm_arch_suspend(kvm_usage_count);
}
static void kvm_resume(void)
{
lockdep_assert_not_held(&kvm_lock);
-
- if (kvm_arch_check_processor_compat())
- /*
- * No warning here because kvm_arch_check_processor_compat()
- * would have warned with more information.
- */
- return; /* FIXME: disable KVM */
-
- preempt_disable();
- if (kvm_usage_count)
- hardware_enable_nolock((void *)__func__);
kvm_arch_resume(kvm_usage_count);
- preempt_enable();
}
static struct syscore_ops kvm_syscore_ops = {
@@ -5924,11 +5778,6 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
if (r)
goto out_irqfd;
- if (!zalloc_cpumask_var(&cpus_hardware_enabled, GFP_KERNEL)) {
- r = -ENOMEM;
- goto out_free_0;
- }
-
r = kvm_arch_hardware_setup(opaque);
if (r < 0)
goto out_free_1;
@@ -6005,8 +5854,6 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
out_free_2:
kvm_arch_hardware_unsetup();
out_free_1:
- free_cpumask_var(cpus_hardware_enabled);
-out_free_0:
kvm_irqfd_exit();
out_irqfd:
kvm_arch_exit();
@@ -6028,11 +5875,12 @@ void kvm_exit(void)
unregister_syscore_ops(&kvm_syscore_ops);
unregister_reboot_notifier(&kvm_reboot_notifier);
cpuhp_remove_state_nocalls(CPUHP_AP_KVM_ONLINE);
- on_each_cpu(hardware_disable_nolock, NULL, 1);
+ cpus_read_lock();
+ kvm_arch_pre_hardware_unsetup();
kvm_arch_hardware_unsetup();
+ cpus_read_unlock();
kvm_arch_exit();
kvm_irqfd_exit();
- free_cpumask_var(cpus_hardware_enabled);
kvm_vfio_ops_exit();
}
EXPORT_SYMBOL_GPL(kvm_exit);
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 15/26] KVM: kvm_arch.c: Remove _nolock post fix
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (13 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 14/26] KVM: Move out KVM arch PM hooks and hardware enable/disable logic isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 6:30 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed isaku.yamahata
` (10 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Now all related callbacks are called under kvm_lock, no point for _nolock
post fix. Remove _nolock post fix for readability with shorter function
names.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
virt/kvm/kvm_arch.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 32befdbf7d6e..4fe16e8ef2e5 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -20,7 +20,7 @@ __weak int kvm_arch_post_init_vm(struct kvm *kvm)
return 0;
}
-static void hardware_enable_nolock(void *caller_name)
+static void hardware_enable(void *caller_name)
{
int cpu = raw_smp_processor_id();
int r;
@@ -42,7 +42,7 @@ static void hardware_enable_nolock(void *caller_name)
}
}
-static void hardware_disable_nolock(void *junk)
+static void hardware_disable(void *junk)
{
int cpu = raw_smp_processor_id();
@@ -56,7 +56,7 @@ static void hardware_disable_nolock(void *junk)
__weak void kvm_arch_pre_hardware_unsetup(void)
{
- on_each_cpu(hardware_disable_nolock, NULL, 1);
+ on_each_cpu(hardware_disable, NULL, 1);
}
/*
@@ -71,7 +71,7 @@ __weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
return 0;
atomic_set(&hardware_enable_failed, 0);
- on_each_cpu(hardware_enable_nolock, (void *)__func__, 1);
+ on_each_cpu(hardware_enable, (void *)__func__, 1);
if (atomic_read(&hardware_enable_failed)) {
r = -EBUSY;
@@ -81,7 +81,7 @@ __weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
r = kvm_arch_post_init_vm(kvm);
err:
if (r)
- on_each_cpu(hardware_disable_nolock, NULL, 1);
+ on_each_cpu(hardware_disable, NULL, 1);
return r;
}
@@ -90,7 +90,7 @@ __weak int kvm_arch_del_vm(int usage_count)
if (usage_count)
return 0;
- on_each_cpu(hardware_disable_nolock, NULL, 1);
+ on_each_cpu(hardware_disable, NULL, 1);
return 0;
}
@@ -116,7 +116,7 @@ __weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
* preemption until all arch callbacks are fixed.
*/
preempt_disable();
- hardware_enable_nolock((void *)__func__);
+ hardware_enable((void *)__func__);
preempt_enable();
if (atomic_read(&hardware_enable_failed)) {
atomic_set(&hardware_enable_failed, 0);
@@ -135,7 +135,7 @@ __weak int kvm_arch_offline_cpu(unsigned int cpu, int usage_count)
* preemption until all arch callbacks are fixed.
*/
preempt_disable();
- hardware_disable_nolock(NULL);
+ hardware_disable(NULL);
preempt_enable();
}
return 0;
@@ -143,7 +143,7 @@ __weak int kvm_arch_offline_cpu(unsigned int cpu, int usage_count)
__weak int kvm_arch_reboot(int val)
{
- on_each_cpu(hardware_disable_nolock, NULL, 1);
+ on_each_cpu(hardware_disable, NULL, 1);
return NOTIFY_OK;
}
@@ -151,7 +151,7 @@ __weak int kvm_arch_suspend(int usage_count)
{
if (usage_count) {
preempt_disable();
- hardware_disable_nolock(NULL);
+ hardware_disable(NULL);
preempt_enable();
}
return 0;
@@ -168,7 +168,7 @@ __weak void kvm_arch_resume(int usage_count)
if (usage_count) {
preempt_disable();
- hardware_enable_nolock((void *)__func__);
+ hardware_enable((void *)__func__);
preempt_enable();
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (14 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 15/26] KVM: kvm_arch.c: Remove _nolock post fix isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 6:54 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 17/26] KVM: Introduce a arch wrapper to check all processor compatibility isaku.yamahata
` (9 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
A global variable hardware_enable_failed in kvm_arch.c is used only by
kvm_arch_add_vm() and hardware_enable(). It doesn't have to be a global
variable. Make it function local.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
virt/kvm/kvm_arch.c | 62 ++++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 4fe16e8ef2e5..ad23537ebe3b 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -13,14 +13,13 @@
#include <linux/kvm_host.h>
static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
-static atomic_t hardware_enable_failed;
__weak int kvm_arch_post_init_vm(struct kvm *kvm)
{
return 0;
}
-static void hardware_enable(void *caller_name)
+static int __hardware_enable(void *caller_name)
{
int cpu = raw_smp_processor_id();
int r;
@@ -28,18 +27,22 @@ static void hardware_enable(void *caller_name)
WARN_ON_ONCE(preemptible());
if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
- return;
-
- cpumask_set_cpu(cpu, &cpus_hardware_enabled);
-
+ return 0;
r = kvm_arch_hardware_enable();
-
- if (r) {
- cpumask_clear_cpu(cpu, &cpus_hardware_enabled);
- atomic_inc(&hardware_enable_failed);
+ if (r)
pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
cpu, (const char *)caller_name);
- }
+ else
+ cpumask_set_cpu(cpu, &cpus_hardware_enabled);
+ return r;
+}
+
+static void hardware_enable(void *arg)
+{
+ atomic_t *failed = arg;
+
+ if (__hardware_enable((void *)__func__))
+ atomic_inc(failed);
}
static void hardware_disable(void *junk)
@@ -65,15 +68,16 @@ __weak void kvm_arch_pre_hardware_unsetup(void)
*/
__weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
{
+ atomic_t failed;
int r = 0;
if (usage_count != 1)
return 0;
- atomic_set(&hardware_enable_failed, 0);
- on_each_cpu(hardware_enable, (void *)__func__, 1);
+ atomic_set(&failed, 0);
+ on_each_cpu(hardware_enable, &failed, 1);
- if (atomic_read(&hardware_enable_failed)) {
+ if (atomic_read(&failed)) {
r = -EBUSY;
goto err;
}
@@ -96,33 +100,29 @@ __weak int kvm_arch_del_vm(int usage_count)
__weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
{
- int ret = 0;
+ int ret;
ret = kvm_arch_check_processor_compat();
if (ret)
return ret;
+ if (!usage_count)
+ return 0;
+
+ /*
+ * arch callback kvm_arch_hardware_eanble() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
/*
* Abort the CPU online process if hardware virtualization cannot
* be enabled. Otherwise running VMs would encounter unrecoverable
* errors when scheduled to this CPU.
*/
- if (usage_count) {
- WARN_ON_ONCE(atomic_read(&hardware_enable_failed));
+ ret = __hardware_enable((void *)__func__);
+ preempt_enable();
- /*
- * arch callback kvm_arch_hardware_eanble() assumes that
- * preemption is disabled for historical reason. Disable
- * preemption until all arch callbacks are fixed.
- */
- preempt_disable();
- hardware_enable((void *)__func__);
- preempt_enable();
- if (atomic_read(&hardware_enable_failed)) {
- atomic_set(&hardware_enable_failed, 0);
- ret = -EIO;
- }
- }
return ret;
}
@@ -168,7 +168,7 @@ __weak void kvm_arch_resume(int usage_count)
if (usage_count) {
preempt_disable();
- hardware_enable((void *)__func__);
+ (void)__hardware_enable((void *)__func__);
preempt_enable();
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 17/26] KVM: Introduce a arch wrapper to check all processor compatibility
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (15 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 7:32 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 18/26] KVM: x86: Duplicate arch callbacks related to pm events and compat check isaku.yamahata
` (8 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Introduce a arch wrapper to check all processor compatibility and define
default implementation as weak symbol to keep the current logic.
The hardware feature compatibility check is arch dependent, only x86 KVM
does cpu feature check on all processors. It doesn't make much sense to
enforce the current implementation to invoke check function on each
processors. Introduce a arch callback,
kvm_arch_check_processor_compat_all(), so that arch code can override it.
Eventually feature check should be pushed down into arch callback,
(kvm_arch_hardware_setup(), kvm_arch_online_cpu(), and kvm_arch_resume()),
the two compatibility check, kvm_arch_check_processor_compat{,_all}(), will
be eliminated. This is a transitional step for it.
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
include/linux/kvm_host.h | 1 +
virt/kvm/kvm_arch.c | 18 ++++++++++++++++++
virt/kvm/kvm_main.c | 13 +++----------
3 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 60f4ae9d6f48..74cae99fbf09 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1440,6 +1440,7 @@ int kvm_arch_hardware_setup(void *opaque);
void kvm_arch_pre_hardware_unsetup(void);
void kvm_arch_hardware_unsetup(void);
int kvm_arch_check_processor_compat(void);
+int kvm_arch_check_processor_compat_all(void);
int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index ad23537ebe3b..9476c500d571 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -98,6 +98,24 @@ __weak int kvm_arch_del_vm(int usage_count)
return 0;
}
+static void check_processor_compat(void *rtn)
+{
+ *(int *)rtn = kvm_arch_check_processor_compat();
+}
+
+__weak int kvm_arch_check_processor_compat_all(void)
+{
+ int cpu;
+ int r;
+
+ for_each_online_cpu(cpu) {
+ smp_call_function_single(cpu, check_processor_compat, &r, 1);
+ if (r < 0)
+ return r;
+ }
+ return 0;
+}
+
__weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
{
int ret;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 5373127dcdb6..51315d454dc2 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5752,11 +5752,6 @@ void kvm_unregister_perf_callbacks(void)
}
#endif
-static void check_processor_compat(void *rtn)
-{
- *(int *)rtn = kvm_arch_check_processor_compat();
-}
-
int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
struct module *module)
{
@@ -5782,11 +5777,9 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
if (r < 0)
goto out_free_1;
- for_each_online_cpu(cpu) {
- smp_call_function_single(cpu, check_processor_compat, &r, 1);
- if (r < 0)
- goto out_free_2;
- }
+ r = kvm_arch_check_processor_compat_all();
+ if (r < 0)
+ goto out_free_2;
r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online",
kvm_online_cpu, kvm_offline_cpu);
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 18/26] KVM: x86: Duplicate arch callbacks related to pm events and compat check
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (16 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 17/26] KVM: Introduce a arch wrapper to check all processor compatibility isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 19/26] KVM: Eliminate kvm_arch_post_init_vm() isaku.yamahata
` (7 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
KVM/X86 can change those callbacks without worrying about breaking other
archs.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 168 +++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 163 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 23623b6a789b..feee7739219e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11845,6 +11845,169 @@ void kvm_arch_hardware_disable(void)
drop_user_return_notifiers();
}
+static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
+
+int kvm_arch_post_init_vm(struct kvm *kvm)
+{
+ return kvm_mmu_post_init_vm(kvm);
+}
+
+static int __hardware_enable(void *caller_name)
+{
+ 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_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
+ cpu, (const char *)caller_name);
+ else
+ cpumask_set_cpu(cpu, &cpus_hardware_enabled);
+ return r;
+}
+
+static void hardware_enable(void *arg)
+{
+ atomic_t *failed = arg;
+
+ if (__hardware_enable((void *)__func__))
+ atomic_inc(failed);
+}
+
+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();
+}
+
+void kvm_arch_pre_hardware_unsetup(void)
+{
+ on_each_cpu(hardware_disable, NULL, 1);
+}
+
+/*
+ * Called after the VM is otherwise initialized, but just before adding it to
+ * the vm_list.
+ */
+int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
+{
+ atomic_t failed;
+ int r = 0;
+
+ if (usage_count != 1)
+ return 0;
+
+ atomic_set(&failed, 0);
+ on_each_cpu(hardware_enable, &failed, 1);
+
+ if (atomic_read(&failed)) {
+ r = -EBUSY;
+ goto err;
+ }
+
+ r = kvm_arch_post_init_vm(kvm);
+err:
+ if (r)
+ on_each_cpu(hardware_disable, NULL, 1);
+ return r;
+}
+
+int kvm_arch_del_vm(int usage_count)
+{
+ if (usage_count)
+ return 0;
+
+ on_each_cpu(hardware_disable, NULL, 1);
+ return 0;
+}
+
+static void check_processor_compat(void *rtn)
+{
+ *(int *)rtn = kvm_arch_check_processor_compat();
+}
+
+int kvm_arch_check_processor_compat_all(void)
+{
+ int cpu;
+ int r;
+
+ for_each_online_cpu(cpu) {
+ smp_call_function_single(cpu, check_processor_compat, &r, 1);
+ if (r < 0)
+ return r;
+ }
+ return 0;
+}
+
+int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
+{
+ int ret;
+
+ ret = kvm_arch_check_processor_compat();
+ if (ret)
+ return ret;
+
+ if (!usage_count)
+ return 0;
+
+ /*
+ * arch callback kvm_arch_hardware_eanble() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
+ /*
+ * Abort the CPU online process if hardware virtualization cannot
+ * be enabled. Otherwise running VMs would encounter unrecoverable
+ * errors when scheduled to this CPU.
+ */
+ ret = __hardware_enable((void *)__func__);
+ preempt_enable();
+
+ return ret;
+}
+
+int kvm_arch_offline_cpu(unsigned int cpu, int usage_count)
+{
+ if (usage_count) {
+ /*
+ * arch callback kvm_arch_hardware_disable() assumes that
+ * preemption is disabled for historical reason. Disable
+ * preemption until all arch callbacks are fixed.
+ */
+ preempt_disable();
+ hardware_disable(NULL);
+ preempt_enable();
+ }
+ return 0;
+}
+
+int kvm_arch_reboot(int val)
+{
+ on_each_cpu(hardware_disable, NULL, 1);
+ return NOTIFY_OK;
+}
+
+int kvm_arch_suspend(int usage_count)
+{
+ if (usage_count) {
+ preempt_disable();
+ hardware_disable(NULL);
+ preempt_enable();
+ }
+ return 0;
+}
+
void kvm_arch_resume(int usage_count)
{
struct kvm *kvm;
@@ -12122,11 +12285,6 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
return ret;
}
-int kvm_arch_post_init_vm(struct kvm *kvm)
-{
- return kvm_mmu_post_init_vm(kvm);
-}
-
static void kvm_unload_vcpu_mmu(struct kvm_vcpu *vcpu)
{
vcpu_load(vcpu);
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 19/26] KVM: Eliminate kvm_arch_post_init_vm()
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (17 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 18/26] KVM: x86: Duplicate arch callbacks related to pm events and compat check isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 20/26] KVM: Add config to not compile kvm_arch.c isaku.yamahata
` (6 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Now kvm_arch_post_init_vm() is used only by x86 kvm_arch_add_vm(). Other
arch doesn't define it. Merge x86 kvm_arch_post_init_vm() into x86
kvm_arch_add_vm() and eliminate kvm_arch_post_init_vm().
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 7 +------
include/linux/kvm_host.h | 1 -
virt/kvm/kvm_arch.c | 12 +-----------
3 files changed, 2 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index feee7739219e..6dcc6ed90421 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11847,11 +11847,6 @@ void kvm_arch_hardware_disable(void)
static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
-int kvm_arch_post_init_vm(struct kvm *kvm)
-{
- return kvm_mmu_post_init_vm(kvm);
-}
-
static int __hardware_enable(void *caller_name)
{
int cpu = raw_smp_processor_id();
@@ -11915,7 +11910,7 @@ int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
goto err;
}
- r = kvm_arch_post_init_vm(kvm);
+ r = kvm_mmu_post_init_vm(kvm);
err:
if (r)
on_each_cpu(hardware_disable, NULL, 1);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 74cae99fbf09..d0b5fdb084f4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1446,7 +1446,6 @@ bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
bool kvm_arch_dy_runnable(struct kvm_vcpu *vcpu);
bool kvm_arch_dy_has_pending_interrupt(struct kvm_vcpu *vcpu);
-int kvm_arch_post_init_vm(struct kvm *kvm);
int kvm_arch_add_vm(struct kvm *kvm, int usage_count);
int kvm_arch_del_vm(int usage_count);
void kvm_arch_pre_destroy_vm(struct kvm *kvm);
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 9476c500d571..9bf7b3920c44 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -14,11 +14,6 @@
static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
-__weak int kvm_arch_post_init_vm(struct kvm *kvm)
-{
- return 0;
-}
-
static int __hardware_enable(void *caller_name)
{
int cpu = raw_smp_processor_id();
@@ -79,13 +74,8 @@ __weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
if (atomic_read(&failed)) {
r = -EBUSY;
- goto err;
- }
-
- r = kvm_arch_post_init_vm(kvm);
-err:
- if (r)
on_each_cpu(hardware_disable, NULL, 1);
+ }
return r;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 20/26] KVM: Add config to not compile kvm_arch.c
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (18 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 19/26] KVM: Eliminate kvm_arch_post_init_vm() isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 21/26] KVM: x86: Delete kvm_arch_hardware_enable/disable() isaku.yamahata
` (5 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
So that kvm_arch_hardware_enable/disable() aren't required.
Once the conversion of all KVM archs is done, this config and kvm_arch.c
should be removed.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/Kconfig | 1 +
include/linux/kvm_host.h | 3 +++
virt/kvm/Kconfig | 3 +++
virt/kvm/Makefile.kvm | 5 ++++-
4 files changed, 11 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 d0b5fdb084f4..f538fc3356a9 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1434,8 +1434,11 @@ 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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 21/26] KVM: x86: Delete kvm_arch_hardware_enable/disable()
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (19 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 20/26] KVM: Add config to not compile kvm_arch.c isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 22/26] KVM: x86: Make x86 processor compat check callback empty isaku.yamahata
` (4 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Now they're function call and there is no point to keep them.
Opportunistically make kvm_arch_pre_hardware_unsetup() empty.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 27 +++++++--------------------
include/linux/kvm_host.h | 6 ++++--
2 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 6dcc6ed90421..0c9d965859c6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -354,7 +354,7 @@ static void kvm_on_user_return(struct user_return_notifier *urn)
/*
* Disabling irqs at this point since the following code could be
- * interrupted and executed through kvm_arch_hardware_disable()
+ * interrupted and executed through hardware_disable()
*/
local_irq_save(flags);
if (msrs->registered) {
@@ -11834,17 +11834,6 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
}
EXPORT_SYMBOL_GPL(kvm_vcpu_deliver_sipi_vector);
-int kvm_arch_hardware_enable(void)
-{
- return static_call(kvm_x86_hardware_enable)();
-}
-
-void kvm_arch_hardware_disable(void)
-{
- static_call(kvm_x86_hardware_disable)();
- drop_user_return_notifiers();
-}
-
static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
static int __hardware_enable(void *caller_name)
@@ -11856,7 +11845,7 @@ static int __hardware_enable(void *caller_name)
if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
return 0;
- r = kvm_arch_hardware_enable();
+ r = static_call(kvm_x86_hardware_enable)();
if (r)
pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
cpu, (const char *)caller_name);
@@ -11882,12 +11871,8 @@ static void hardware_disable(void *junk)
if (!cpumask_test_cpu(cpu, &cpus_hardware_enabled))
return;
cpumask_clear_cpu(cpu, &cpus_hardware_enabled);
- kvm_arch_hardware_disable();
-}
-
-void kvm_arch_pre_hardware_unsetup(void)
-{
- on_each_cpu(hardware_disable, NULL, 1);
+ static_call(kvm_x86_hardware_disable)();
+ drop_user_return_notifiers();
}
/*
@@ -12019,7 +12004,7 @@ void kvm_arch_resume(int usage_count)
return;
preempt_disable();
- if (kvm_arch_hardware_enable()) {
+ if (static_call(kvm_x86_hardware_enable)()) {
preempt_enable();
return;
}
@@ -12163,6 +12148,8 @@ int kvm_arch_hardware_setup(void *opaque)
void kvm_arch_hardware_unsetup(void)
{
+ on_each_cpu(hardware_disable, NULL, 1);
+
kvm_unregister_perf_callbacks();
static_call(kvm_x86_hardware_unsetup)();
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f538fc3356a9..5f4d6f641b03 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1434,13 +1434,15 @@ 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
+#ifdef CONFIG_HAVE_KVM_OVERRIDE_HARDWARE_ENABLE
+static inline void kvm_arch_pre_hardware_unsetup(void) {}
+#else
+void kvm_arch_pre_hardware_unsetup(void);
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);
int kvm_arch_check_processor_compat(void);
int kvm_arch_check_processor_compat_all(void);
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 22/26] KVM: x86: Make x86 processor compat check callback empty
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (20 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 21/26] KVM: x86: Delete kvm_arch_hardware_enable/disable() isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup isaku.yamahata
` (3 subsequent siblings)
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Move processor compatibility check on all processors into
kvm_arch_hardware_setup() and make kvm_arch_check_processor_compat{,_all}()
empty. This is a preparation step to eliminate them.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 47 +++++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0c9d965859c6..9dd90f0521c3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -129,6 +129,8 @@ static int kvm_vcpu_do_singlestep(struct kvm_vcpu *vcpu);
static int __set_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2);
static void __get_sregs2(struct kvm_vcpu *vcpu, struct kvm_sregs2 *sregs2);
+static int kvm_check_processor_compatibility(void);
+
struct kvm_x86_ops kvm_x86_ops __read_mostly;
#define KVM_X86_OP(func) \
@@ -11911,21 +11913,8 @@ int kvm_arch_del_vm(int usage_count)
return 0;
}
-static void check_processor_compat(void *rtn)
-{
- *(int *)rtn = kvm_arch_check_processor_compat();
-}
-
int kvm_arch_check_processor_compat_all(void)
{
- int cpu;
- int r;
-
- for_each_online_cpu(cpu) {
- smp_call_function_single(cpu, check_processor_compat, &r, 1);
- if (r < 0)
- return r;
- }
return 0;
}
@@ -11933,7 +11922,7 @@ int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
{
int ret;
- ret = kvm_arch_check_processor_compat();
+ ret = kvm_check_processor_compatibility();
if (ret)
return ret;
@@ -11997,7 +11986,7 @@ void kvm_arch_resume(int usage_count)
u64 max_tsc = 0;
bool stable, backwards_tsc = false;
- if (kvm_arch_check_processor_compat())
+ if (kvm_check_processor_compatibility())
return; /* FIXME: disable KVM. */
if (!usage_count)
@@ -12103,6 +12092,24 @@ static inline void kvm_ops_update(struct kvm_x86_init_ops *ops)
kvm_pmu_ops_update(ops->pmu_ops);
}
+static void check_processor_compat(void *rtn)
+{
+ *(int *)rtn = kvm_check_processor_compatibility();
+}
+
+static int kvm_check_processor_compatibility_all(void)
+{
+ int cpu;
+ int r;
+
+ for_each_online_cpu(cpu) {
+ smp_call_function_single(cpu, check_processor_compat, &r, 1);
+ if (r < 0)
+ return r;
+ }
+ return 0;
+}
+
int kvm_arch_hardware_setup(void *opaque)
{
struct kvm_x86_init_ops *ops = opaque;
@@ -12143,7 +12150,8 @@ int kvm_arch_hardware_setup(void *opaque)
}
kvm_caps.default_tsc_scaling_ratio = 1ULL << kvm_caps.tsc_scaling_ratio_frac_bits;
kvm_init_msr_list();
- return 0;
+
+ return kvm_check_processor_compatibility_all();
}
void kvm_arch_hardware_unsetup(void)
@@ -12155,7 +12163,7 @@ void kvm_arch_hardware_unsetup(void)
static_call(kvm_x86_hardware_unsetup)();
}
-int kvm_arch_check_processor_compat(void)
+static int kvm_check_processor_compatibility(void)
{
int cpu = smp_processor_id();
struct cpuinfo_x86 *c = &cpu_data(cpu);
@@ -12175,6 +12183,11 @@ int kvm_arch_check_processor_compat(void)
return static_call(kvm_x86_check_processor_compatibility)();
}
+int kvm_arch_check_processor_compat(void)
+{
+ return 0;
+}
+
bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu)
{
return vcpu->kvm->arch.bsp_vcpu_id == vcpu->vcpu_id;
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (21 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 22/26] KVM: x86: Make x86 processor compat check callback empty isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-09 5:55 ` Christophe Leroy
2022-09-08 23:25 ` [PATCH v4 24/26] KVM: Eliminate kvm_arch_check_processor_compat() isaku.yamahata
` (2 subsequent siblings)
25 siblings, 1 reply; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen, linuxppc-dev, Fabiano Rosas
From: Isaku Yamahata <isaku.yamahata@intel.com>
Move processor compatibility check from kvm_arch_processor_compat() into
kvm_arch_hardware_setup(). The check does model name comparison with a
global variable, cur_cpu_spec. There is no point to check it at run time
on all processors.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: Fabiano Rosas <farosas@linux.ibm.com>
---
arch/powerpc/kvm/powerpc.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 7b56d6ccfdfb..7e3a6659f107 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -444,12 +444,21 @@ int kvm_arch_hardware_enable(void)
int kvm_arch_hardware_setup(void *opaque)
{
- return 0;
+ /*
+ * kvmppc_core_check_processor_compat() checks the global variable.
+ * No point to check on all processors or at runtime.
+ * arch/powerpc/kvm/book3s.c: return 0
+ * arch/powerpc/kvm/e500.c: strcmp(cur_cpu_spec->cpu_name, "e500v2")
+ * arch/powerpc/kvm/e500mc.c: strcmp(cur_cpu_spec->cpu_name, "e500mc")
+ * strcmp(cur_cpu_spec->cpu_name, "e5500")
+ * strcmp(cur_cpu_spec->cpu_name, "e6500")
+ */
+ return kvmppc_core_check_processor_compat();
}
int kvm_arch_check_processor_compat(void)
{
- return kvmppc_core_check_processor_compat();
+ return 0;
}
int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 24/26] KVM: Eliminate kvm_arch_check_processor_compat()
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (22 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 25/26] RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 26/26] RFC: KVM: " isaku.yamahata
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
Now all arch has "return 0" implementation. Eliminate it. If feature
compatibility check is needed, it should be done in
kvm_arch_hardware_setup(), kvm_arch_online_cpu(), and kvm_arch_resume().
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/arm64/kvm/arm.c | 5 -----
arch/mips/kvm/mips.c | 5 -----
arch/powerpc/kvm/powerpc.c | 5 -----
arch/riscv/kvm/main.c | 5 -----
arch/s390/kvm/kvm-s390.c | 5 -----
arch/x86/kvm/x86.c | 10 ----------
include/linux/kvm_host.h | 2 --
virt/kvm/kvm_arch.c | 29 -----------------------------
virt/kvm/kvm_main.c | 4 ----
9 files changed, 70 deletions(-)
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 7e83498b83aa..de0397bd7b18 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -68,11 +68,6 @@ int kvm_arch_hardware_setup(void *opaque)
return 0;
}
-int kvm_arch_check_processor_compat(void)
-{
- return 0;
-}
-
int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
struct kvm_enable_cap *cap)
{
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 092d09fb6a7e..f4feae89075c 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -140,11 +140,6 @@ int kvm_arch_hardware_setup(void *opaque)
return 0;
}
-int kvm_arch_check_processor_compat(void)
-{
- return 0;
-}
-
extern void kvm_init_loongson_ipi(struct kvm *kvm);
int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 7e3a6659f107..d840f6d498eb 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -456,11 +456,6 @@ int kvm_arch_hardware_setup(void *opaque)
return kvmppc_core_check_processor_compat();
}
-int kvm_arch_check_processor_compat(void)
-{
- return 0;
-}
-
int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
{
struct kvmppc_ops *kvm_ops = NULL;
diff --git a/arch/riscv/kvm/main.c b/arch/riscv/kvm/main.c
index f8d6372d208f..ebabcb3dfb8c 100644
--- a/arch/riscv/kvm/main.c
+++ b/arch/riscv/kvm/main.c
@@ -20,11 +20,6 @@ long kvm_arch_dev_ioctl(struct file *filp,
return -EINVAL;
}
-int kvm_arch_check_processor_compat(void)
-{
- return 0;
-}
-
int kvm_arch_hardware_setup(void *opaque)
{
return 0;
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index e26d4dd85668..9c5d3e4b464f 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -254,11 +254,6 @@ int kvm_arch_hardware_enable(void)
return 0;
}
-int kvm_arch_check_processor_compat(void)
-{
- return 0;
-}
-
/* forward declarations */
static void kvm_gmap_notifier(struct gmap *gmap, unsigned long start,
unsigned long end);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9dd90f0521c3..84cc459575e2 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11913,11 +11913,6 @@ int kvm_arch_del_vm(int usage_count)
return 0;
}
-int kvm_arch_check_processor_compat_all(void)
-{
- return 0;
-}
-
int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
{
int ret;
@@ -12183,11 +12178,6 @@ static int kvm_check_processor_compatibility(void)
return static_call(kvm_x86_check_processor_compatibility)();
}
-int kvm_arch_check_processor_compat(void)
-{
- return 0;
-}
-
bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu)
{
return vcpu->kvm->arch.bsp_vcpu_id == vcpu->vcpu_id;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5f4d6f641b03..2cd835c8bc1b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1444,8 +1444,6 @@ void kvm_arch_hardware_disable(void);
int kvm_arch_hardware_setup(void *opaque);
void kvm_arch_hardware_unsetup(void);
-int kvm_arch_check_processor_compat(void);
-int kvm_arch_check_processor_compat_all(void);
int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
bool kvm_arch_vcpu_in_kernel(struct kvm_vcpu *vcpu);
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 9bf7b3920c44..68fb679d71f2 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -88,32 +88,10 @@ __weak int kvm_arch_del_vm(int usage_count)
return 0;
}
-static void check_processor_compat(void *rtn)
-{
- *(int *)rtn = kvm_arch_check_processor_compat();
-}
-
-__weak int kvm_arch_check_processor_compat_all(void)
-{
- int cpu;
- int r;
-
- for_each_online_cpu(cpu) {
- smp_call_function_single(cpu, check_processor_compat, &r, 1);
- if (r < 0)
- return r;
- }
- return 0;
-}
-
__weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
{
int ret;
- ret = kvm_arch_check_processor_compat();
- if (ret)
- return ret;
-
if (!usage_count)
return 0;
@@ -167,13 +145,6 @@ __weak int kvm_arch_suspend(int usage_count)
__weak void kvm_arch_resume(int usage_count)
{
- if (kvm_arch_check_processor_compat())
- /*
- * No warning here because kvm_arch_check_processor_compat()
- * would have warned with more information.
- */
- return; /* FIXME: disable KVM */
-
if (usage_count) {
preempt_disable();
(void)__hardware_enable((void *)__func__);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 51315d454dc2..3f82162c8441 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5777,10 +5777,6 @@ int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
if (r < 0)
goto out_free_1;
- r = kvm_arch_check_processor_compat_all();
- if (r < 0)
- goto out_free_2;
-
r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_ONLINE, "kvm/cpu:online",
kvm_online_cpu, kvm_offline_cpu);
if (r)
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 25/26] RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (23 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 24/26] KVM: Eliminate kvm_arch_check_processor_compat() isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 26/26] RFC: KVM: " isaku.yamahata
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
cpus_hardware_enabled mask seems incomplete protection against other kernel
component using CPU virtualization feature. Because it's obscure and
incomplete, remove the check.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
arch/x86/kvm/x86.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 84cc459575e2..623a5993d608 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -11836,23 +11836,16 @@ void kvm_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector)
}
EXPORT_SYMBOL_GPL(kvm_vcpu_deliver_sipi_vector);
-static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
-
static int __hardware_enable(void *caller_name)
{
- int cpu = raw_smp_processor_id();
int r;
WARN_ON_ONCE(preemptible());
- if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
- return 0;
r = static_call(kvm_x86_hardware_enable)();
if (r)
pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
- cpu, (const char *)caller_name);
- else
- cpumask_set_cpu(cpu, &cpus_hardware_enabled);
+ smp_processor_id(), (const char *)caller_name);
return r;
}
@@ -11866,13 +11859,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);
static_call(kvm_x86_hardware_disable)();
drop_user_return_notifiers();
}
--
2.25.1
^ permalink raw reply related [flat|nested] 44+ messages in thread
* [PATCH v4 26/26] RFC: KVM: Remove cpus_hardware_enabled and related sanity check
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
` (24 preceding siblings ...)
2022-09-08 23:25 ` [PATCH v4 25/26] RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check isaku.yamahata
@ 2022-09-08 23:25 ` isaku.yamahata
25 siblings, 0 replies; 44+ messages in thread
From: isaku.yamahata @ 2022-09-08 23:25 UTC (permalink / raw)
To: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao
Cc: isaku.yamahata, isaku.yamahata, Kai Huang, Chao Gao, Atish Patra,
Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano, Huang Ying,
Huacai Chen
From: Isaku Yamahata <isaku.yamahata@intel.com>
cpus_hardware_enabled mask seems incomplete protection against other kernel
component using CPU virtualization feature. Because it's obscure and
incomplete, remove the check.
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
---
virt/kvm/kvm_arch.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
index 68fb679d71f2..383b0065ecb0 100644
--- a/virt/kvm/kvm_arch.c
+++ b/virt/kvm/kvm_arch.c
@@ -12,23 +12,16 @@
#include <linux/kvm_host.h>
-static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
-
static int __hardware_enable(void *caller_name)
{
- 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_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
- cpu, (const char *)caller_name);
- else
- cpumask_set_cpu(cpu, &cpus_hardware_enabled);
+ smp_processor_id(), (const char *)caller_name);
return r;
}
@@ -42,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
^ permalink raw reply related [flat|nested] 44+ messages in thread
* Re: [PATCH v4 09/26] KVM: Do processor compatibility check on resume
2022-09-08 23:25 ` [PATCH v4 09/26] KVM: Do processor compatibility check on resume isaku.yamahata
@ 2022-09-09 2:15 ` Chao Gao
2022-09-10 17:59 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Chao Gao @ 2022-09-09 2:15 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:25PM -0700, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>So far the processor compatibility check is not done on resume. It should
>be done. The resume is called for resuming from S3/S4. CPUs can be
>replaced or the kernel can resume from S4 on a different machine. So
Are they valid cases handled by other kernel components? Trying to
handle them in KVM only doesn't help as kernel is likely to fail
somewhere else.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock
2022-09-08 23:25 ` [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock isaku.yamahata
@ 2022-09-09 3:05 ` Chao Gao
2022-09-10 18:11 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Chao Gao @ 2022-09-09 3:05 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:26PM -0700, isaku.yamahata@intel.com wrote:
>-
>-``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.
Probably "/wrt CPU hotplug" is better.
>
>@@ -5708,8 +5728,18 @@ static void kvm_init_debug(void)
>
> static int kvm_suspend(void)
> {
>- if (kvm_usage_count)
>+ /*
>+ * The caller ensures that CPU hotlug is disabled by
^hotplug
>+ * cpu_hotplug_disable() and other CPUs are offlined. No need for
>+ * locking.
>+ */
>+ lockdep_assert_not_held(&kvm_lock);
>+
>+ if (kvm_usage_count) {
>+ preempt_disable();
> hardware_disable_nolock(NULL);
>+ preempt_enable();
kvm_suspend() is called with interrupt disabled. So, no need to disable
preemption.
/**
* syscore_suspend - Execute all the registered system core suspend callbacks.
*
* This function is executed with one CPU on-line and disabled interrupts.
*/
int syscore_suspend(void)
>+ }
> return 0;
> }
>
>@@ -5723,8 +5753,10 @@ static void kvm_resume(void)
> return; /* FIXME: disable KVM */
>
> if (kvm_usage_count) {
>- lockdep_assert_not_held(&kvm_count_lock);
>+ lockdep_assert_not_held(&kvm_lock);
>+ preempt_disable();
> hardware_enable_nolock((void *)__func__);
>+ preempt_enable();
ditto.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub
2022-09-08 23:25 ` [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub isaku.yamahata
@ 2022-09-09 4:34 ` Chao Gao
2022-09-11 2:39 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Chao Gao @ 2022-09-09 4:34 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:27PM -0700, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>Add arch hooks for reboot, suspend, resume, and CPU-online/offline events
>with empty stub functions.
>
>Suggested-by: Sean Christopherson <seanjc@google.com>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
>---
> include/linux/kvm_host.h | 6 +++++
> virt/kvm/Makefile.kvm | 2 +-
> virt/kvm/kvm_arch.c | 44 ++++++++++++++++++++++++++++++
> virt/kvm/kvm_main.c | 58 +++++++++++++++++++++++++---------------
> 4 files changed, 88 insertions(+), 22 deletions(-)
> create mode 100644 virt/kvm/kvm_arch.c
>
>diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>index eab352902de7..dd2a6d98d4de 100644
>--- a/include/linux/kvm_host.h
>+++ b/include/linux/kvm_host.h
>@@ -1448,6 +1448,12 @@ int kvm_arch_post_init_vm(struct kvm *kvm);
> void kvm_arch_pre_destroy_vm(struct kvm *kvm);
> int kvm_arch_create_vm_debugfs(struct kvm *kvm);
>
>+int kvm_arch_suspend(int usage_count);
>+void kvm_arch_resume(int usage_count);
>+int kvm_arch_reboot(int val);
>+int kvm_arch_online_cpu(unsigned int cpu, int usage_count);
>+int kvm_arch_offline_cpu(unsigned int cpu, int usage_count);
Why not extract each of them with one separate patch?
>+
> #ifndef __KVM_HAVE_ARCH_VM_ALLOC
> /*
> * All architectures that want to use vzalloc currently also
>diff --git a/virt/kvm/Makefile.kvm b/virt/kvm/Makefile.kvm
>index 2c27d5d0c367..c4210acabd35 100644
>--- a/virt/kvm/Makefile.kvm
>+++ b/virt/kvm/Makefile.kvm
>@@ -5,7 +5,7 @@
>
> KVM ?= ../../../virt/kvm
>
>-kvm-y := $(KVM)/kvm_main.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
>+kvm-y := $(KVM)/kvm_main.o $(KVM)/kvm_arch.o $(KVM)/eventfd.o $(KVM)/binary_stats.o
> kvm-$(CONFIG_KVM_VFIO) += $(KVM)/vfio.o
> kvm-$(CONFIG_KVM_MMIO) += $(KVM)/coalesced_mmio.o
> kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
>diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
>new file mode 100644
>index 000000000000..4748a76bcb03
>--- /dev/null
>+++ b/virt/kvm/kvm_arch.c
>@@ -0,0 +1,44 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * kvm_arch.c: kvm default arch hooks for hardware enabling/disabling
>+ * Copyright (c) 2022 Intel Corporation.
>+ *
>+ * Author:
>+ * Isaku Yamahata <isaku.yamahata@intel.com>
>+ * <isaku.yamahata@gmail.com>
>+ */
>+
>+#include <linux/kvm_host.h>
>+
>+/*
>+ * Called after the VM is otherwise initialized, but just before adding it to
>+ * the vm_list.
>+ */
>+__weak int kvm_arch_post_init_vm(struct kvm *kvm)
>+{
>+ return 0;
>+}
use "int __weak" to comply with kernel's convension.
>
> static int kvm_offline_cpu(unsigned int cpu)
> {
>+ int ret = 0;
>+
> mutex_lock(&kvm_lock);
> if (kvm_usage_count) {
> /*
>@@ -5069,10 +5067,15 @@ static int kvm_offline_cpu(unsigned int cpu)
> */
> preempt_disable();
> hardware_disable_nolock(NULL);
>+ ret = kvm_arch_offline_cpu(cpu, kvm_usage_count);
>+ if (ret) {
>+ (void)hardware_enable_nolock(NULL);
>+ atomic_set(&hardware_enable_failed, 0);
The error-handling code ignores hardware enabling failure which looks
weird to me. If you extract kvm_arch_offline_cpu() directly like what
you do in patch 14 (rather than add a stub function first and then move
some code to the stub function), the error-handling code isn't needed.
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback
2022-09-08 23:25 ` [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback isaku.yamahata
@ 2022-09-09 5:48 ` Chao Gao
2022-09-11 2:45 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Chao Gao @ 2022-09-09 5:48 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:28PM -0700, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>commit 0dd6a6edb012 ("KVM: Dont mark TSC unstable due to S4 suspend") made
>use of kvm_arch_hardware_enable() callback to detect that TSC goes backward
>due to S4 suspend. It has to check it only when resuming from S4. Not
>every time virtualization hardware ennoblement. Move the logic to
>kvm_arch_resume() callback.
IIUC, kvm_arch_resume() is called on the first CPU waking up from suspension.
But the detection was done on every CPU. Is it a problem (i.e., we fail to
detect TSC goes backward on some CPUs)?
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup
2022-09-08 23:25 ` [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup isaku.yamahata
@ 2022-09-09 5:55 ` Christophe Leroy
2022-09-10 1:39 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Christophe Leroy @ 2022-09-09 5:55 UTC (permalink / raw)
To: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao
Cc: Huang Ying, Daniel Lezcano, Fabiano Rosas, linuxppc-dev,
John Garry, Shaokun Zhang, Kai Huang, Atish Patra, Huacai Chen,
Qi Liu, isaku.yamahata, Chao Gao
Le 09/09/2022 à 01:25, isaku.yamahata@intel.com a écrit :
> [Vous ne recevez pas souvent de courriers de isaku.yamahata@intel.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Isaku Yamahata <isaku.yamahata@intel.com>
>
> Move processor compatibility check from kvm_arch_processor_compat() into
> kvm_arch_hardware_setup(). The check does model name comparison with a
> global variable, cur_cpu_spec. There is no point to check it at run time
> on all processors.
>
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> arch/powerpc/kvm/powerpc.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 7b56d6ccfdfb..7e3a6659f107 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -444,12 +444,21 @@ int kvm_arch_hardware_enable(void)
>
> int kvm_arch_hardware_setup(void *opaque)
> {
> - return 0;
> + /*
> + * kvmppc_core_check_processor_compat() checks the global variable.
> + * No point to check on all processors or at runtime.
> + * arch/powerpc/kvm/book3s.c: return 0
> + * arch/powerpc/kvm/e500.c: strcmp(cur_cpu_spec->cpu_name, "e500v2")
> + * arch/powerpc/kvm/e500mc.c: strcmp(cur_cpu_spec->cpu_name, "e500mc")
> + * strcmp(cur_cpu_spec->cpu_name, "e5500")
> + * strcmp(cur_cpu_spec->cpu_name, "e6500")
> + */
This explanation shouldn't be in the code. The content of other file may
change in the future, the files may be renamed or deleted, new files may
be added. And there is no added value with that comment.
That detailed explanation should go in the commit message.
> + return kvmppc_core_check_processor_compat();
> }
>
> int kvm_arch_check_processor_compat(void)
> {
> - return kvmppc_core_check_processor_compat();
> + return 0;
> }
>
> int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted
2022-09-08 23:25 ` [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted isaku.yamahata
@ 2022-09-09 6:08 ` Chao Gao
2022-09-11 2:50 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Chao Gao @ 2022-09-09 6:08 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:29PM -0700, isaku.yamahata@intel.com wrote:
>+__weak int kvm_arch_post_init_vm(struct kvm *kvm)
int __weak
>+{
>+ return 0;
>+}
>+
> /*
> * Called after the VM is otherwise initialized, but just before adding it to
> * the vm_list.
> */
>-__weak int kvm_arch_post_init_vm(struct kvm *kvm)
>+__weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
>+{
>+ return kvm_arch_post_init_vm(kvm);
>+}
>+
>+__weak int kvm_arch_del_vm(int usage_count)
ditto
>+static void kvm_del_vm(void)
>+{
>+ cpus_read_lock();
>+ mutex_lock(&kvm_lock);
>+ kvm_arch_del_vm(kvm_usage_count);
>+ mutex_unlock(&kvm_lock);
>+ cpus_read_unlock();
kvm_arch_del_vm() is an empty function now. I don't understand why
cpus_read_lock() is needed. Maybe add the lock when you fill up
kvm_arch_del_vm().
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 15/26] KVM: kvm_arch.c: Remove _nolock post fix
2022-09-08 23:25 ` [PATCH v4 15/26] KVM: kvm_arch.c: Remove _nolock post fix isaku.yamahata
@ 2022-09-09 6:30 ` Chao Gao
0 siblings, 0 replies; 44+ messages in thread
From: Chao Gao @ 2022-09-09 6:30 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:31PM -0700, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>Now all related callbacks are called under kvm_lock, no point for _nolock
>post fix. Remove _nolock post fix for readability with shorter function
>names.
>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed
2022-09-08 23:25 ` [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed isaku.yamahata
@ 2022-09-09 6:54 ` Chao Gao
2022-09-11 3:00 ` Isaku Yamahata
0 siblings, 1 reply; 44+ messages in thread
From: Chao Gao @ 2022-09-09 6:54 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:32PM -0700, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>A global variable hardware_enable_failed in kvm_arch.c is used only by
>kvm_arch_add_vm() and hardware_enable(). It doesn't have to be a global
>variable. Make it function local.
>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
>---
> virt/kvm/kvm_arch.c | 62 ++++++++++++++++++++++-----------------------
> 1 file changed, 31 insertions(+), 31 deletions(-)
>
>diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
>index 4fe16e8ef2e5..ad23537ebe3b 100644
>--- a/virt/kvm/kvm_arch.c
>+++ b/virt/kvm/kvm_arch.c
>@@ -13,14 +13,13 @@
> #include <linux/kvm_host.h>
>
> static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
>-static atomic_t hardware_enable_failed;
>
> __weak int kvm_arch_post_init_vm(struct kvm *kvm)
> {
> return 0;
> }
>
>-static void hardware_enable(void *caller_name)
>+static int __hardware_enable(void *caller_name)
nit: caller_name can be dropped and use __builtin_return_address(0) instead.
> {
> int cpu = raw_smp_processor_id();
> int r;
>@@ -28,18 +27,22 @@ static void hardware_enable(void *caller_name)
> WARN_ON_ONCE(preemptible());
>
> if (cpumask_test_cpu(cpu, &cpus_hardware_enabled))
>- return;
>-
>- cpumask_set_cpu(cpu, &cpus_hardware_enabled);
>-
>+ return 0;
> r = kvm_arch_hardware_enable();
>-
>- if (r) {
>- cpumask_clear_cpu(cpu, &cpus_hardware_enabled);
>- atomic_inc(&hardware_enable_failed);
>+ if (r)
> pr_warn("kvm: enabling virtualization on CPU%d failed during %s()\n",
> cpu, (const char *)caller_name);
>- }
>+ else
>+ cpumask_set_cpu(cpu, &cpus_hardware_enabled);
>+ return r;
>+}
>+
>+static void hardware_enable(void *arg)
>+{
>+ atomic_t *failed = arg;
>+
>+ if (__hardware_enable((void *)__func__))
>+ atomic_inc(failed);
> }
>
> static void hardware_disable(void *junk)
>@@ -65,15 +68,16 @@ __weak void kvm_arch_pre_hardware_unsetup(void)
> */
> __weak int kvm_arch_add_vm(struct kvm *kvm, int usage_count)
> {
>+ atomic_t failed;
nit:
atomic_t failed = ATOMIC_INIT(0);
then you can drop the atomic_set() below.
> int r = 0;
>
> if (usage_count != 1)
> return 0;
>
>- atomic_set(&hardware_enable_failed, 0);
>- on_each_cpu(hardware_enable, (void *)__func__, 1);
>+ atomic_set(&failed, 0);
>+ on_each_cpu(hardware_enable, &failed, 1);
>
>- if (atomic_read(&hardware_enable_failed)) {
>+ if (atomic_read(&failed)) {
> r = -EBUSY;
> goto err;
> }
>@@ -96,33 +100,29 @@ __weak int kvm_arch_del_vm(int usage_count)
>
> __weak int kvm_arch_online_cpu(unsigned int cpu, int usage_count)
> {
>- int ret = 0;
>+ int ret;
>
> ret = kvm_arch_check_processor_compat();
> if (ret)
> return ret;
>
>+ if (!usage_count)
>+ return 0;
>+
>+ /*
>+ * arch callback kvm_arch_hardware_eanble() assumes that
^ enable
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 17/26] KVM: Introduce a arch wrapper to check all processor compatibility
2022-09-08 23:25 ` [PATCH v4 17/26] KVM: Introduce a arch wrapper to check all processor compatibility isaku.yamahata
@ 2022-09-09 7:32 ` Chao Gao
0 siblings, 0 replies; 44+ messages in thread
From: Chao Gao @ 2022-09-09 7:32 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Marc Zyngier, Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Thu, Sep 08, 2022 at 04:25:33PM -0700, isaku.yamahata@intel.com wrote:
>From: Isaku Yamahata <isaku.yamahata@intel.com>
>
>Introduce a arch wrapper to check all processor compatibility and define
>default implementation as weak symbol to keep the current logic.
>
>The hardware feature compatibility check is arch dependent, only x86 KVM
>does cpu feature check on all processors. It doesn't make much sense to
>enforce the current implementation to invoke check function on each
>processors. Introduce a arch callback,
>kvm_arch_check_processor_compat_all(), so that arch code can override it.
>
>Eventually feature check should be pushed down into arch callback,
>(kvm_arch_hardware_setup(), kvm_arch_online_cpu(), and kvm_arch_resume()),
>the two compatibility check, kvm_arch_check_processor_compat{,_all}(), will
>be eliminated. This is a transitional step for it.
>
>Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Reviewed-by: Chao Gao <chao.gao@intel.com>
>+__weak int kvm_arch_check_processor_compat_all(void)
nit: int __weak
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup
2022-09-09 5:55 ` Christophe Leroy
@ 2022-09-10 1:39 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-10 1:39 UTC (permalink / raw)
To: Christophe Leroy
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
Huang Ying, Daniel Lezcano, Fabiano Rosas, linuxppc-dev,
John Garry, Shaokun Zhang, Kai Huang, Atish Patra, Huacai Chen,
Qi Liu, isaku.yamahata, Chao Gao
On Fri, Sep 09, 2022 at 05:55:14AM +0000,
Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
>
>
> Le 09/09/2022 à 01:25, isaku.yamahata@intel.com a écrit :
> > [Vous ne recevez pas souvent de courriers de isaku.yamahata@intel.com. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >
> > From: Isaku Yamahata <isaku.yamahata@intel.com>
> >
> > Move processor compatibility check from kvm_arch_processor_compat() into
> > kvm_arch_hardware_setup(). The check does model name comparison with a
> > global variable, cur_cpu_spec. There is no point to check it at run time
> > on all processors.
> >
> > Suggested-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> > arch/powerpc/kvm/powerpc.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index 7b56d6ccfdfb..7e3a6659f107 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -444,12 +444,21 @@ int kvm_arch_hardware_enable(void)
> >
> > int kvm_arch_hardware_setup(void *opaque)
> > {
> > - return 0;
> > + /*
> > + * kvmppc_core_check_processor_compat() checks the global variable.
> > + * No point to check on all processors or at runtime.
> > + * arch/powerpc/kvm/book3s.c: return 0
> > + * arch/powerpc/kvm/e500.c: strcmp(cur_cpu_spec->cpu_name, "e500v2")
> > + * arch/powerpc/kvm/e500mc.c: strcmp(cur_cpu_spec->cpu_name, "e500mc")
> > + * strcmp(cur_cpu_spec->cpu_name, "e5500")
> > + * strcmp(cur_cpu_spec->cpu_name, "e6500")
> > + */
>
> This explanation shouldn't be in the code. The content of other file may
> change in the future, the files may be renamed or deleted, new files may
> be added. And there is no added value with that comment.
>
> That detailed explanation should go in the commit message.
Ok, will move the comment into the commit message.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 09/26] KVM: Do processor compatibility check on resume
2022-09-09 2:15 ` Chao Gao
@ 2022-09-10 17:59 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-10 17:59 UTC (permalink / raw)
To: Chao Gao
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
isaku.yamahata, Kai Huang, Atish Patra, Shaokun Zhang, Qi Liu,
John Garry, Daniel Lezcano, Huang Ying, Huacai Chen
On Fri, Sep 09, 2022 at 10:15:16AM +0800,
Chao Gao <chao.gao@intel.com> wrote:
> On Thu, Sep 08, 2022 at 04:25:25PM -0700, isaku.yamahata@intel.com wrote:
> >From: Isaku Yamahata <isaku.yamahata@intel.com>
> >
> >So far the processor compatibility check is not done on resume. It should
> >be done. The resume is called for resuming from S3/S4. CPUs can be
> >replaced or the kernel can resume from S4 on a different machine. So
>
> Are they valid cases handled by other kernel components? Trying to
> handle them in KVM only doesn't help as kernel is likely to fail
> somewhere else.
I guess no. Actually without the compatibility check, kvm_resume() tries to
enable it by calling hardware_enable_nolock(). The real effect on error case of
this patch is to print out (potentially) more helpful message gracefully by
check_processor_compatibility instead of error message of haredware_enable.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock
2022-09-09 3:05 ` Chao Gao
@ 2022-09-10 18:11 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-10 18:11 UTC (permalink / raw)
To: Chao Gao
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
isaku.yamahata, Kai Huang, Atish Patra, Shaokun Zhang, Qi Liu,
John Garry, Daniel Lezcano, Huang Ying, Huacai Chen
On Fri, Sep 09, 2022 at 11:05:34AM +0800,
Chao Gao <chao.gao@intel.com> wrote:
> >+ * cpu_hotplug_disable() and other CPUs are offlined. No need for
> >+ * locking.
> >+ */
> >+ lockdep_assert_not_held(&kvm_lock);
> >+
> >+ if (kvm_usage_count) {
> >+ preempt_disable();
> > hardware_disable_nolock(NULL);
> >+ preempt_enable();
>
> kvm_suspend() is called with interrupt disabled. So, no need to disable
> preemption.
>
> /**
> * syscore_suspend - Execute all the registered system core suspend callbacks.
> *
> * This function is executed with one CPU on-line and disabled interrupts.
> */
> int syscore_suspend(void)
Thanks, I'll fix it with a comment.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub
2022-09-09 4:34 ` Chao Gao
@ 2022-09-11 2:39 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-11 2:39 UTC (permalink / raw)
To: Chao Gao
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
isaku.yamahata, Kai Huang, Atish Patra, Shaokun Zhang, Qi Liu,
John Garry, Daniel Lezcano, Huang Ying, Huacai Chen
On Fri, Sep 09, 2022 at 12:34:39PM +0800,
Chao Gao <chao.gao@intel.com> wrote:
> On Thu, Sep 08, 2022 at 04:25:27PM -0700, isaku.yamahata@intel.com wrote:
> >From: Isaku Yamahata <isaku.yamahata@intel.com>
> >
> >Add arch hooks for reboot, suspend, resume, and CPU-online/offline events
> >with empty stub functions.
> >
> >Suggested-by: Sean Christopherson <seanjc@google.com>
> >Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> >---
> > include/linux/kvm_host.h | 6 +++++
> > virt/kvm/Makefile.kvm | 2 +-
> > virt/kvm/kvm_arch.c | 44 ++++++++++++++++++++++++++++++
> > virt/kvm/kvm_main.c | 58 +++++++++++++++++++++++++---------------
> > 4 files changed, 88 insertions(+), 22 deletions(-)
> > create mode 100644 virt/kvm/kvm_arch.c
> >
> >diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> >index eab352902de7..dd2a6d98d4de 100644
> >--- a/include/linux/kvm_host.h
> >+++ b/include/linux/kvm_host.h
> >@@ -1448,6 +1448,12 @@ int kvm_arch_post_init_vm(struct kvm *kvm);
> > void kvm_arch_pre_destroy_vm(struct kvm *kvm);
> > int kvm_arch_create_vm_debugfs(struct kvm *kvm);
> >
> >+int kvm_arch_suspend(int usage_count);
> >+void kvm_arch_resume(int usage_count);
> >+int kvm_arch_reboot(int val);
> >+int kvm_arch_online_cpu(unsigned int cpu, int usage_count);
> >+int kvm_arch_offline_cpu(unsigned int cpu, int usage_count);
>
> Why not extract each of them with one separate patch?
Do you mean one patch for each arch callback? They are convoluted.
See the comment below.
> >diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
> >new file mode 100644
> >index 000000000000..4748a76bcb03
> >--- /dev/null
> >+++ b/virt/kvm/kvm_arch.c
> >@@ -0,0 +1,44 @@
> >+// SPDX-License-Identifier: GPL-2.0-only
> >+/*
> >+ * kvm_arch.c: kvm default arch hooks for hardware enabling/disabling
> >+ * Copyright (c) 2022 Intel Corporation.
> >+ *
> >+ * Author:
> >+ * Isaku Yamahata <isaku.yamahata@intel.com>
> >+ * <isaku.yamahata@gmail.com>
> >+ */
> >+
> >+#include <linux/kvm_host.h>
> >+
> >+/*
> >+ * Called after the VM is otherwise initialized, but just before adding it to
> >+ * the vm_list.
> >+ */
> >+__weak int kvm_arch_post_init_vm(struct kvm *kvm)
> >+{
> >+ return 0;
> >+}
>
> use "int __weak" to comply with kernel's convension.
Will fix.
> > static int kvm_offline_cpu(unsigned int cpu)
> > {
> >+ int ret = 0;
> >+
> > mutex_lock(&kvm_lock);
> > if (kvm_usage_count) {
> > /*
> >@@ -5069,10 +5067,15 @@ static int kvm_offline_cpu(unsigned int cpu)
> > */
> > preempt_disable();
> > hardware_disable_nolock(NULL);
> >+ ret = kvm_arch_offline_cpu(cpu, kvm_usage_count);
> >+ if (ret) {
> >+ (void)hardware_enable_nolock(NULL);
> >+ atomic_set(&hardware_enable_failed, 0);
>
> The error-handling code ignores hardware enabling failure which looks
> weird to me. If you extract kvm_arch_offline_cpu() directly like what
> you do in patch 14 (rather than add a stub function first and then move
> some code to the stub function), the error-handling code isn't needed.
I did it for x86 tsc fix. It relates to suspend/resume. I would split those
- introduce suspend/resuem/reboot arch hooks
- fix x86 tsc issue
- move
- introduce cpu online/offline arch hooks
- move out PM hooks. probably this can be combined into the previous one.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback
2022-09-09 5:48 ` Chao Gao
@ 2022-09-11 2:45 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-11 2:45 UTC (permalink / raw)
To: Chao Gao
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
isaku.yamahata, Kai Huang, Atish Patra, Shaokun Zhang, Qi Liu,
John Garry, Daniel Lezcano, Huang Ying, Huacai Chen
On Fri, Sep 09, 2022 at 01:48:32PM +0800,
Chao Gao <chao.gao@intel.com> wrote:
> On Thu, Sep 08, 2022 at 04:25:28PM -0700, isaku.yamahata@intel.com wrote:
> >From: Isaku Yamahata <isaku.yamahata@intel.com>
> >
> >commit 0dd6a6edb012 ("KVM: Dont mark TSC unstable due to S4 suspend") made
> >use of kvm_arch_hardware_enable() callback to detect that TSC goes backward
> >due to S4 suspend. It has to check it only when resuming from S4. Not
> >every time virtualization hardware ennoblement. Move the logic to
> >kvm_arch_resume() callback.
>
> IIUC, kvm_arch_resume() is called on the first CPU waking up from suspension.
> But the detection was done on every CPU. Is it a problem (i.e., we fail to
> detect TSC goes backward on some CPUs)?
The problem is, TSC fixup logic is only needed once on resuming.
The current code calls TSC fixup logic on each cpu onlining. It's quick
plumbing the logic to each cpu online. Although it won't harm to call the logic,
it's ugly and this time is good occasion to clean it up.
I will clarify the commit message.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted
2022-09-09 6:08 ` Chao Gao
@ 2022-09-11 2:50 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-11 2:50 UTC (permalink / raw)
To: Chao Gao
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
isaku.yamahata, Kai Huang, Atish Patra, Shaokun Zhang, Qi Liu,
John Garry, Daniel Lezcano, Huang Ying, Huacai Chen
On Fri, Sep 09, 2022 at 02:08:04PM +0800,
Chao Gao <chao.gao@intel.com> wrote:
> >+static void kvm_del_vm(void)
> >+{
> >+ cpus_read_lock();
> >+ mutex_lock(&kvm_lock);
> >+ kvm_arch_del_vm(kvm_usage_count);
> >+ mutex_unlock(&kvm_lock);
> >+ cpus_read_unlock();
>
> kvm_arch_del_vm() is an empty function now. I don't understand why
> cpus_read_lock() is needed. Maybe add the lock when you fill up
> kvm_arch_del_vm().
The next patch populates it with on_each_cpu(hardware_disable_nolock, NULL, 1);
I will move adding lock to the next one. Or revise how adding arch hooks and
populating part.
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed
2022-09-09 6:54 ` Chao Gao
@ 2022-09-11 3:00 ` Isaku Yamahata
0 siblings, 0 replies; 44+ messages in thread
From: Isaku Yamahata @ 2022-09-11 3:00 UTC (permalink / raw)
To: Chao Gao
Cc: isaku.yamahata, linux-kernel, kvm, Paolo Bonzini,
Sean Christopherson, Marc Zyngier, Will Deacon, Yuan Yao,
isaku.yamahata, Kai Huang, Atish Patra, Shaokun Zhang, Qi Liu,
John Garry, Daniel Lezcano, Huang Ying, Huacai Chen
On Fri, Sep 09, 2022 at 02:54:05PM +0800,
Chao Gao <chao.gao@intel.com> wrote:
> >diff --git a/virt/kvm/kvm_arch.c b/virt/kvm/kvm_arch.c
> >index 4fe16e8ef2e5..ad23537ebe3b 100644
> >--- a/virt/kvm/kvm_arch.c
> >+++ b/virt/kvm/kvm_arch.c
> >@@ -13,14 +13,13 @@
> > #include <linux/kvm_host.h>
> >
> > static cpumask_t cpus_hardware_enabled = CPU_MASK_NONE;
> >-static atomic_t hardware_enable_failed;
> >
> > __weak int kvm_arch_post_init_vm(struct kvm *kvm)
> > {
> > return 0;
> > }
> >
> >-static void hardware_enable(void *caller_name)
> >+static int __hardware_enable(void *caller_name)
>
> nit: caller_name can be dropped and use __builtin_return_address(0) instead.
Do you want to update your patch
"KVM: Provide more information in kernel log if hardware enabling fails"?
--
Isaku Yamahata <isaku.yamahata@gmail.com>
^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH v4 06/26] KVM: arm64: Simplify the CPUHP logic
2022-09-08 23:25 ` [PATCH v4 06/26] KVM: arm64: Simplify the CPUHP logic isaku.yamahata
@ 2022-09-12 15:07 ` Marc Zyngier
0 siblings, 0 replies; 44+ messages in thread
From: Marc Zyngier @ 2022-09-12 15:07 UTC (permalink / raw)
To: isaku.yamahata
Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson,
Will Deacon, Yuan Yao, isaku.yamahata, Kai Huang, Chao Gao,
Atish Patra, Shaokun Zhang, Qi Liu, John Garry, Daniel Lezcano,
Huang Ying, Huacai Chen
On Fri, 09 Sep 2022 00:25:22 +0100,
isaku.yamahata@intel.com wrote:
>
> From: Marc Zyngier <maz@kernel.org>
>
> For a number of historical reasons, the KVM/arm64 hotplug setup is pretty
> complicated, and we have two extra CPUHP notifiers for vGIC and timers.
>
> It looks pretty pointless, and gets in the way of further changes.
> So let's just expose some helpers that can be called from the core
> CPUHP callback, and get rid of everything else.
>
> This gives us the opportunity to drop a useless notifier entry,
> as well as tidy-up the timer enable/disable, which was a bit odd.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Chao Gao <chao.gao@intel.com>
> Link: https://lore.kernel.org/r/20220216031528.92558-5-chao.gao@intel.com
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Since this patch is substantially different from the version pointed
to on the list, please drop Chao's SoB and the Link: tag, as they are
not relevant anymore.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2022-09-12 15:07 UTC | newest]
Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-08 23:25 [PATCH v4 00/26] KVM: hardware enable/disable reorganize isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 01/26] KVM: x86: Drop kvm_user_return_msr_cpu_online() isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 02/26] KVM: x86: Use this_cpu_ptr() instead of per_cpu_ptr(smp_processor_id()) isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 03/26] KVM: x86: Move check_processor_compatibility from init ops to runtime ops isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 04/26] Partially revert "KVM: Pass kvm_init()'s opaque param to additional arch funcs" isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 05/26] KVM: Provide more information in kernel log if hardware enabling fails isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 06/26] KVM: arm64: Simplify the CPUHP logic isaku.yamahata
2022-09-12 15:07 ` Marc Zyngier
2022-09-08 23:25 ` [PATCH v4 07/26] KVM: Rename and move CPUHP_AP_KVM_STARTING to ONLINE section isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 08/26] KVM: Do compatibility checks on hotplugged CPUs isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 09/26] KVM: Do processor compatibility check on resume isaku.yamahata
2022-09-09 2:15 ` Chao Gao
2022-09-10 17:59 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 10/26] KVM: Drop kvm_count_lock and instead protect kvm_usage_count with kvm_lock isaku.yamahata
2022-09-09 3:05 ` Chao Gao
2022-09-10 18:11 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 11/26] KVM: Add arch hooks for PM events with empty stub isaku.yamahata
2022-09-09 4:34 ` Chao Gao
2022-09-11 2:39 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 12/26] KVM: x86: Move TSC fixup logic to KVM arch resume callback isaku.yamahata
2022-09-09 5:48 ` Chao Gao
2022-09-11 2:45 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 13/26] KVM: Add arch hook when VM is added/deleted isaku.yamahata
2022-09-09 6:08 ` Chao Gao
2022-09-11 2:50 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 14/26] KVM: Move out KVM arch PM hooks and hardware enable/disable logic isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 15/26] KVM: kvm_arch.c: Remove _nolock post fix isaku.yamahata
2022-09-09 6:30 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 16/26] KVM: kvm_arch.c: Remove a global variable, hardware_enable_failed isaku.yamahata
2022-09-09 6:54 ` Chao Gao
2022-09-11 3:00 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 17/26] KVM: Introduce a arch wrapper to check all processor compatibility isaku.yamahata
2022-09-09 7:32 ` Chao Gao
2022-09-08 23:25 ` [PATCH v4 18/26] KVM: x86: Duplicate arch callbacks related to pm events and compat check isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 19/26] KVM: Eliminate kvm_arch_post_init_vm() isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 20/26] KVM: Add config to not compile kvm_arch.c isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 21/26] KVM: x86: Delete kvm_arch_hardware_enable/disable() isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 22/26] KVM: x86: Make x86 processor compat check callback empty isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup isaku.yamahata
2022-09-09 5:55 ` Christophe Leroy
2022-09-10 1:39 ` Isaku Yamahata
2022-09-08 23:25 ` [PATCH v4 24/26] KVM: Eliminate kvm_arch_check_processor_compat() isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 25/26] RFC: KVM: x86: Remove cpus_hardware_enabled and related sanity check isaku.yamahata
2022-09-08 23:25 ` [PATCH v4 26/26] RFC: KVM: " isaku.yamahata
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).