[v4] KVM: hardware enable/disable reorganize

[PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup

Posted by isaku.yamahata@intel.com 3 years, 6 months ago

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

Re: [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup

Posted by Christophe Leroy 3 years, 6 months ago


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
>

Re: [PATCH v4 23/26] RFC: KVM: powerpc: Move processor compatibility check to hardware setup

Posted by Isaku Yamahata 3 years, 6 months ago

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>