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

From: "Jan Beulich" <JBeulich@suse.com>
To: Paul Durrant <paul.durrant@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>,
	george.dunlap@citrix.com, Julien Grall <julien.grall@arm.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Ian Jackson <Ian.Jackson@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH v8 10/11] viridian: add implementation of synthetic timers
Date: Mon, 18 Mar 2019 10:22:34 -0600	[thread overview]
Message-ID: <5C8FC5CA020000780021FF9A@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <6001528cc0ae46f481f156802d8bcdd4@AMSPEX02CL02.citrite.net>

>>> On 18.03.19 at 16:46, <Paul.Durrant@citrix.com> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel