lkml.org 
[lkml]   [2011]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH] release kvmclock page on reset
    From
    Date
    On Sat, 2011-01-29 at 09:56 +0100, Jan Kiszka wrote:
    > On 2011-01-29 03:07, Glauber Costa wrote:
    > > On Fri, 2011-01-28 at 22:09 +0100, Jan Kiszka wrote:
    > >> On 2011-01-28 20:48, Glauber Costa wrote:
    > >>> Up to know, we were relying on guest cooperation to turn off kvmclock.
    > >>> I just realized that even though this is fine and nice, a more robust
    > >>> method is to (also) turn it off on vcpu_reset on the hypervisor side.
    > >>> This will protect us against reboots, and we don't expect the guest
    > >>> to reset its cpu during normal operation anyway.
    > >>>
    > >>> Signed-off-by: Glauber Costa <glommer@redhat.com>
    > >>> ---
    > >>> arch/x86/kvm/x86.c | 5 +++++
    > >>> 1 files changed, 5 insertions(+), 0 deletions(-)
    > >>>
    > >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
    > >>> index bcc0efc..38b55b3 100644
    > >>> --- a/arch/x86/kvm/x86.c
    > >>> +++ b/arch/x86/kvm/x86.c
    > >>> @@ -5878,6 +5878,11 @@ int kvm_arch_vcpu_reset(struct kvm_vcpu *vcpu)
    > >>> kvm_make_request(KVM_REQ_EVENT, vcpu);
    > >>> vcpu->arch.apf.msr_val = 0;
    > >>>
    > >>> + if (vcpu->arch.time_page) {
    > >>> + kvm_release_page_dirty(vcpu->arch.time_page);
    > >>> + vcpu->arch.time_page = NULL;
    > >>> + }
    > >>> +
    > >>
    > >> kvm_arch_vcpu_reset is only called on vcpu setup and when it receives a
    > >> sipi (provided in-kernel irqchip is in use). If you want this page to be
    > >> consistently reset on guest reboot, you have to trigger this from user
    > >> space. But I thought we are doing this already in qemu, don't we?
    > >
    > > Humm, you might as well be right regarding reboots.
    > > But in the end, it doesn't affect correctness here. If we're resetting
    > > the vcpu, we should not let that kind of data live.
    > >
    >
    > Right, just checked that we reset other states like nmi_pending or
    > async_pf here as well. So doing the same for the time_page looks
    > appropriate.
    >
    > But I think you should encapsulate the pattern above in a function and
    > substitute other occurrences at this chance. Also, the changelog should
    > clarify in which cases the code matters.

    Fair. I will respin this today.

    > Jan
    >




    \
     
     \ /
      Last update: 2011-02-01 17:07    [W:0.041 / U:0.136 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site