Re: [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu

2014-09-16 Thread Anup Patel
On Thu, Sep 11, 2014 at 9:24 PM, Andre Przywara andre.przyw...@arm.com wrote:
 Hi Anup,

 On 08/09/14 09:17, Anup Patel wrote:
 Instead, of trying out each and every target type we should
 use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type
 for KVM ARM/ARM64.

 If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to
 old method of trying all known target types.

 If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
 target type is not known to KVMTOOL then we forcefully init
 VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.

 Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org
 Signed-off-by: Anup Patel anup.pa...@linaro.org
 ---
  tools/kvm/arm/kvm-cpu.c |   52 
 +--
  1 file changed, 41 insertions(+), 11 deletions(-)

 diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
 index aeaa4cf..ba7a762 100644
 --- a/tools/kvm/arm/kvm-cpu.c
 +++ b/tools/kvm/arm/kvm-cpu.c
 @@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   struct kvm_arm_target *target;
   struct kvm_cpu *vcpu;
   int coalesced_offset, mmap_size, err = -1;
 - unsigned int i;
 + unsigned int i, target_type;
 + struct kvm_vcpu_init preferred_init;
   struct kvm_vcpu_init vcpu_init = {
   .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
   };
 @@ -55,19 +56,47 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   if (vcpu-kvm_run == MAP_FAILED)
   die(unable to mmap vcpu fd);

 - /* Find an appropriate target CPU type. */
 - for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 - if (!kvm_arm_targets[i])
 - continue;
 - target = kvm_arm_targets[i];
 - vcpu_init.target = target-id;
 - err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);
 - if (!err)
 - break;
 + /*
 +  * If preferred target ioctl successful then use preferred target
 +  * else try each and every target type.
 +  */
 + err = ioctl(kvm-vm_fd, KVM_ARM_PREFERRED_TARGET, preferred_init);
 + if (!err) {
 + /* Match preferred target CPU type. */
 + target = NULL;
 + for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 + if (!kvm_arm_targets[i])
 + continue;
 + if (kvm_arm_targets[i]-id == preferred_init.target) {
 + target = kvm_arm_targets[i];
 + target_type = kvm_arm_targets[i]-id;
 + break;
 + }
 + }
 + if (!target) {
 + target = kvm_arm_targets[0];

 I think you missed the part of the patch which adds the now magic zero
 member of kvm_arm_targets[]. A simple static initializer should work.

Yes, good catch. I will fix this.


 + target_type = preferred_init.target;

 Can't you move that out of the loop, in front of it actually? Then you
 can get rid of the line above setting the target_type also, since you
 always use the same value now, regardless whether you found that CPU in
 the list or not.

Sure, I'll rearrange this.


 + }
 + } else {
 + /* Find an appropriate target CPU type. */
 + for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 + if (!kvm_arm_targets[i])
 + continue;
 + target = kvm_arm_targets[i];
 + target_type = target-id;
 + vcpu_init.target = target_type;
 + err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, 
 vcpu_init);
 + if (!err)
 + break;
 + }
 + if (err)
 + die(Unable to find matching target);
   }

 + vcpu_init.target = target_type;
 + err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);

 You should do this only in the if-branch above, since you (try to) call
 KVM_ARM_VCPU_INIT already in the else branch before. Otherwise in the
 latter case you would do it twice.

I was trying to follow the old code which was doing KVM_ARM_VCPU_INIT
twice for the same VCPU but it makes sense to do it only once.

Regards,
Anup


 Regards,
 Andre.

   if (err || target-init(vcpu))
 - die(Unable to initialise ARM vcpu);
 + die(Unable to initialise vcpu);

   coalesced_offset = ioctl(kvm-sys_fd, KVM_CHECK_EXTENSION,
KVM_CAP_COALESCED_MMIO);
 @@ -81,6 +110,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   vcpu-cpu_type  = target-id;
   vcpu-cpu_compatible= target-compatible;
   vcpu-is_running= true;
 +
   return vcpu;
  }


 CONFIDENTIALITY 

Re: [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu

2014-09-16 Thread Anup Patel
On Wed, Sep 17, 2014 at 3:43 AM, Anup Patel apa...@apm.com wrote:
 On Thu, Sep 11, 2014 at 9:24 PM, Andre Przywara andre.przyw...@arm.com 
 wrote:
 Hi Anup,

 On 08/09/14 09:17, Anup Patel wrote:
 Instead, of trying out each and every target type we should
 use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type
 for KVM ARM/ARM64.

 If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to
 old method of trying all known target types.

 If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
 target type is not known to KVMTOOL then we forcefully init
 VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.

 Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org
 Signed-off-by: Anup Patel anup.pa...@linaro.org
 ---
  tools/kvm/arm/kvm-cpu.c |   52 
 +--
  1 file changed, 41 insertions(+), 11 deletions(-)

 diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
 index aeaa4cf..ba7a762 100644
 --- a/tools/kvm/arm/kvm-cpu.c
 +++ b/tools/kvm/arm/kvm-cpu.c
 @@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   struct kvm_arm_target *target;
   struct kvm_cpu *vcpu;
   int coalesced_offset, mmap_size, err = -1;
 - unsigned int i;
 + unsigned int i, target_type;
 + struct kvm_vcpu_init preferred_init;
   struct kvm_vcpu_init vcpu_init = {
   .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
   };
 @@ -55,19 +56,47 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   if (vcpu-kvm_run == MAP_FAILED)
   die(unable to mmap vcpu fd);

 - /* Find an appropriate target CPU type. */
 - for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 - if (!kvm_arm_targets[i])
 - continue;
 - target = kvm_arm_targets[i];
 - vcpu_init.target = target-id;
 - err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);
 - if (!err)
 - break;
 + /*
 +  * If preferred target ioctl successful then use preferred target
 +  * else try each and every target type.
 +  */
 + err = ioctl(kvm-vm_fd, KVM_ARM_PREFERRED_TARGET, preferred_init);
 + if (!err) {
 + /* Match preferred target CPU type. */
 + target = NULL;
 + for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 + if (!kvm_arm_targets[i])
 + continue;
 + if (kvm_arm_targets[i]-id == preferred_init.target) {
 + target = kvm_arm_targets[i];
 + target_type = kvm_arm_targets[i]-id;
 + break;
 + }
 + }
 + if (!target) {
 + target = kvm_arm_targets[0];

 I think you missed the part of the patch which adds the now magic zero
 member of kvm_arm_targets[]. A simple static initializer should work.

 Yes, good catch. I will fix this.


 + target_type = preferred_init.target;

 Can't you move that out of the loop, in front of it actually? Then you
 can get rid of the line above setting the target_type also, since you
 always use the same value now, regardless whether you found that CPU in
 the list or not.

 Sure, I'll rearrange this.


 + }
 + } else {
 + /* Find an appropriate target CPU type. */
 + for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 + if (!kvm_arm_targets[i])
 + continue;
 + target = kvm_arm_targets[i];
 + target_type = target-id;
 + vcpu_init.target = target_type;
 + err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, 
 vcpu_init);
 + if (!err)
 + break;
 + }
 + if (err)
 + die(Unable to find matching target);
   }

 + vcpu_init.target = target_type;
 + err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);

 You should do this only in the if-branch above, since you (try to) call
 KVM_ARM_VCPU_INIT already in the else branch before. Otherwise in the
 latter case you would do it twice.

 I was trying to follow the old code which was doing KVM_ARM_VCPU_INIT
 twice for the same VCPU but it makes sense to do it only once.

 Regards,
 Anup


 Regards,
 Andre.

   if (err || target-init(vcpu))
 - die(Unable to initialise ARM vcpu);
 + die(Unable to initialise vcpu);

   coalesced_offset = ioctl(kvm-sys_fd, KVM_CHECK_EXTENSION,
KVM_CAP_COALESCED_MMIO);
 @@ -81,6 +110,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   vcpu-cpu_type  = target-id;
   vcpu-cpu_compatible= target-compatible;
   

Re: [PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu

2014-09-11 Thread Andre Przywara
Hi Anup,

On 08/09/14 09:17, Anup Patel wrote:
 Instead, of trying out each and every target type we should
 use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type
 for KVM ARM/ARM64.
 
 If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to
 old method of trying all known target types.
 
 If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
 target type is not known to KVMTOOL then we forcefully init
 VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.
 
 Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org
 Signed-off-by: Anup Patel anup.pa...@linaro.org
 ---
  tools/kvm/arm/kvm-cpu.c |   52 
 +--
  1 file changed, 41 insertions(+), 11 deletions(-)
 
 diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
 index aeaa4cf..ba7a762 100644
 --- a/tools/kvm/arm/kvm-cpu.c
 +++ b/tools/kvm/arm/kvm-cpu.c
 @@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   struct kvm_arm_target *target;
   struct kvm_cpu *vcpu;
   int coalesced_offset, mmap_size, err = -1;
 - unsigned int i;
 + unsigned int i, target_type;
 + struct kvm_vcpu_init preferred_init;
   struct kvm_vcpu_init vcpu_init = {
   .features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
   };
 @@ -55,19 +56,47 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   if (vcpu-kvm_run == MAP_FAILED)
   die(unable to mmap vcpu fd);
  
 - /* Find an appropriate target CPU type. */
 - for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 - if (!kvm_arm_targets[i])
 - continue;
 - target = kvm_arm_targets[i];
 - vcpu_init.target = target-id;
 - err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);
 - if (!err)
 - break;
 + /*
 +  * If preferred target ioctl successful then use preferred target
 +  * else try each and every target type.
 +  */
 + err = ioctl(kvm-vm_fd, KVM_ARM_PREFERRED_TARGET, preferred_init);
 + if (!err) {
 + /* Match preferred target CPU type. */
 + target = NULL;
 + for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 + if (!kvm_arm_targets[i])
 + continue;
 + if (kvm_arm_targets[i]-id == preferred_init.target) {
 + target = kvm_arm_targets[i];
 + target_type = kvm_arm_targets[i]-id;
 + break;
 + }
 + }
 + if (!target) {
 + target = kvm_arm_targets[0];

I think you missed the part of the patch which adds the now magic zero
member of kvm_arm_targets[]. A simple static initializer should work.

 + target_type = preferred_init.target;

Can't you move that out of the loop, in front of it actually? Then you
can get rid of the line above setting the target_type also, since you
always use the same value now, regardless whether you found that CPU in
the list or not.

 + }
 + } else {
 + /* Find an appropriate target CPU type. */
 + for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
 + if (!kvm_arm_targets[i])
 + continue;
 + target = kvm_arm_targets[i];
 + target_type = target-id;
 + vcpu_init.target = target_type;
 + err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, 
 vcpu_init);
 + if (!err)
 + break;
 + }
 + if (err)
 + die(Unable to find matching target);
   }
  
 + vcpu_init.target = target_type;
 + err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);

You should do this only in the if-branch above, since you (try to) call
KVM_ARM_VCPU_INIT already in the else branch before. Otherwise in the
latter case you would do it twice.

Regards,
Andre.

   if (err || target-init(vcpu))
 - die(Unable to initialise ARM vcpu);
 + die(Unable to initialise vcpu);
  
   coalesced_offset = ioctl(kvm-sys_fd, KVM_CHECK_EXTENSION,
KVM_CAP_COALESCED_MMIO);
 @@ -81,6 +110,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
 unsigned long cpu_id)
   vcpu-cpu_type  = target-id;
   vcpu-cpu_compatible= target-compatible;
   vcpu-is_running= true;
 +
   return vcpu;
  }
  
 
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3 1/4] kvmtool: ARM: Use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target cpu

2014-09-08 Thread Anup Patel
Instead, of trying out each and every target type we should
use KVM_ARM_PREFERRED_TARGET vm ioctl to determine target type
for KVM ARM/ARM64.

If KVM_ARM_PREFERRED_TARGET vm ioctl fails then we fallback to
old method of trying all known target types.

If KVM_ARM_PREFERRED_TARGET vm ioctl succeeds but the returned
target type is not known to KVMTOOL then we forcefully init
VCPU with target type returned by KVM_ARM_PREFERRED_TARGET vm ioctl.

Signed-off-by: Pranavkumar Sawargaonkar pranavku...@linaro.org
Signed-off-by: Anup Patel anup.pa...@linaro.org
---
 tools/kvm/arm/kvm-cpu.c |   52 +--
 1 file changed, 41 insertions(+), 11 deletions(-)

diff --git a/tools/kvm/arm/kvm-cpu.c b/tools/kvm/arm/kvm-cpu.c
index aeaa4cf..ba7a762 100644
--- a/tools/kvm/arm/kvm-cpu.c
+++ b/tools/kvm/arm/kvm-cpu.c
@@ -33,7 +33,8 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned 
long cpu_id)
struct kvm_arm_target *target;
struct kvm_cpu *vcpu;
int coalesced_offset, mmap_size, err = -1;
-   unsigned int i;
+   unsigned int i, target_type;
+   struct kvm_vcpu_init preferred_init;
struct kvm_vcpu_init vcpu_init = {
.features = ARM_VCPU_FEATURE_FLAGS(kvm, cpu_id)
};
@@ -55,19 +56,47 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, 
unsigned long cpu_id)
if (vcpu-kvm_run == MAP_FAILED)
die(unable to mmap vcpu fd);
 
-   /* Find an appropriate target CPU type. */
-   for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
-   if (!kvm_arm_targets[i])
-   continue;
-   target = kvm_arm_targets[i];
-   vcpu_init.target = target-id;
-   err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);
-   if (!err)
-   break;
+   /*
+* If preferred target ioctl successful then use preferred target
+* else try each and every target type.
+*/
+   err = ioctl(kvm-vm_fd, KVM_ARM_PREFERRED_TARGET, preferred_init);
+   if (!err) {
+   /* Match preferred target CPU type. */
+   target = NULL;
+   for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
+   if (!kvm_arm_targets[i])
+   continue;
+   if (kvm_arm_targets[i]-id == preferred_init.target) {
+   target = kvm_arm_targets[i];
+   target_type = kvm_arm_targets[i]-id;
+   break;
+   }
+   }
+   if (!target) {
+   target = kvm_arm_targets[0];
+   target_type = preferred_init.target;
+   }
+   } else {
+   /* Find an appropriate target CPU type. */
+   for (i = 0; i  ARRAY_SIZE(kvm_arm_targets); ++i) {
+   if (!kvm_arm_targets[i])
+   continue;
+   target = kvm_arm_targets[i];
+   target_type = target-id;
+   vcpu_init.target = target_type;
+   err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, 
vcpu_init);
+   if (!err)
+   break;
+   }
+   if (err)
+   die(Unable to find matching target);
}
 
+   vcpu_init.target = target_type;
+   err = ioctl(vcpu-vcpu_fd, KVM_ARM_VCPU_INIT, vcpu_init);
if (err || target-init(vcpu))
-   die(Unable to initialise ARM vcpu);
+   die(Unable to initialise vcpu);
 
coalesced_offset = ioctl(kvm-sys_fd, KVM_CHECK_EXTENSION,
 KVM_CAP_COALESCED_MMIO);
@@ -81,6 +110,7 @@ struct kvm_cpu *kvm_cpu__arch_init(struct kvm *kvm, unsigned 
long cpu_id)
vcpu-cpu_type  = target-id;
vcpu-cpu_compatible= target-compatible;
vcpu-is_running= true;
+
return vcpu;
 }
 
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html