[Xen-devel] Re: [PATCH] Allow guest to register a secondary vcpu_time_info structure

[Jeremy Fitzhardinge <jeremy@xxxxxxxx>]

On 10/06/09 00:10, Keir Fraser wrote:
> On 06/10/2009 01:10, "Jeremy Fitzhardinge" <jeremy@xxxxxxxx> wrote:
>
>   
>> The second vcpu_time_info structure is updated by copy, rather than
>> being a shared page between the guest and Xen.  It is not directly
>> updated by copy; instead, Xen preserves and increments the existing
>> version number in place.  This allows the guest to also update the
>> version number (useful to indicate vcpu context switches to usermode).
>> This assumes that the guest will only ever update the structure for a
>> given vcpu on that vcpu (as Xen does, so there are never any cross-cpu
>> accesses).
>>     
> How do you feel about the 32-bit version number? We have space to easily
> expand it to 64 bits, and now would be the time to do it while we're
> extending the ABI.
>   

I think 32 bit (well, 31) is fine:

The version number doesn't increment that often, so I think it would
take a long time to wrap.  The only failure mode is if the version wraps
and gets an aliased value between the "get version" and "re-test
version" phases of the algorithm, which is only a few tens of
instructions at most.  I would think it unlikely the version increments
once in that period, let alone 2^31 times.  As far as I know, nobody is
keeping long-term copies of the time info.

(One unlikely scenario is that you have usermode in the middle of a
vsyscall gettimeofday when the domain gets suspended/migrated and it is
suspended for some long period of time.  But this is OK because the
version gets saved along with the rest of the domain and incremented
from there.  I'm not sure if the same applies to the shared_info
version, but that's less of a concern because the kernel will never
preempt itself doing a clocksource_read to suspend.)

Aside from that, this isn't really an ABI extension.  The registration
of the secondary vcpu_time_info is a new ABI, but from there it is
identical to the existing pvclock ABI; I use the same code for both the
kernel and in the vsyscall page, so making them different would be
awkward (of course I could change the kernel to use the "secondary" time
values too, but it would still need to support old hypervisors so it
wouldn't save anything).  KVM is also using this ABI, so I wouldn't want
to introduce an arbitrary ABI change.

Also, a >32 bit version field would be hard for 32 bit guests to deal
with; it's not easy to either read or update its value in a single
instruction. (32 bit Xen too, but I guess we're less concerned about that.)

    J

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel