[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from C
From: |
Chen Fan |
Subject: |
Re: [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU |
Date: |
Tue, 24 Dec 2013 10:08:49 +0800 |
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 <address@hidden>
> > ---
> > 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
>
- [Qemu-devel] [RFC qom-next v5 0/8] i386: add cpu hot remove support, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 1/8] x86: move apic_state field from CPUX86State to X86CPU, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 2/8] x86: add x86_cpu_unrealizefn() for cpu apic remove, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 8/8] cpus: reclaim allocated vCPU objects, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 3/8] qmp: add 'cpu-del' command support, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 4/8] qom cpu: rename variable 'cpu_added_notifier' to 'cpu_hotplug_notifier', Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 7/8] piix4: implement function cpu_status_write() for vcpu ejection, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 6/8] i386: implement pc interface cpu_common_unrealizefn() in qom/cpu.c, Chen Fan, 2013/12/23
- [Qemu-devel] [RFC qom-next v5 5/8] qom cpu: add UNPLUG cpu notifier support, Chen Fan, 2013/12/23