Glauber de Oliveira Costa wrote:
void kvm_inject_pending_timer_irqs(struct kvm_vcpu *vcpu)
{
+ vcpu->time_needs_update = 1;
Why here and not in __vcpu_run()? It isn't timer irq related.
Because my plan was exactly, updating it at each timer interrupt.
I think kvm_inject_pending_timer_irqs() is called every __vcpu_run(), so
your cunning plan has been foiled.
Did you mean each guest interrupt of host interrupt?
There's a trade off between
updating every run (hopefully more precision, but more overhead), versus
updating at timer irqs, or other events.
What would you prefer?
I think that we should update it every time a heavyweight exit has been
taken. That takes care of the tradeoff quite nicely -- heavyweight
exits are already dog slow.
+ /* Updating the tsc count is the first thing we do */
+ kvm_get_msr(vcpu, MSR_IA32_TIME_STAMP_COUNTER, &vcpu->hv_clock.last_tsc);
+ ktime_get_ts(&ts);
+ vcpu->hv_clock.now_ns = ts.tv_nsec + (NSEC_PER_SEC * (u64)ts.tv_sec);
+ vcpu->hv_clock.wc_sec = get_seconds();
+ vcpu->hv_clock.version++;
+
+ clock_addr = vcpu->clock_addr;
+ memcpy(clock_addr, &vcpu->hv_clock, sizeof(vcpu->hv_clock));
+ mark_page_dirty(vcpu->kvm, vcpu->clock_gfn);
Just use kvm_write_guest().
Too slow. Updating guest time, even only in timer interrupts, was a too
frequent operation, and the kmap / kunmap (atomic) at every iteration
deemed the whole thing
unusable.
kvm_write_guest() will eventually be a copy_to_user(), so you need not
fear the overhead.
+ ret = 0;
switch (nr) {
+ case KVM_HCALL_REGISTER_CLOCK: {
+ struct kvm_vcpu *dst_vcpu;
+
+ if (!((a1 < KVM_MAX_VCPUS) && (vcpu->kvm->vcpus[a1]))) {
+ ret = -KVM_EINVAL;
+ break;
+ }
+
+ dst_vcpu = vcpu->kvm->vcpus[a1];
What if !dst_vcpu? What about locking?
Suggest simply using vcpu. Every guest cpu can register its own
Earlier version had a check for !dst_vcpu, you are absolutely right.
Locking was not a problem in practice, because these operations are done
serialized, by the same cpu.
Think evil guest that cares not for the well-being of the host.
This hypercall is called by cpu_up, which, at least in the beginning,
it's called by cpu0. And that's why each vcpu cannot register its own.
(And why we don't need locking).
Well, theorectically each vcpu do can register its own clocksource, it
will just be a little bit more complicated, we have to fire out an IPI,
and have the other cpu to catch it, and call the hypercall.
Can it not be done via the processor startup sequence? Then there's no
need for ipis and locking.
I imagine a normal guest initializes the apic in the same way.
But I honestly don't like it.
Usually, the cpu leaves start_secondary with a clock already registered,
so the kernel relies on it.
+ dst_vcpu->clock_page = gfn_to_page(vcpu->kvm, a0 >> PAGE_SHIFT);
Shift right? Why?
a0 is not a gfn, but a physical address.
What if the guest wants to place it in address 5GB? That's unlikely for
Linux and Windows, but let's do it right anyway.
--
error compiling committee.c: too many arguments to function
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
