Re: [PATCH] release kvmclock page on reset

[Glauber Costa <glommer@redhat.com>]

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.