| Message ID | 574ca90fdaec0f37c197d9600d47d48a74f324bd.1663869838.git.isaku.yamahata@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: hardware enable/disable reorganize | expand |
isaku.yamahata@intel.com writes: > From: Isaku Yamahata <isaku.yamahata@intel.com> > > Move processor compatibility check from kvm_arch_processor_compat() into ^ kvm_arch_check_processor_compat() > 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. A key detail I had to look up is that both kvm_arch_hardware_setup() and kvm_arch_check_processor_compat() are called from kvm_init(), one after the other. But the latter is called on each CPU. And because the powerpc implementation of kvm_arch_check_processor_compat() just checks a global, there's no need to call it on every CPU. > kvmppc_core_check_processor_compat() checks the global variable. There are > five implementation for it as follows. There are three implementations not five. > arch/powerpc/include/asm/cputable.h: extern struct cpu_spec *cur_cpu_spec; > 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") > > 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 | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 7b56d6ccfdfb..31dc4f231e9d 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -444,12 +444,12 @@ int kvm_arch_hardware_enable(void) > > int kvm_arch_hardware_setup(void *opaque) > { > - return 0; > + return kvmppc_core_check_processor_compat(); > } > > int kvm_arch_check_processor_compat(void) > { > - return kvmppc_core_check_processor_compat(); > + return 0; > } The actual change seems OK. I gave it a quick test boot and ran some VMs, everything seems to work as before. Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) cheers
On Fri, Sep 23, 2022 at 04:58:41PM +1000, Michael Ellerman <mpe@ellerman.id.au> wrote: > isaku.yamahata@intel.com writes: > > From: Isaku Yamahata <isaku.yamahata@intel.com> > > > > Move processor compatibility check from kvm_arch_processor_compat() into > ^ > kvm_arch_check_processor_compat() > > > 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. > > A key detail I had to look up is that both kvm_arch_hardware_setup() and > kvm_arch_check_processor_compat() are called from kvm_init(), one after > the other. But the latter is called on each CPU. > > And because the powerpc implementation of kvm_arch_check_processor_compat() > just checks a global, there's no need to call it on every CPU. > > > kvmppc_core_check_processor_compat() checks the global variable. There are > > five implementation for it as follows. > > There are three implementations not five. Thanks. I'll update the commit message. > > arch/powerpc/include/asm/cputable.h: extern struct cpu_spec *cur_cpu_spec; > > 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") > > > > 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 | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > > index 7b56d6ccfdfb..31dc4f231e9d 100644 > > --- a/arch/powerpc/kvm/powerpc.c > > +++ b/arch/powerpc/kvm/powerpc.c > > @@ -444,12 +444,12 @@ int kvm_arch_hardware_enable(void) > > > > int kvm_arch_hardware_setup(void *opaque) > > { > > - return 0; > > + return kvmppc_core_check_processor_compat(); > > } > > > > int kvm_arch_check_processor_compat(void) > > { > > - return kvmppc_core_check_processor_compat(); > > + return 0; > > } > > The actual change seems OK. I gave it a quick test boot and ran some > VMs, everything seems to work as before. > > Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc) Thanks so much for testing. I'll remove RFC.
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 7b56d6ccfdfb..31dc4f231e9d 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -444,12 +444,12 @@ int kvm_arch_hardware_enable(void) int kvm_arch_hardware_setup(void *opaque) { - return 0; + 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)