Re: [Xen-devel] [PATCH v8 10/11] viridian: add implementation of synthetic timers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.03.19 at 16:46, <Paul.Durrant@xxxxxxxxxx> wrote:
>> > > +    {
>> > > +        expiration = vs->count;
>> > > +        if ( expiration - now <= 0 )
>> > > +        {
>> > > +            vs->expiration = expiration;
>> > > +            stimer_expire(vs);
>> >
>> > Aren't you introducing a risk for races by calling the timer function
>> > directly from here? start_timer(), after all, gets called from quite a
>> > few places.
>> 
>> In practice I don't think there should be any problematic race, but I'll 
>> check again.
> 
> I think the 'periodic' name might be confusing things... The Xen timers are 
> all single-shot, it's just that start_stimer() is re-called after a 
> successful poll if the viridian timer is configured to be periodic. So I 
> don't think there is case where the underlying Xen timer could actually be 
> running when we enter start_stimer().

One of the callers of the function is the WRMSR handler. Why would
it be guaranteed that the timer isn't active when such a WRMSR
occurs? Of course, this alone is not enough for there to be a problem,
as we're fine as long as the guest can only harm itself. But I don't
think it goes without saying that there's no issue here; having looked
a 2nd time just now, I think I agree though that there's no risk for our
own health here.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel