From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NFmlC-0003Kd-6I for qemu-devel@nongnu.org; Wed, 02 Dec 2009 05:48:38 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NFml6-0003IK-R0 for qemu-devel@nongnu.org; Wed, 02 Dec 2009 05:48:37 -0500 Received: from [199.232.76.173] (port=42596 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NFml6-0003IH-Jo for qemu-devel@nongnu.org; Wed, 02 Dec 2009 05:48:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45973) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NFml6-0005qW-Ae for qemu-devel@nongnu.org; Wed, 02 Dec 2009 05:48:32 -0500 Date: Wed, 2 Dec 2009 12:48:26 +0200 From: Gleb Natapov Subject: Re: [Qemu-devel] [PATCH v2 05/11] tell kernel about all registers instead of just mp_state Message-ID: <20091202104826.GA9537@redhat.com> References: <1259671897-22232-1-git-send-email-glommer@redhat.com> <1259671897-22232-2-git-send-email-glommer@redhat.com> <1259671897-22232-3-git-send-email-glommer@redhat.com> <1259671897-22232-4-git-send-email-glommer@redhat.com> <1259671897-22232-5-git-send-email-glommer@redhat.com> <1259671897-22232-6-git-send-email-glommer@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1259671897-22232-6-git-send-email-glommer@redhat.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Glauber Costa Cc: agraf@suse.de, aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com On Tue, Dec 01, 2009 at 10:51:31AM -0200, Glauber Costa wrote: > This fix a bug with -smp in kvm. Since we have updated apic_base, > we also have to tell kernel about it. So instead of just updating > mp_state, update every regs. > > It is mandatory that this happens synchronously, without waiting for > the next vcpu run. Otherwise, if we are migrating, or initializing > the cpu's APIC, other cpus can still see an invalid state. > > Since putting registers already happen in vcpu entry, we factor > out the required code in cpu_flush_state() > > Signed-off-by: Glauber Costa > --- > hw/apic-kvm.c | 5 ++++- > kvm-all.c | 14 +++++++++----- > kvm.h | 8 ++++++++ > 3 files changed, 21 insertions(+), 6 deletions(-) > > diff --git a/hw/apic-kvm.c b/hw/apic-kvm.c > index e5a0bfc..9e9790f 100644 > --- a/hw/apic-kvm.c > +++ b/hw/apic-kvm.c > @@ -126,7 +126,10 @@ static void kvm_apic_reset(void *opaque) > s->cpu_env->mp_state > = bsp ? KVM_MP_STATE_RUNNABLE : KVM_MP_STATE_UNINITIALIZED; > > - kvm_put_mp_state(s->cpu_env); > + /* We have to tell the kernel about mp_state, but also save sregs, since > + * apic base was just updated > + */ > + cpu_flush_state(s->cpu_env); > > if (bsp) { > /* > diff --git a/kvm-all.c b/kvm-all.c > index 40203f0..318a4e6 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -622,7 +622,6 @@ static void kvm_run_coalesced_mmio(CPUState *env, struct kvm_run *run) > } > #endif > } > - Spurious line removing. > void kvm_cpu_synchronize_state(CPUState *env) > { > if (!env->kvm_state->regs_modified) { > @@ -631,6 +630,14 @@ void kvm_cpu_synchronize_state(CPUState *env) > } > } > > +void kvm_cpu_flush_state(CPUState *env) > +{ > + if (env->kvm_state->regs_modified) { > + kvm_arch_put_registers(env); > + env->kvm_state->regs_modified = 0; > + } > +} > + > int kvm_cpu_exec(CPUState *env) > { > struct kvm_run *run = env->kvm_run; > @@ -645,10 +652,7 @@ int kvm_cpu_exec(CPUState *env) > break; > } > > - if (env->kvm_state->regs_modified) { > - kvm_arch_put_registers(env); > - env->kvm_state->regs_modified = 0; > - } > + kvm_cpu_flush_state(env); > > kvm_arch_pre_run(env, run); > qemu_mutex_unlock_iothread(); > diff --git a/kvm.h b/kvm.h > index a474d95..d9af176 100644 > --- a/kvm.h > +++ b/kvm.h > @@ -139,6 +139,7 @@ int kvm_check_extension(KVMState *s, unsigned int extension); > uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function, > int reg); > void kvm_cpu_synchronize_state(CPUState *env); > +void kvm_cpu_flush_state(CPUState *env); > > /* generic hooks - to be moved/refactored once there are more users */ > > @@ -149,4 +150,11 @@ static inline void cpu_synchronize_state(CPUState *env) > } > } > > +static inline void cpu_flush_state(CPUState *env) > +{ > + if (kvm_enabled()) { Is this ever called or intended to be called when kvm is disabled? > + kvm_cpu_flush_state(env); > + } > +} > + > #endif > -- > 1.6.5.2 > > -- Gleb.