Re: [Qemu-devel] [RFC qom-cpu v2 1/8] apic: remove apic_no from apic_init_common()

[Igor Mammedov]

On Tue, 10 Sep 2013 17:43:41 +0800
Chen Fan <address@hidden> wrote:

> the 'apic_no' is increased by one when initialize/create a vCPU each time,
> which causes APICCommonState s->idx always is increased.
> but if we want to re-add a vCPU after removing a vCPU, we need to re-use the
> vacant s->idx which the corresponding vCPU had been removed. 
> so we could use the unique cpu apic_id instead of the progressive s->idx.
> 
> Signed-off-by: Chen Fan <address@hidden>
> ---
>  hw/intc/apic_common.c | 4 +---
>  target-i386/cpu.c     | 1 +
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index a0beb10..5568621 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -289,13 +289,11 @@ static int apic_init_common(ICCDevice *dev)
>      APICCommonState *s = APIC_COMMON(dev);
>      APICCommonClass *info;
>      static DeviceState *vapic;
> -    static int apic_no;
>      static bool mmio_registered;
>  
> -    if (apic_no >= MAX_APICS) {
> +    if (s->idx >= MAX_APICS) {
>          return -1;
>      }
> -    s->idx = apic_no++;
>  
>      info = APIC_COMMON_GET_CLASS(s);
>      info->init(s);
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 42c5de0..2b99683 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2322,6 +2322,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error 
> **errp)
>      /* TODO: convert to link<> */
>      apic = APIC_COMMON(env->apic_state);
>      apic->cpu = cpu;
> +    apic->idx = env->cpuid_apic_id;
earlier here we set:
 qdev_prop_set_uint8(env->apic_state, "id", env->cpuid_apic_id);
so apic->idx = env->cpuid_apic_id is redundant.

it would be better to search by apic->id and preferably replace O(MAX_APIC) 
scans with
a faster approach since for TCG iqr delivery might be a hot path, dropping 
MAX_APIC
altogether and using dynamic present APICs list.

>  }
>  
>  static void x86_cpu_apic_realize(X86CPU *cpu, Error **errp)