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

[Blue Swirl]

On Sun, Jun 6, 2010 at 7:36 AM, Jan Kiszka <address@hidden> wrote:
> Blue Swirl wrote:
>> 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);
>
> We are now lacking 'halted' initialization after system reset. Could be
> addressed by a special reset handler in hw/pc.c, I guess.

Good catch, I forgot to do that.