On Sun, 2022-08-07 at 15:00 -0700, isaku.yamahata@xxxxxxxxx wrote: > From: Chao Gao <chao.gao@xxxxxxxxx> > > 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. The change about kvm_arch_hardware_setup() > in original commit are still needed so they are not reverted. > > The current implementation enables hardware (e.g. enable VMX on all CPUs), > arch-specific initialization for VM creation, > I guess you need to point out _first_ VM? > and disables hardware (in > x86, disable VMX on all CPUs) for last VM destruction. > > TDX requires its initialization on loading KVM module with VMX enabled on > all available CPUs. It needs to enable/disable hardware on module > initialization. To reuse the same logic, one way is to pass around the To reuse the same logic for what? I think you need to be specific (and focus) on why we need this patch: we will opportunistically move CPU compatibility check to hardware_enable_nolock(), which doesn't take any argument, and this patch is a preparation to do that. > unused opaque argument, another way is to remove the unused opaque > argument. This patch is a preparation for the latter by removing the > argument So how about replacing the last two paragraphs with: " Initializing TDX will be done during module loading time, and in order to do that hardware_enable_all() will be done during module loading time too, as initializing TDX requires all cpus being in VMX operation. As a result, CPU compatibility check will be opportunistically moved to hardware_enable_nolock(), which doesn't take any argument. Instead of passing 'opaque' around to hardware_enable_nolock() and hardware_enable_all(), just remove the unused 'opaque' argument from kvm_arch_check_processor_compat(). " Or even simpler: " To support TDX, hardware_enable_all() will be done during module loading time. As a result, CPU compatibility check will be opportunistically moved to hardware_enable_nolock(), which doesn't take any argument. Instead of passing 'opaque' around to hardware_enable_nolock() and hardware_enable_all(), just remove the unused 'opaque' argument from kvm_arch_check_processor_compat(). " With changelog updated: Reviewed-by: Kai Huang <kai.huang@xxxxxxxxx> > > Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx> > Reviewed-by: Sean Christopherson <seanjc@xxxxxxxxxx> > Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx> > Acked-by: Anup Patel <anup@xxxxxxxxxxxxxx> > Acked-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx> > Link: https://lore.kernel.org/r/20220216031528.92558-3-chao.gao@xxxxxxxxx > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx> > --- > 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 83a7f61354d3..c551ca587f16 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 191992fcb2c2..ca8ef51092c6 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -446,7 +446,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 f5ff9b28e119..e533cce7a70b 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -11990,7 +11990,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 1c480b1821e1..9643b8eadefe 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 da263c370d00..0f5767e5ae45 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -5793,22 +5793,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; > > @@ -5836,10 +5828,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; > }