On 10/12/07, Jeremy Fitzhardinge <[EMAIL PROTECTED]> wrote: > > * Measure the time it takes for a hypercall, and subtract this time > > for calculating the expiry time for the timer event. > > > > I don't think there's much point in trying to do stuff like this. The > guest can be preempted at any time, so there's an arbitrary amount of > time between deciding to set a timeout, and the time the timeout > actually happens.
Yes, we can't be precise. But my point here is just being _more_ accurate whenever possible. Yes, it might have been preempted, we don't know how much time it spent out, but at least this time has elapsed. It's a win in some scenarios, without too much overhead but an initial measure and a summation. Even with tsc adjustments to wallclock, and clock read, I'm using it regardless of the host having accurate tsc. My main point behind it is that it will, accurate or not, be _more_ accurate than the simple version. Of course, we can discuss if it will really make a difference. > > + * If the platform provides a stable tsc, we just use it, and there is no > > need > > + * for the host to update anything. > > > How would you deal with suspend/resume/migrate? Also, do you assume > that stable_tsc also means synchronized tsc on an SMP host? I have to think further on that. But for all cases , I think that the host knows what happened, and can adjust the timer accordingly. About sync tsc, this code shouldn't be making this assumption, but right now... it is. I'll think further. > So this returns a tsc here? ----> [ .. snip .. ] > ---> But returns ns here? Yes. But if you look below, (unless I've really forgot to include in the patch I fired out), I change the multiplier in case we're using a tsc value. We multiply by one if using nanosecond timer, and by a host provided multiplier if tsc timer. > > > + } > > + > > + do { > > + last_tsc = hv_clock.last_tsc; > > + rmb(); > > + now = &hv_clock.now; > > > > Shouldn't this be taking a copy of now, rather than a pointer to it? > Otherwise what's the point of this loop? Yes, thanks. > > + rmb(); > > + } while (hv_clock.last_tsc != last_tsc); > > > > This won't be an atomic compare on 32-bit; it could get confused by > seeing a half-updated tsc value. You are right, it might be better using a version number like xen does, then. humm... Rusty's approach of checking against seconds, and then using nanoseconds only if it has not yet changed may work... but maybe not as well, due to the tsc adjustments. Right now, I'm more inclined to the first option. > > > > Not worthwhile. It would be better to make the hypercall take an > absolute time, and pass it now+delta. At least then if you get > preempted past the timeout period you can return -ETIME, and the clock > subsystem will know what to do. Since the host and guest times are synchronized by assumption, it might work. I'll give it a try. > > +#ifdef CONFIG_KVM_CLOCK > > + kvmclock_init(); > > +#endif > > > > Why is this necessary? Can't you hook one of the existing pvops? Well, as you all have seen, I've just arrived from weekend, just took an overlook over all the answers, but I think anthony already answered this one: Someone has to override the pvops. However, I think I can do better on this, doing what I did for vsmp on x86_64, by using ARCH_INIT to check for availability. To be honest, I considered both approaches, but saw no clear benefit in any, against the other, and did this way just because vmi is already there. But yeah, it seems to have some special requirements for placement, that we do not. If you feel more confortable with it, I can move it to ARCH_INIT. > > J > -- Glauber de Oliveira Costa. "Free as in Freedom" http://glommer.net "The less confident you are, the more serious you have to act." ------------------------------------------------------------------------- This SF.net email is sponsored by: Splunk Inc. Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/ _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel