On 09/08/2013 15:24, Peter Maydell wrote:
> On 23 July 2013 10:33, Mian M. Hamayun <m.hamayun@xxxxxxxxxxxxxxxxxxxxxx> wrote:
>> From: "Mian M. Hamayun" <m.hamayun@xxxxxxxxxxxxxxxxxxxxxx>
>>
>> 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 <m.hamayun@xxxxxxxxxxxxxxxxxxxxxx>
>>
>> Conflicts:
>>
>> target-arm/kvm.c
>>
>> Conflicts:
>>
>> target-arm/cpu.c
> This sort of Conflicts note shouldn't be in a commit message.
Agreed, will remove it in the next revision.
>
>> ---
>> 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
Agreed, will do it like this.
>
>> 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.
My initial intuition was to separate 32 and 64 bit versions, but as
many functions are common, so I decided to use #ifdefs instead.
btw, I have a similar situation in hw/arm/boot.c as well.
>
>> + /* 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.)
I tend to agree with this proposition as well. But the point that I don't
understand is how something is acceptable in kvmtool and not in the
qemu. If this implementation lets us use the 64-bit architecture in the
current state of affairs, then why not use it. We can always replace
this particular part, when the -cpu host support becomes available, right ?
Hamayun
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
