Re: [RFC PATCH 09/11] kvm: simplify processor compat check
On Sat, Sep 28, 2013 at 09:06:47PM +0530, Aneesh Kumar K.V wrote:
Paolo Bonzini pbonz...@redhat.com writes:
Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
Alexander Graf ag...@suse.de writes:
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the
day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
But whether to check on all cpus or not is arch dependent right?.
IIUC only x86 and ppc64 need to do that. Also on ppc64 it really
depends on whether HV or PR. We need to check on all cpus only if it is
HV.
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think do I need to check this on
all online CPUs?) and making the leaf POWER code less legible because
it still has the weird void()(void *) calling convention.
IIUC what we wanted to check is to find out whether kvm can run on this
system. That is really an arch specific check. So for core kvm the call
should be a simple
if (kvm_arch_check_process_compat() 0)
error;
We have that already, just return error from kvm_arch_hardware_setup. This
is specific processor compatibility check and you are arguing that the
processor check should be part of kvm_arch_hardware_setup().
Now how each arch figure out whether kvm can run on this system should
be arch specific. For x86 we do need to check all the cpus. On ppc64 for
HV we need to. For other archs we always allow kvm.
This is really a sanity check. Theoretically on x86 we also should
not need to check all cpus since SMP configuration with different cpu
models is not supported by the architecture (AFAIK), but bugs happen
(BIOS bugs may cause difference in capabilities for instance). So some
arches opted out from this sanity check for now and this is their choice,
but the code makes it explicit what are we checking here.
If anything, you could change kvm_arch_check_processor_compat to return
an int and accept no argument, and introduce a wrapper that kvm_init
passes to smp_call_function_single.
What i am suggesting in the patch is to avoid calling
smp_call_function_single from kvm_init and let arch decide whether to
check on all cpus or not.
-aneesh
--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 09/11] kvm: simplify processor compat check
Gleb Natapov g...@redhat.com writes:
On Sat, Sep 28, 2013 at 09:06:47PM +0530, Aneesh Kumar K.V wrote:
Paolo Bonzini pbonz...@redhat.com writes:
Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
Alexander Graf ag...@suse.de writes:
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the
day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
But whether to check on all cpus or not is arch dependent right?.
IIUC only x86 and ppc64 need to do that. Also on ppc64 it really
depends on whether HV or PR. We need to check on all cpus only if it is
HV.
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think do I need to check this on
all online CPUs?) and making the leaf POWER code less legible because
it still has the weird void()(void *) calling convention.
IIUC what we wanted to check is to find out whether kvm can run on this
system. That is really an arch specific check. So for core kvm the call
should be a simple
if (kvm_arch_check_process_compat() 0)
error;
We have that already, just return error from kvm_arch_hardware_setup. This
is specific processor compatibility check and you are arguing that the
processor check should be part of kvm_arch_hardware_setup().
What about the success case ?. ie, on arch like arm we do
void kvm_arch_check_processor_compat(void *rtn)
{
*(int *)rtn = 0;
}
for_each_online_cpu(cpu) {
smp_call_function_single(cpu,
kvm_arch_check_processor_compat,
r, 1);
if (r 0)
goto out_free_1;
}
There is no need to do that for loop for arm.
The only reason I wanted this patch in the series is to make
kvm_arch_check_processor_compat take additional argument opaque.
I am dropping that requirement in the last patch. Considering
that we have objection to this one, I will drop this patch in
the next posting by rearranging the patches.
-aneesh
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 09/11] kvm: simplify processor compat check
On Sun, Sep 29, 2013 at 08:35:16PM +0530, Aneesh Kumar K.V wrote:
Gleb Natapov g...@redhat.com writes:
On Sat, Sep 28, 2013 at 09:06:47PM +0530, Aneesh Kumar K.V wrote:
Paolo Bonzini pbonz...@redhat.com writes:
Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
Alexander Graf ag...@suse.de writes:
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the
day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
But whether to check on all cpus or not is arch dependent right?.
IIUC only x86 and ppc64 need to do that. Also on ppc64 it really
depends on whether HV or PR. We need to check on all cpus only if it is
HV.
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think do I need to check this on
all online CPUs?) and making the leaf POWER code less legible because
it still has the weird void()(void *) calling convention.
IIUC what we wanted to check is to find out whether kvm can run on this
system. That is really an arch specific check. So for core kvm the call
should be a simple
if (kvm_arch_check_process_compat() 0)
error;
We have that already, just return error from kvm_arch_hardware_setup. This
is specific processor compatibility check and you are arguing that the
processor check should be part of kvm_arch_hardware_setup().
What about the success case ?. ie, on arch like arm we do
void kvm_arch_check_processor_compat(void *rtn)
{
*(int *)rtn = 0;
}
for_each_online_cpu(cpu) {
As I said they opted out from doing the check. They may reconsider after
first bad HW will be discovered.
smp_call_function_single(cpu,
kvm_arch_check_processor_compat,
r, 1);
if (r 0)
goto out_free_1;
}
There is no need to do that for loop for arm.
It's done once during module initialisation. Why is this a big deal?
--
Gleb.
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 09/11] kvm: simplify processor compat check
Paolo Bonzini pbonz...@redhat.com writes:
Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
Alexander Graf ag...@suse.de writes:
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the
day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
But whether to check on all cpus or not is arch dependent right?.
IIUC only x86 and ppc64 need to do that. Also on ppc64 it really
depends on whether HV or PR. We need to check on all cpus only if it is
HV.
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think do I need to check this on
all online CPUs?) and making the leaf POWER code less legible because
it still has the weird void()(void *) calling convention.
IIUC what we wanted to check is to find out whether kvm can run on this
system. That is really an arch specific check. So for core kvm the call
should be a simple
if (kvm_arch_check_process_compat() 0)
error;
Now how each arch figure out whether kvm can run on this system should
be arch specific. For x86 we do need to check all the cpus. On ppc64 for
HV we need to. For other archs we always allow kvm.
If anything, you could change kvm_arch_check_processor_compat to return
an int and accept no argument, and introduce a wrapper that kvm_init
passes to smp_call_function_single.
What i am suggesting in the patch is to avoid calling
smp_call_function_single from kvm_init and let arch decide whether to
check on all cpus or not.
-aneesh
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 09/11] kvm: simplify processor compat check
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
---
arch/arm/kvm/arm.c | 4 ++--
arch/ia64/kvm/kvm-ia64.c | 4 ++--
arch/mips/kvm/kvm_mips.c | 6 ++
arch/powerpc/include/asm/kvm_ppc.h | 2 +-
arch/powerpc/kvm/44x.c | 2 +-
arch/powerpc/kvm/book3s.c | 15 ---
arch/powerpc/kvm/book3s_hv.c | 9 ++---
arch/powerpc/kvm/book3s_pr.c | 5 +++--
arch/powerpc/kvm/e500.c| 2 +-
arch/powerpc/kvm/e500mc.c | 2 +-
arch/powerpc/kvm/powerpc.c | 5 -
arch/s390/kvm/kvm-s390.c | 3 ++-
arch/x86/kvm/x86.c | 13 +++--
include/linux/kvm_host.h | 2 +-
virt/kvm/kvm_main.c| 14 +-
15 files changed, 50 insertions(+), 38 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9c697db..cccb121 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -109,9 +109,9 @@ void kvm_arch_hardware_unsetup(void)
{
}
-void kvm_arch_check_processor_compat(void *rtn)
+int kvm_arch_check_processor_compat(void *opaque)
{
- *(int *)rtn = 0;
+ return 0;
}
void kvm_arch_sync_events(struct kvm *kvm)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index bdfd878..065942c 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -185,9 +185,9 @@ void kvm_arch_hardware_disable(void *garbage)
ia64_ptr_entry(0x3, slot);
}
-void kvm_arch_check_processor_compat(void *rtn)
+int kvm_arch_check_processor_compat(void *opaque)
{
- *(int *)rtn = 0;
+ return 0;
}
int kvm_dev_ioctl_check_extension(long ext)
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index a7b0445..4512739 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -97,11 +97,9 @@ void kvm_arch_hardware_unsetup(void)
{
}
-void kvm_arch_check_processor_compat(void *rtn)
+int kvm_arch_check_processor_compat(void *opaque)
{
- int *r = (int *)rtn;
- *r = 0;
- return;
+ return 0;
}
static void kvm_mips_init_tlbs(struct kvm *kvm)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 58e732f..592501b 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -204,7 +204,7 @@ struct kvmppc_ops {
unsigned long npages);
int (*init_vm)(struct kvm *kvm);
void (*destroy_vm)(struct kvm *kvm);
- int (*check_processor_compat)(void);
+ void (*check_processor_compat)(void *r);
int (*get_smmu_info)(struct kvm *kvm, struct kvm_ppc_smmu_info *info);
int (*emulate_op)(struct kvm_run *run, struct kvm_vcpu *vcpu,
unsigned int inst, int *advance);
diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c
index 2f5c6b6..a1f4e60 100644
--- a/arch/powerpc/kvm/44x.c
+++ b/arch/powerpc/kvm/44x.c
@@ -43,7 +43,7 @@ void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu)
kvmppc_booke_vcpu_put(vcpu);
}
-int kvmppc_core_check_processor_compat(void)
+int kvm_arch_check_processor_compat(void *opaque)
{
int r;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index ca617e1..485a6ff 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -827,9 +827,18 @@ void kvmppc_core_destroy_vm(struct kvm *kvm)
#endif
}
-int kvmppc_core_check_processor_compat(void)
-{
- return kvmppc_ops-check_processor_compat();
+int kvm_arch_check_processor_compat(void *opaque)
+{
+ int r,cpu;
+ struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
+ for_each_online_cpu(cpu) {
+ smp_call_function_single(cpu,
+kvm_ops-check_processor_compat,
+r, 1);
+ if (r 0)
+ break;
+ }
+ return r;
}
EXPORT_SYMBOL_GPL(kvm_get_dirty_log);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index ff57be8..4322db4 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1980,11 +1980,14 @@ static int kvmppc_core_emulate_mfspr_hv(struct kvm_vcpu
*vcpu, int sprn,
return EMULATE_FAIL;
}
-static int kvmppc_core_check_processor_compat_hv(void)
+
+static void kvmppc_core_check_processor_compat_hv(void *r)
{
if (!cpu_has_feature(CPU_FTR_HVMODE))
- return -EIO;
- return 0;
+ *(int *)r = -EIO;
+ else
+ *(int *)r = 0;
+ return;
}
static long kvm_arch_vm_ioctl_hv(struct file *filp,
diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c
index df48d89..127b961 100644
--- a/arch/powerpc/kvm/book3s_pr.c
+++ b/arch/powerpc/kvm/book3s_pr.c
@@ -1490,10 +1490,11 @@ static void kvmppc_core_destroy_vm_pr(struct kvm *kvm)
Re: [RFC PATCH 09/11] kvm: simplify processor compat check
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the day it's
Gleb's and Paolo's call.
Which brings me to the next issue: You forgot to CC kvm@vger on your patch set.
Gleb and Paolo don't read kvm-ppc@vger. And they shouldn't have to. Every kvm
patch that you want review on or that should get applied needs to be sent to
kvm@vger. If you want to tag it as PPC specific patch, do so by CC'ing
kvm-ppc@vger.
Alex
---
arch/arm/kvm/arm.c | 4 ++--
arch/ia64/kvm/kvm-ia64.c | 4 ++--
arch/mips/kvm/kvm_mips.c | 6 ++
arch/powerpc/include/asm/kvm_ppc.h | 2 +-
arch/powerpc/kvm/44x.c | 2 +-
arch/powerpc/kvm/book3s.c | 15 ---
arch/powerpc/kvm/book3s_hv.c | 9 ++---
arch/powerpc/kvm/book3s_pr.c | 5 +++--
arch/powerpc/kvm/e500.c| 2 +-
arch/powerpc/kvm/e500mc.c | 2 +-
arch/powerpc/kvm/powerpc.c | 5 -
arch/s390/kvm/kvm-s390.c | 3 ++-
arch/x86/kvm/x86.c | 13 +++--
include/linux/kvm_host.h | 2 +-
virt/kvm/kvm_main.c| 14 +-
15 files changed, 50 insertions(+), 38 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 9c697db..cccb121 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -109,9 +109,9 @@ void kvm_arch_hardware_unsetup(void)
{
}
-void kvm_arch_check_processor_compat(void *rtn)
+int kvm_arch_check_processor_compat(void *opaque)
{
- *(int *)rtn = 0;
+ return 0;
}
void kvm_arch_sync_events(struct kvm *kvm)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index bdfd878..065942c 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -185,9 +185,9 @@ void kvm_arch_hardware_disable(void *garbage)
ia64_ptr_entry(0x3, slot);
}
-void kvm_arch_check_processor_compat(void *rtn)
+int kvm_arch_check_processor_compat(void *opaque)
{
- *(int *)rtn = 0;
+ return 0;
}
int kvm_dev_ioctl_check_extension(long ext)
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index a7b0445..4512739 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -97,11 +97,9 @@ void kvm_arch_hardware_unsetup(void)
{
}
-void kvm_arch_check_processor_compat(void *rtn)
+int kvm_arch_check_processor_compat(void *opaque)
{
- int *r = (int *)rtn;
- *r = 0;
- return;
+ return 0;
}
static void kvm_mips_init_tlbs(struct kvm *kvm)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 58e732f..592501b 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -204,7 +204,7 @@ struct kvmppc_ops {
unsigned long npages);
int (*init_vm)(struct kvm *kvm);
void (*destroy_vm)(struct kvm *kvm);
- int (*check_processor_compat)(void);
+ void (*check_processor_compat)(void *r);
int (*get_smmu_info)(struct kvm *kvm, struct kvm_ppc_smmu_info *info);
int (*emulate_op)(struct kvm_run *run, struct kvm_vcpu *vcpu,
unsigned int inst, int *advance);
diff --git a/arch/powerpc/kvm/44x.c b/arch/powerpc/kvm/44x.c
index 2f5c6b6..a1f4e60 100644
--- a/arch/powerpc/kvm/44x.c
+++ b/arch/powerpc/kvm/44x.c
@@ -43,7 +43,7 @@ void kvmppc_core_vcpu_put(struct kvm_vcpu *vcpu)
kvmppc_booke_vcpu_put(vcpu);
}
-int kvmppc_core_check_processor_compat(void)
+int kvm_arch_check_processor_compat(void *opaque)
{
int r;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index ca617e1..485a6ff 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -827,9 +827,18 @@ void kvmppc_core_destroy_vm(struct kvm *kvm)
#endif
}
-int kvmppc_core_check_processor_compat(void)
-{
- return kvmppc_ops-check_processor_compat();
+int kvm_arch_check_processor_compat(void *opaque)
+{
+ int r,cpu;
+ struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
+ for_each_online_cpu(cpu) {
+ smp_call_function_single(cpu,
+ kvm_ops-check_processor_compat,
+ r, 1);
+ if (r 0)
+ break;
+ }
+ return r;
}
EXPORT_SYMBOL_GPL(kvm_get_dirty_log);
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index ff57be8..4322db4 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1980,11 +1980,14 @@ static int kvmppc_core_emulate_mfspr_hv(struct
kvm_vcpu *vcpu, int sprn,
return EMULATE_FAIL;
}
-static int kvmppc_core_check_processor_compat_hv(void)
+
+static void
Re: [RFC PATCH 09/11] kvm: simplify processor compat check
Alexander Graf ag...@suse.de writes:
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the
day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
We also want to make the smp call function a callback of opaque. Hence
this should be made arch specific.
int kvm_arch_check_processor_compat(void *opaque)
{
int r,cpu;
struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
for_each_online_cpu(cpu) {
smp_call_function_single(cpu,
kvm_ops-check_processor_compat,
r, 1);
if (r 0)
break;
}
return r;
}
against
- for_each_online_cpu(cpu) {
- smp_call_function_single(cpu,
- kvm_arch_check_processor_compat,
- r, 1);
- if (r 0)
- goto out_free_1;
- }
+
+ r = kvm_arch_check_processor_compat(opaque);
+ if (r 0)
+ goto out_free_1;
Which brings me to the next issue: You forgot to CC kvm@vger on your
patch set. Gleb and Paolo don't read kvm-ppc@vger. And they shouldn't
have to. Every kvm patch that you want review on or that should get
applied needs to be sent to kvm@vger. If you want to tag it as PPC
specific patch, do so by CC'ing kvm-ppc@vger.
Will do in the next update
-aneesh
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 09/11] kvm: simplify processor compat check
Il 27/09/2013 15:13, Aneesh Kumar K.V ha scritto:
Alexander Graf ag...@suse.de writes:
On 27.09.2013, at 12:03, Aneesh Kumar K.V wrote:
From: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Missing patch description.
Signed-off-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
I fail to see how this really simplifies things, but at the end of the
day it's Gleb's and Paolo's call.
will do. It avoid calling
for_each_online_cpu(cpu) {
smp_call_function_single()
on multiple architecture.
I agree with Alex.
The current code is not specially awesome; having
kvm_arch_check_processor_compat take an int* disguised as a void* is a
bit ugly indeed.
However, the API makes sense and tells you that it is being passed as a
callback (to smp_call_function_single in this case).
You are making the API more complicated to use on the arch layer
(because arch maintainers now have to think do I need to check this on
all online CPUs?) and making the leaf POWER code less legible because
it still has the weird void()(void *) calling convention.
If anything, you could change kvm_arch_check_processor_compat to return
an int and accept no argument, and introduce a wrapper that kvm_init
passes to smp_call_function_single.
Paolo
We also want to make the smp call function a callback of opaque. Hence
this should be made arch specific.
int kvm_arch_check_processor_compat(void *opaque)
{
int r,cpu;
struct kvmppc_ops *kvm_ops = (struct kvmppc_ops *)opaque;
for_each_online_cpu(cpu) {
smp_call_function_single(cpu,
kvm_ops-check_processor_compat,
r, 1);
if (r 0)
break;
}
return r;
}
against
- for_each_online_cpu(cpu) {
- smp_call_function_single(cpu,
- kvm_arch_check_processor_compat,
- r, 1);
- if (r 0)
- goto out_free_1;
- }
+
+ r = kvm_arch_check_processor_compat(opaque);
+ if (r 0)
+ goto out_free_1;
Which brings me to the next issue: You forgot to CC kvm@vger on your
patch set. Gleb and Paolo don't read kvm-ppc@vger. And they shouldn't
have to. Every kvm patch that you want review on or that should get
applied needs to be sent to kvm@vger. If you want to tag it as PPC
specific patch, do so by CC'ing kvm-ppc@vger.
Will do in the next update
-aneesh
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
