[Qemu-devel] [PATCH 6/6] apic: avoid using CPUState internals

[Blue Swirl]

Use only an opaque CPUState pointer and move the actual CPUState
contents handling to cpu.h and cpuid.c.

Set env->halted in pc.c and add a function to get the local APIC state
of the current CPU for the MMIO.

Signed-off-by: Blue Swirl <address@hidden>
---
 hw/apic.c           |   40 +++++++++++++++-------------------------
 hw/apic.h           |    9 ++++++++-
 hw/pc.c             |   12 +++++++++++-
 target-i386/cpu.h   |   27 ++++++++++++++++-----------
 target-i386/cpuid.c |    6 ++++++
 5 files changed, 56 insertions(+), 38 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 91c8d93..332c66e 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -95,7 +95,7 @@
 #define MSI_ADDR_SIZE                   0x100000

 struct APICState {
-    CPUState *cpu_env;
+    void *cpu_env;
     uint32_t apicbase;
     uint8_t id;
     uint8_t arb_id;
@@ -320,7 +320,7 @@ void cpu_set_apic_base(APICState *s, uint64_t val)
     /* if disabled, cannot be enabled again */
     if (!(val & MSR_IA32_APICBASE_ENABLE)) {
         s->apicbase &= ~MSR_IA32_APICBASE_ENABLE;
-        s->cpu_env->cpuid_features &= ~CPUID_APIC;
+        cpu_clear_apic_feature(s->cpu_env);
         s->spurious_vec &= ~APIC_SV_ENABLE;
     }
 }
@@ -508,8 +508,6 @@ void apic_init_reset(APICState *s)
     s->initial_count_load_time = 0;
     s->next_time = 0;
     s->wait_for_sipi = 1;
-
-    s->cpu_env->halted = !(s->apicbase & MSR_IA32_APICBASE_BSP);
 }

 static void apic_startup(APICState *s, int vector_num)
@@ -524,13 +522,7 @@ void apic_sipi(APICState *s)

     if (!s->wait_for_sipi)
         return;
-
-    s->cpu_env->eip = 0;
-    cpu_x86_load_seg_cache(s->cpu_env, R_CS, s->sipi_vector << 8,
-                           s->sipi_vector << 12,
-                           s->cpu_env->segs[R_CS].limit,
-                           s->cpu_env->segs[R_CS].flags);
-    s->cpu_env->halted = 0;
+    cpu_x86_load_seg_cache_sipi(s->cpu_env, s->sipi_vector);
     s->wait_for_sipi = 0;
 }

@@ -692,15 +684,14 @@ static void apic_mem_writew(void *opaque,
target_phys_addr_t addr, uint32_t val)

 static uint32_t apic_mem_readl(void *opaque, target_phys_addr_t addr)
 {
-    CPUState *env;
     APICState *s;
     uint32_t val;
     int index;

-    env = cpu_single_env;
-    if (!env)
+    s = cpu_get_current_apic();
+    if (!s) {
         return 0;
-    s = env->apic_state;
+    }

     index = (addr >> 4) & 0xff;
     switch(index) {
@@ -782,7 +773,6 @@ static void apic_send_msi(target_phys_addr_t addr,
uint32 data)

 static void apic_mem_writel(void *opaque, target_phys_addr_t addr,
uint32_t val)
 {
-    CPUState *env;
     APICState *s;
     int index = (addr >> 4) & 0xff;
     if (addr > 0xfff || !index) {
@@ -795,10 +785,10 @@ static void apic_mem_writel(void *opaque,
target_phys_addr_t addr, uint32_t val)
         return;
     }

-    env = cpu_single_env;
-    if (!env)
+    s = cpu_get_current_apic();
+    if (!s) {
         return;
-    s = env->apic_state;
+    }

     DPRINTF("write: " TARGET_FMT_plx " = %08x\n", addr, val);

@@ -974,16 +964,16 @@ static CPUWriteMemoryFunc * const apic_mem_write[3] = {
     apic_mem_writel,
 };

-int apic_init(CPUState *env)
+APICState *apic_init(void *env, uint32_t apic_id)
 {
     APICState *s;

-    if (last_apic_idx >= MAX_APICS)
-        return -1;
+    if (last_apic_idx >= MAX_APICS) {
+        return NULL;
+    }
     s = qemu_mallocz(sizeof(APICState));
-    env->apic_state = s;
     s->idx = last_apic_idx++;
-    s->id = env->cpuid_apic_id;
+    s->id = apic_id;
     s->cpu_env = env;

     msix_supported = 1;
@@ -1004,5 +994,5 @@ int apic_init(CPUState *env)
     qemu_register_reset(apic_reset, s);

     local_apics[s->idx] = s;
-    return 0;
+    return s;
 }
diff --git a/hw/apic.h b/hw/apic.h
index 7dc7c62..a6af078 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -7,14 +7,21 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode,
                              uint8_t delivery_mode,
                              uint8_t vector_num, uint8_t polarity,
                              uint8_t trigger_mode);
-int apic_init(CPUState *env);
+APICState *apic_init(void *env, uint32_t apic_id);
 int apic_accept_pic_intr(APICState *s);
 void apic_deliver_pic_intr(APICState *s, int level);
 int apic_get_interrupt(APICState *s);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
+void cpu_set_apic_base(APICState *s, uint64_t val);
+uint64_t cpu_get_apic_base(APICState *s);
+void cpu_set_apic_tpr(APICState *s, uint8_t val);
+uint8_t cpu_get_apic_tpr(APICState *s);
+void apic_init_reset(APICState *s);
+void apic_sipi(APICState *s);

 /* pc.c */
 int cpu_is_bsp(CPUState *env);
+APICState *cpu_get_current_apic(void);

 #endif
diff --git a/hw/pc.c b/hw/pc.c
index fe4ebbe..913e461 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -749,6 +749,15 @@ int cpu_is_bsp(CPUState *env)
     return env->cpu_index == 0;
 }

+APICState *cpu_get_current_apic(void)
+{
+    if (cpu_single_env) {
+        return cpu_single_env->apic_state;
+    } else {
+        return NULL;
+    }
+}
+
 /* set CMOS shutdown status register (index 0xF) as S3_resume(0xFE)
    BIOS will read it and start S3 resume at POST Entry */
 void pc_cmos_set_s3_resume(void *opaque, int irq, int level)
@@ -781,10 +790,11 @@ static CPUState *pc_new_cpu(const char *cpu_model)
     if ((env->cpuid_features & CPUID_APIC) || smp_cpus > 1) {
         env->cpuid_apic_id = env->cpu_index;
         /* APIC reset callback resets cpu */
-        apic_init(env);
+        env->apic_state = apic_init(env, env->cpuid_apic_id);
     } else {
         qemu_register_reset((QEMUResetHandler*)cpu_reset, env);
     }
+    env->halted = cpu_is_bsp(env);
     return env;
 }

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 0b19fe3..619a747 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -790,6 +790,17 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env,
     }
 }

+static inline void cpu_x86_load_seg_cache_sipi(CPUX86State *env,
+                                               int sipi_vector)
+{
+    env->eip = 0;
+    cpu_x86_load_seg_cache(env, R_CS, sipi_vector << 8,
+                           sipi_vector << 12,
+                           env->segs[R_CS].limit,
+                           env->segs[R_CS].flags);
+    env->halted = 0;
+}
+
 int cpu_x86_get_descr_debug(CPUX86State *env, unsigned int selector,
                             target_ulong *base, unsigned int *limit,
                             unsigned int *flags);
@@ -827,6 +838,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
index, uint32_t count,
                    uint32_t *eax, uint32_t *ebx,
                    uint32_t *ecx, uint32_t *edx);
 int cpu_x86_register (CPUX86State *env, const char *cpu_model);
+void cpu_clear_apic_feature(CPUX86State *env);

 /* helper.c */
 int cpu_x86_handle_mmu_fault(CPUX86State *env, target_ulong addr,
@@ -859,15 +871,6 @@ void cpu_x86_update_cr0(CPUX86State *env,
uint32_t new_cr0);
 void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3);
 void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4);

-/* hw/apic.c */
-typedef struct APICState APICState;
-void cpu_set_apic_base(APICState *s, uint64_t val);
-uint64_t cpu_get_apic_base(APICState *s);
-void cpu_set_apic_tpr(APICState *s, uint8_t val);
-#ifndef NO_CPU_IO_DEFS
-uint8_t cpu_get_apic_tpr(APICState *s);
-#endif
-
 /* hw/pc.c */
 void cpu_smm_update(CPUX86State *env);
 uint64_t cpu_get_tsc(CPUX86State *env);
@@ -929,6 +932,10 @@ static inline void cpu_clone_regs(CPUState *env,
target_ulong newsp)

 #include "svm.h"

+#if !defined(CONFIG_USER_ONLY)
+#include "hw/apic.h"
+#endif
+
 static inline void cpu_pc_from_tb(CPUState *env, TranslationBlock *tb)
 {
     env->eip = tb->pc - tb->cs_base;
@@ -943,8 +950,6 @@ static inline void cpu_get_tb_cpu_state(CPUState
*env, target_ulong *pc,
         (env->eflags & (IOPL_MASK | TF_MASK | RF_MASK | VM_MASK));
 }

-void apic_init_reset(APICState *s);
-void apic_sipi(APICState *s);
 void do_cpu_init(CPUState *env);
 void do_cpu_sipi(CPUState *env);
 #endif /* CPU_I386_H */
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 7a11215..6a0f7ca 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -944,6 +944,12 @@ static int cpudef_register(QemuOpts *opts, void *opaque)
     x86_defs = def;
     return (0);
 }
+
+void cpu_clear_apic_feature(CPUX86State *env)
+{
+    env->cpuid_features &= ~CPUID_APIC;
+}
+
 #endif /* !CONFIG_USER_ONLY */

 /* register "cpudef" models defined in configuration file.  Here we first
-- 
1.7.1