Re: [PATCH v2 2/6] KVM-HV: KVM Steal time implementation

Avi Kivity
2011-01-30T13:13:27Z

On 01/28/2011 09:52 PM, Glauber Costa wrote:
> To implement steal time, we need the hypervisor to pass the guest information
> about how much time was spent running other processes outside the VM.
> This is per-vcpu, and using the kvmclock structure for that is an abuse
> we decided not to make.
>
> In this patchset, I am introducing a new msr, KVM_MSR_STEAL_TIME, that
> holds the memory area address containing information about steal time
>
> This patch contains the hypervisor part for it. I am keeping it separate from
> the headers to facilitate backports to people who wants to backport the kernel
> part but not the hypervisor, or the other way around.
>
>
> @@ -1528,16 +1528,23 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 data)
> vcpu->arch.time_page =
> gfn_to_page(vcpu->kvm, data>> PAGE_SHIFT);
>
> - if (is_error_page(vcpu->arch.time_page)) {
> - kvm_release_page_clean(vcpu->arch.time_page);
> - vcpu->arch.time_page = NULL;
> - }
> break;
> }

Unrelated?

> @@ -2106,6 +2120,25 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> kvm_migrate_timers(vcpu);
> vcpu->cpu = cpu;
> }
> +
> + if (vcpu->arch.this_time_out) {
> + u64 to = (get_kernel_ns() - vcpu->arch.this_time_out);
> + /*
> + * using nanoseconds introduces noise, which accumulates easily
> + * leading to big steal time values. We want, however, to keep the
> + * interface nanosecond-based for future-proofness.
> + */
> + to /= NSEC_PER_USEC;
> + to *= NSEC_PER_USEC;

Seems there is a real problem and that this is just papering it over.
I'd like to understand the root cause.

> + vcpu->arch.time_out += to;
> + kvm_write_guest(vcpu->kvm, (gpa_t)&st->steal,
> + &vcpu->arch.time_out, sizeof(st->steal));

Error check.

> + vcpu->arch.sversion += 2;

Doesn't survive live migration. You need to use the version from the
guest area.

> + kvm_write_guest(vcpu->kvm, (gpa_t)&st->version,
> + &vcpu->arch.sversion, sizeof(st->version));
> + /* is it possible to have 2 loads in sequence? */
> + vcpu->arch.this_time_out = 0;
> + }
> }
>

--
error compiling committee.c: too many arguments to function