Re: [Qemu-devel] [kvmarm] [PATCH] target-arm: kvm: use KVM_SET_SREGS to set target to Cortex A15

[Alexander Graf]

On 13.07.2012, at 05:37, Rusty Russell <address@hidden> wrote:

> Recent kernels use this to set the cpu and features (currently, only
> the A15 is supported).
> 
> Note that this causes the registers in the CPU to be initialized, so
> it's important that all CPUs are created first (they are, as it turns
> out).
> 
> This code ignores errors, for backwards compatibility with older
> kernels.
> 
> Signed-off-by: Rusty Russell <address@hidden>
> 
> diff --git a/linux-headers/asm-arm/kvm.h b/linux-headers/asm-arm/kvm.h
> index 38ff1d6..988890a 100644
> --- a/linux-headers/asm-arm/kvm.h
> +++ b/linux-headers/asm-arm/kvm.h
> @@ -66,7 +66,13 @@ struct kvm_regs {
> 
> };
> 
> +/* Supported Processor Types */
> +#define KVM_ARM_TARGET_CORTEX_A15    (0xC0F)
> +
> struct kvm_sregs {
> +    __u32 target;
> +    __u32 num_features;
> +    __u32 features[14];
> };

Are you sure you want to use sregs? We did the mistake of reusing it on ppc, 
but that doesn't mean you need to repeat the same one :)

Basically sregs are an x86 specific struct for its segment register 
information. I'm quite sure that this is not what your use of them is here.

In general you have 3 sane options for API additions:

  1) ONE_REG

If you have information that is syncable in register granularity, this is what 
you want. We will (once necessary) add an API that allows us to sync multiple 
ONE_REG items at once, so it'll be like a big dynamic kvm_regs implementation. 
I don't think this applies here though.

2) new ioctl

Just define an ARM specific ioctl that sets the compat features. Believe me, it 
will make your code easier to read.

3) ENABLE_CAP

If you only need to enable a feature and care about backwards compatibility of 
the API (which you don't yet), this is a good one. it basically allows you to 
enable new features in newer kernel versions which would otherwise break 
compatibility. You can also pass arbitrary data to ENABLE_CAP to pass in 
additional information.


Usually in released KVM versions, I'd use ENABLE_CAP for a feature like this. 
Since you can still define and break the API as you wish, option 2 works as 
well :).

Alex

> 
> struct kvm_fpu {
> diff --git a/target-arm/kvm.c b/target-arm/kvm.c
> index 29bb51f..67d005f 100644
> --- a/target-arm/kvm.c
> +++ b/target-arm/kvm.c
> @@ -34,7 +34,13 @@ int kvm_arch_init(KVMState *s)
> 
> int kvm_arch_init_vcpu(CPUARMState *env)
> {
> -    return 0;
> +    struct kvm_sregs sregs;
> +
> +    sregs.target = KVM_ARM_TARGET_CORTEX_A15;
> +    sregs.num_features = 0;
> +
> +    /* Ignore failure for compatibility with old kvm versions. */
> +    return kvm_vcpu_ioctl(env, KVM_SET_SREGS, &sregs) ? 0 : 0;
> }
> 
> int kvm_arch_put_registers(CPUARMState *env, int level)
> _______________________________________________
> kvmarm mailing list
> address@hidden
> https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm