2022-09-08 23:30:52

by Isaku Yamahata

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

From: Isaku Yamahata <[email protected]>

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 <[email protected]>
Signed-off-by: Isaku Yamahata <[email protected]>
Cc: [email protected]
Cc: Fabiano Rosas <[email protected]>
---
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

2022-09-09 07:00:49

by Christophe Leroy

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

Le 09/09/2022 à 01:25, [email protected] a écrit :
> [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
>
> From: Isaku Yamahata <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Isaku Yamahata <[email protected]>
> Cc: [email protected]
> Cc: Fabiano Rosas <[email protected]>
> ---
> 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
>

2022-09-10 01:57:51

by Isaku Yamahata

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

On Fri, Sep 09, 2022 at 05:55:14AM +0000,
Christophe Leroy <[email protected]> wrote:

>
>
> Le 09/09/2022 à 01:25, [email protected] a écrit :
> > [Vous ne recevez pas souvent de courriers de [email protected]. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >
> > From: Isaku Yamahata <[email protected]>
> >
> > 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 <[email protected]>
> > Signed-off-by: Isaku Yamahata <[email protected]>
> > Cc: [email protected]
> > Cc: Fabiano Rosas <[email protected]>
> > ---
> > 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 <[email protected]>