On Mon, 2013-12-23 at 16:36 +0100, Andreas Färber wrote: > Am 23.12.2013 10:04, schrieb Chen Fan: > > This motion is preparing for refactoring vCPU apic subsequently. > > > > Signed-off-by: Chen Fan <[email protected]> > > --- > > 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(-) > > > > 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 &= ~CPU_INTERRUPT_POLL; > > - 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 = 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. > > > > > - intno = apic_get_interrupt(env->apic_state); > > + intno = apic_get_interrupt(cpu->apic_state); > > if (intno >= 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; > > } > > > [...] > > 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 { > > > > CPUX86State env; > > > > + /* 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, > > TPRAccess access) > > } else { > > cpu_restore_state(env, env->mem_io_pc); > > > > - apic_handle_tpr_access_report(env->apic_state, env->eip, access); > > + 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 = x86_env_get_cpu(env); > + > if (kvm_enabled()) { > env->tpr_access_type = 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
looks nice, Thanks. Chen > Thanks, > Andreas >
