Re: [Qemu-devel] [PATCH v2 3/7] AARCH64: Add aarch64 CPU initialization, get and put registers support

[Peter Maydell]

On 23 July 2013 10:33, Mian M. Hamayun <address@hidden> wrote:
> From: "Mian M. Hamayun" <address@hidden>
>
> The cpu init function tries to initialize with all possible cpu types, as
> KVM does not provide a means to detect the real cpu type and simply refuses
> to initialize on cpu type mis-match. By using the loop based init function,
> we avoid the need to modify code if the underlying platform is different,
> such as Fast Models instead of Foundation Models.
>
> Get and Put Registers deal with the basic state of Aarch64 CPUs for the 
> moment.
>
> Signed-off-by: Mian M. Hamayun <address@hidden>
>
> Conflicts:
>
>         target-arm/kvm.c
>
> Conflicts:
>
>         target-arm/cpu.c

This sort of Conflicts note shouldn't be in a commit message.

> ---
>  linux-headers/linux/kvm.h |    1 +
>  target-arm/kvm.c          |  125 
> +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 126 insertions(+)
>
> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> index c614070..4df5292 100644
> --- a/linux-headers/linux/kvm.h
> +++ b/linux-headers/linux/kvm.h
> @@ -783,6 +783,7 @@ struct kvm_dirty_tlb {
>  #define KVM_REG_IA64           0x3000000000000000ULL
>  #define KVM_REG_ARM            0x4000000000000000ULL
>  #define KVM_REG_S390           0x5000000000000000ULL
> +#define KVM_REG_ARM64          0x6000000000000000ULL
>
>  #define KVM_REG_SIZE_SHIFT     52
>  #define KVM_REG_SIZE_MASK      0x00f0000000000000ULL

Updates to the linux-headers/ files need to:
 * be a separate patch
 * be the result of running scripts/update-linux-headers.sh
   on an upstream mainline kernel or kvm-next kernel tree
 * include the kernel tree/commit hash in the commit message

> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index b92e00d..c96b871 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -32,6 +32,11 @@
>  #error mismatch between cpu.h and KVM header definitions
>  #endif
>
> +#ifdef TARGET_AARCH64
> +#define AARCH64_CORE_REG(x)   (KVM_REG_ARM64 | KVM_REG_SIZE_U64 | \
> +                 KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(x))
> +#endif
> +
>  const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
>      KVM_CAP_LAST_INFO
>  };
> @@ -50,6 +55,33 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>      return cpu->cpu_index;
>  }
>
> +#ifdef TARGET_AARCH64

This set of #ifdefs is pretty messy. I suggest splitting kvm.c
into three:
  target-arm/kvm.c   -- anything common to both 32 and 64 bit
  target-arm/kvm32.c -- non-TARGET_AARCH64 functions
  target-arm/kvm64.c -- TARGET_AARCH64 functions

and have target-arm/Makefile.objs build only one of kvm32.c
or kvm64.c depending on whether TARGET_AARCH64 is set.

> +    /* Find an appropriate target CPU type.
> +     * KVM does not provide means to detect the host CPU type on aarch64,
> +     * and simply refuses to initialize, if the CPU type mis-matches;
> +     * so we try each possible CPU type on aarch64 before giving up! */
> +    for (i = 0; i < KVM_ARM_NUM_TARGETS; ++i) {
> +        init.target = kvm_arm_targets[i];
> +        ret = kvm_vcpu_ioctl(cs, KVM_ARM_VCPU_INIT, &init);
> +        if (!ret)
> +            break;
> +    }

We definitely don't want to do this -- see my notes on
'-cpu host' in another email thread.  (We should make sure
we coordinate who of you or Linaro is going to do the
-cpu host support, incidentally.)

-- PMM