linux-kernel - Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4E1B031C.1000604@redhat.com>
Date:	Mon, 11 Jul 2011 11:05:16 -0300
From:	Glauber Costa <glommer@...hat.com>
To:	Avi Kivity <avi@...hat.com>
CC:	Marcelo Tosatti <mtosatti@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, Rik van Riel <riel@...hat.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Anthony Liguori <aliguori@...ibm.com>,
	Eric B Munson <emunson@...bm.net>
Subject: Re: [PATCH v5 4/9] KVM-HV: KVM Steal time implementation

On 07/11/2011 09:58 AM, Avi Kivity wrote:
> On 07/07/2011 08:07 PM, Glauber Costa wrote:
>>>> +static void record_steal_time(struct kvm_vcpu *vcpu)
>>>> +{
>>>> + u64 delta;
>>>> +
>>>> + if (!(vcpu->arch.st.msr_val& KVM_MSR_ENABLED))
>>>> + return;
>>>> +
>>>> + if (unlikely(kvm_read_guest_cached(vcpu->kvm,&vcpu->arch.st.stime,
>>>> + &vcpu->arch.st.steal, sizeof(struct kvm_steal_time))))
>>>> + return;
>>>
>>> The guest memory page is not pinned, sleeping via
>>> __copy_from_user/to_user is not allowed in vcpu_load context. Either pin
>>> it or use atomic acessors.
>>
>>
>> I do recognize the problem.
>> Avi, what's your take here?
>>
>
> The easiest solution is to set a KVM_REQ bit in atomic context, and move
> the sleepy code to vcpu_enter_guest().

Or I can move it all inside vcpu_run, or close enough to it. This will 
account more hypervisor time as steal time, but it seemed to be what 
some people wanted in the first place.

Given the simplification we would win - not needing a REQ set, it might 
be worth it.

>>>> + case MSR_KVM_STEAL_TIME:
>>>> + vcpu->arch.st.msr_val = data;
>>>> +
>>>> + if (!(data& KVM_MSR_ENABLED)) {
>>>> + break;
>>>> + }
>>>
>>> On failure below this point, msr_val should be cleared of
>>> KVM_MSR_ENABLED?
>> No, msr_val has to hold whatever the guest wrote into it.
>> We should probably use an independent variable here to indicate that
>> we failed to activate it.
>
> If we fail, we return a #GP to the guest (and don't write any value into
> the msr).
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/