On Mon, May 23, 2022 at 03:27:10PM -0700,
Sagi Shahar <sagis@xxxxxxxxxx> wrote:
> On Thu, May 5, 2022 at 11:15 AM <isaku.yamahata@xxxxxxxxx> wrote:
> >
> > From: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> >
> > Although non-x86 arch doesn't break as long as I inspected code, it's by
> > code inspection. This should be reviewed by each arch maintainers.
> >
> > kvm_init() checks CPU compatibility by calling
> > kvm_arch_check_processor_compat() on all online CPUs. Move the callback
> > to hardware_enable_nolock() and add hardware_enable_all() and
> > hardware_disable_all().
> > Add arch specific callback kvm_arch_post_hardware_enable_setup() for arch
> > to do arch specific initialization that required hardware_enable_all().
>
> There's no reference to kvm_arch_post_hardware_enable_setup in this
> patch. Looks like it's introduced in a later patch in the series.
>
> This patch might be clearer if the kvm_arch_post_hardware_enable_setup
> is introduced and used here. Otherwise, the commit log should be
> updated to make it clear that kvm_arch_post_hardware_enable_setup is
> introduced in a later patch in the series.
>
> > This makes a room for TDX module to initialize on kvm module loading. TDX
> > module requires all online cpu to enable VMX by VMXON.
> >
> > If kvm_arch_hardware_enable/disable() depend on (*) part, such dependency
> > must be called before kvm_init(). In fact kvm_intel() does. Although
> > other arch doesn't as long as I checked as follows, it should be reviewed
> > by each arch maintainers.
Thank you for pointing it out. I moved the addition of
kvm_arch_post_hardware_enable_setup() to this patch. The related patch is as
follows.
The patch that actually uses kvm_arch_post_hardware_enable_setup() is
"011/104 KVM: TDX: Initialize TDX module when loading kvm_intel.ko". It's
a bit far from this patch. I intentionally positioned this patch early in
this series because this patch requires cross-arch review.
Thanks,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1aead3921a16..55dd08cca5d2 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1441,6 +1441,7 @@ void kvm_arch_create_vcpu_debugfs(struct kvm_vcpu *vcpu, struct dentry *debugfs_
int kvm_arch_hardware_enable(void);
void kvm_arch_hardware_disable(void);
int kvm_arch_hardware_setup(void *opaque);
+int kvm_arch_post_hardware_enable_setup(void *opaque);
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_main.c b/virt/kvm/kvm_main.c
index ec365291c625..55b292fda733 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4883,8 +4883,13 @@ static void hardware_enable_nolock(void *junk)
cpumask_set_cpu(cpu, cpus_hardware_enabled);
+ r = kvm_arch_check_processor_compat();
+ if (r)
+ goto out;
+
r = kvm_arch_hardware_enable();
+out:
if (r) {
cpumask_clear_cpu(cpu, cpus_hardware_enabled);
atomic_inc(&hardware_enable_failed);
@@ -5681,9 +5686,9 @@ void kvm_unregister_perf_callbacks(void)
}
#endif
-static void check_processor_compat(void *rtn)
+__weak int kvm_arch_post_hardware_enable_setup(void *opaque)
{
- *(int *)rtn = kvm_arch_check_processor_compat();
+ return 0;
}
int kvm_init(void *opaque, unsigned vcpu_size, unsigned vcpu_align,
@@ -5716,11 +5721,17 @@ 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;
- }
+ /* hardware_enable_nolock() checks CPU compatibility on each CPUs. */
+ r = hardware_enable_all();
+ if (r)
+ goto out_free_2;
+ /*
+ * Arch specific initialization that requires to enable virtualization
+ * feature. e.g. TDX module initialization requires VMXON on all
+ * present CPUs.
+ */
+ kvm_arch_post_hardware_enable_setup(opaque);
+ hardware_disable_all();
r = cpuhp_setup_state_nocalls(CPUHP_AP_KVM_STARTING, "kvm/cpu:starting",
kvm_starting_cpu, kvm_dying_cpu);
--
Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
