On 16.07.2012, at 09:19, Rusty Russell wrote:
> On Fri, 13 Jul 2012 12:06:26 +0200, Alexander Graf <agraf at suse.de> wrote:
>>> 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.
>
> Since each arch is given a hook already, I just abused it. I'll change
> this to a fresh KVM_ARM_SET_TARGET ioctl.
>
>> 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.
>
> Hmm, it's not quite a clean fit: this bitmap is for guest features, not
> kvm ones. Which ones you can enable depends on the target CPU, at least
> in theory.
Sure. You'd do (pseudo-code):
kvm_ioctl(fd, KVM_ENABLE_CAP, KVM_CAP_ARM_CPU_TARGET, ARM_CPU_NEON | ARM_CPU_VFP16 | ARM_CPU_V7);
>
> eg. FP/NEON support and debug register support are (in theory) optional
> features for an implementation. There may be more in future, I guess.
>
> And you really want to initialize this all at the same time; eg. the cpu
> identification registers need to be initialized depending on the
> presence of various features. It's also possible that various features
> may be related, so you can't turn a single one off at a time.
That argument does hold true. We are in that mess with ppc, where we don't have a proper "init" ioctl. So you'd definitely be better off defining an extensible init handshake now, so you can easily configure the kernel side early on.
Currently for PPC, we just try to either make things stateless or put additional constraints on certain CAPs, like "may only be set before the first vcpu run". Not as pretty as doing it cleanly.
> Currently, it's a bit theoretical, since we don't have any guest
> features, but it was suggested that we'll want them in future.
Yes, hence I'm trying to get you guys to something where you won't be stuck in 6 months from now with a stable ABI that totally doesn't fit you :)
Alex
