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

Reply via email to