lkml.org 
[lkml]   [2007]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: kvmclock - the host part.
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-11-07 15:03    [W:0.044 / U:0.664 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site