From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34469) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vv7Yo-0005fs-JL for qemu-devel@nongnu.org; Mon, 23 Dec 2013 10:36:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vv7Yi-0007wM-4A for qemu-devel@nongnu.org; Mon, 23 Dec 2013 10:36:50 -0500 Received: from cantor2.suse.de ([195.135.220.15]:41565 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vv7Yh-0007wB-Qz for qemu-devel@nongnu.org; Mon, 23 Dec 2013 10:36:44 -0500 Message-ID: <52B85887.7050008@suse.de> Date: Mon, 23 Dec 2013 16:36:39 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <18be99663ab938112a7ce805d33f069d411f4c0a.1387787208.git.chen.fan.fnst@cn.fujitsu.com> In-Reply-To: <18be99663ab938112a7ce805d33f069d411f4c0a.1387787208.git.chen.fan.fnst@cn.fujitsu.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Fan , qemu-devel@nongnu.org Cc: Igor Mammedov Am 23.12.2013 10:04, schrieb Chen Fan: > This motion is preparing for refactoring vCPU apic subsequently. >=20 > Signed-off-by: Chen Fan > --- > cpu-exec.c | 2 +- > cpus.c | 5 ++--- > hw/i386/kvmvapic.c | 8 +++----- > hw/i386/pc.c | 17 ++++++++--------- > target-i386/cpu-qom.h | 4 ++++ > target-i386/cpu.c | 22 ++++++++++------------ > target-i386/cpu.h | 4 ---- > target-i386/helper.c | 9 ++++----- > target-i386/kvm.c | 23 ++++++++++------------- > target-i386/misc_helper.c | 8 ++++---- > 10 files changed, 46 insertions(+), 56 deletions(-) >=20 > diff --git a/cpu-exec.c b/cpu-exec.c > index 30cfa2a..2711c58 100644 > --- a/cpu-exec.c > +++ b/cpu-exec.c > @@ -320,7 +320,7 @@ int cpu_exec(CPUArchState *env) > #if !defined(CONFIG_USER_ONLY) > if (interrupt_request & CPU_INTERRUPT_POLL) { > cpu->interrupt_request &=3D ~CPU_INTERRUPT_POL= L; > - apic_poll_irq(env->apic_state); > + apic_poll_irq(x86_env_get_cpu(env)->apic_state= ); These are starting to become too many inline usages inside that double loop, I'll look into providing a follow-up patch to clean this up. > } > #endif > if (interrupt_request & CPU_INTERRUPT_INIT) { [...] > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index e9831ca..d000995 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -172,13 +172,14 @@ void cpu_smm_update(CPUX86State *env) > int cpu_get_pic_interrupt(CPUX86State *env) > { > int intno; > + X86CPU *cpu =3D x86_env_get_cpu(env); I've swapped these two lines to keep cpu and env close together, with a view to a function argument type change. > =20 > - intno =3D apic_get_interrupt(env->apic_state); > + intno =3D apic_get_interrupt(cpu->apic_state); > if (intno >=3D 0) { > return intno; > } > /* read the irq from the PIC */ > - if (!apic_accept_pic_intr(env->apic_state)) { > + if (!apic_accept_pic_intr(cpu->apic_state)) { > return -1; > } > =20 [...] > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h > index f4fab15..775c82d 100644 > --- a/target-i386/cpu-qom.h > +++ b/target-i386/cpu-qom.h > @@ -66,6 +66,10 @@ typedef struct X86CPU { > =20 > CPUX86State env; > =20 > + /* in order to simplify APIC support, we leave this pointer to the > + user */ > + struct DeviceState *apic_state; Moving this further down since used as a child<> property, with a view to refactoring this further into a non-pointer field. > + > bool hyperv_vapic; > bool hyperv_relaxed_timing; > int hyperv_spinlock_attempts; [...] > diff --git a/target-i386/helper.c b/target-i386/helper.c > index 7c196ff..f2e76ad 100644 > --- a/target-i386/helper.c > +++ b/target-i386/helper.c > @@ -1248,7 +1248,8 @@ void cpu_report_tpr_access(CPUX86State *env, TPRA= ccess access) > } else { > cpu_restore_state(env, env->mem_io_pc); > =20 > - apic_handle_tpr_access_report(env->apic_state, env->eip, acces= s); > + apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state= , > + env->eip, access); > } > } > #endif /* !CONFIG_USER_ONLY */ [snip] Since we would now be using x86_env_get_cpu() in both arms of 'if' (and tpr_access_type being another candidate for a field movement), I'm changing this as follows: diff --git a/target-i386/helper.c b/target-i386/helper.c index f2e76ad..8132ca8 100644 --- a/target-i386/helper.c +++ b/target-i386/helper.c @@ -1241,15 +1241,16 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank, void cpu_report_tpr_access(CPUX86State *env, TPRAccess access) { + X86CPU *cpu =3D x86_env_get_cpu(env); + if (kvm_enabled()) { env->tpr_access_type =3D access; - cpu_interrupt(CPU(x86_env_get_cpu(env)), CPU_INTERRUPT_TPR); + cpu_interrupt(CPU(cpu), CPU_INTERRUPT_TPR); } else { cpu_restore_state(env, env->mem_io_pc); - apic_handle_tpr_access_report(x86_env_get_cpu(env)->apic_state, - env->eip, access); + apic_handle_tpr_access_report(cpu->apic_state, env->eip, access)= ; } } #endif /* !CONFIG_USER_ONLY */ Despite this still being an RFC, this patch is a really nice cleanup contribution, so I'm applying this to qom-cpu already with the above-mentioned modifications: https://github.com/afaerber/qemu-cpu/commits/qom-cpu Thanks, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg