| Message ID | 54192ab9004ed6b528de0846d6a83df432addcd5.1294043582.git.jan.kiszka@web.de |
|---|---|
| State | New |
| Headers | show |
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...
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
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.
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; if (kvm_irqchip_in_kernel()) { env->mp_state = cpu_is_bsp(env) ? KVM_MP_STATE_RUNNABLE : KVM_MP_STATE_UNINITIALIZED; @@ -845,6 +848,13 @@ static int kvm_put_msrs(CPUState *env, int level) if (smp_cpus == 1 || env->tsc != 0) { kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc); } + } + /* + * The following paravirtual MSRs have side effects on the guest or are + * too heavy for normal writeback. Limit them to reset or full state + * updates. + */ + if (level >= KVM_PUT_RESET_STATE) { kvm_msr_entry_set(&msrs[n++], MSR_KVM_SYSTEM_TIME, env->system_time_msr); kvm_msr_entry_set(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr);