Re: [PATCH v2 11/17] kvm: x86: Reset paravirtual MSRs

Glauber Costa
2011-01-03T16:51:09Z

On Mon, 2011-01-03 at 17:46 +0100, Jan Kiszka wrote:
> Am 03.01.2011 17:40, Glauber Costa wrote:
> > On Mon, 2011-01-03 at 09:33 +0100, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Make sure to clear MSR_KVM_SYSTEM_TIME, MSR_KVM_WALL_CLOCK, and
> >> MSR_KVM_ASYNC_PF_EN so that a freshly booted guest cannot be disturbed
> >> by old values.
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> CC: Glauber Costa <glommer@redhat.com>
> >> ---
> >> target-i386/kvm.c | 10 ++++++++++
> >> 1 files changed, 10 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >> index d8f26bf..664a4a0 100644
> >> --- a/target-i386/kvm.c
> >> +++ b/target-i386/kvm.c
> >> @@ -453,6 +453,9 @@ void kvm_arch_reset_vcpu(CPUState *env)
> >> env->nmi_injected = 0;
> >> env->nmi_pending = 0;
> >> env->xcr0 = 1;
> >> + env->system_time_msr = 0;
> >> + env->wall_clock_msr = 0;
> >> + env->async_pf_en_msr = 0;
> >
> > Have you seen this happening? I'd expect CPUState to be zeroed out over
> > init. And if it is not, I guess we should...
>
> Ah, true, those three are part of the section that is zeroed. Will drop
> that hunk on repost.
>
> Guess we should rather move some other variables in that region too and
> avoid clearing them manually like above...
>
> Jan
>
Agreed.