Glauber de Oliveira Costa wrote:
> My next TODOs with it are:
> * Get SMP working
> * Try something for stolen time, as jeremy's last suggestion for anthony's
> patch
> * 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.
In theory you can mitigate this by using an absolute rather than
relative timeout value, but in practice I don't think it makes much
difference.
> +
> +/*
> + * This is our read_clock function. The host puts an tsc timestamp each time
> + * it updates a new time, and then we can use it to derive a slightly more
> + * precise notion of elapsed time, converted to nanoseconds.
> + *
> + * 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?
> + */
> +static cycle_t kvm_clock_read(void) {
> +
> + u64 delta, last_tsc;
> + struct timespec *now;
> +
> + if (hv_clock.stable_tsc) {
> + rdtscll(last_tsc);
> + return last_tsc;
>
So this returns a tsc here? ---->
> + }
> +
> + 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?
> + 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.
> +
> + delta = native_read_tsc() - last_tsc;
> + delta = (delta * hv_clock.tsc_mult) >> KVM_SCALE;
> +
> + return (cycle_t)now->tv_sec * NSEC_PER_SEC + now->tv_nsec + delta;
>
---> But returns ns here?
> +}
> +
> +static void kvm_timer_set_mode(enum clock_event_mode mode,
> + struct clock_event_device *evt)
> +{
> + WARN_ON(!irqs_disabled());
> +
> + switch (mode) {
> + case CLOCK_EVT_MODE_ONESHOT:
> + /* this is what we want */
> + break;
> + case CLOCK_EVT_MODE_RESUME:
> + break;
> + case CLOCK_EVT_MODE_PERIODIC:
> + WARN_ON(1);
> + break;
> + case CLOCK_EVT_MODE_UNUSED:
> + case CLOCK_EVT_MODE_SHUTDOWN:
> + kvm_hypercall0(KVM_HCALL_STOP_ONESHOT);
> + break;
> + default:
> + break;
> + }
> +}
> +
> +/*
> + * Programming the next event is just a matter of asking the host
> + * to generate us an interrupt when the time expires. We pass the
> + * delta on, and hypervisor will do all remaining tricks. For a more
> + * precise timing, we can just subtract the time spent by the hypercall
>
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.
> + */
> +static int kvm_timer_next_event(unsigned long delta,
> + struct clock_event_device *evt)
> +{
> + WARN_ON(evt->mode != CLOCK_EVT_MODE_ONESHOT);
> + kvm_hypercall1(KVM_HCALL_SET_ALARM, delta);
> + return 0;
> +}
> +
> diff --git a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c
> index d474cd6..fd758f9 100644
> --- a/arch/i386/kernel/setup.c
> +++ b/arch/i386/kernel/setup.c
> @@ -46,6 +46,7 @@
> #include <linux/crash_dump.h>
> #include <linux/dmi.h>
> #include <linux/pfn.h>
> +#include <linux/kvm_para.h>
>
> #include <video/edid.h>
>
> @@ -579,6 +580,9 @@ void __init setup_arch(char **cmdline_p)
> vmi_init();
> #endif
>
> +#ifdef CONFIG_KVM_CLOCK
> + kvmclock_init();
> +#endif
>
Why is this necessary? Can't you hook one of the existing pvops?
J
-------------------------------------------------------------------------
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
