Re: [Qemu-devel] [RFC 1/3] using CPUMASK bitmaps to calculate cpu index

[Igor Mammedov]

On Tue, 13 May 2014 18:08:47 +0800
Chen Fan <address@hidden> wrote:

> instead of seeking the number of CPUs, using CPUMASK bitmaps to
> calculate the cpu index, also would be a gread benefit to remove
> cpu index.
How would it help to remove cpu_index?
What if there is only one CPU at start nad there is a need to add
CPU with cpu_index==10?

Bitmap hardly changes here anything, it's just another way of
doing:
   cpu_index = 0;
   CPU_FOREACH(some_cpu) {
       cpu_index++;
   }


What would help however is replacing cpu_index at read points with
CPUClass->get_arch_id()

Then targets that need sparse index could override default
get_arch_id() to suite their needs and use their own properties
to set arch_id value.

Caution: this change would cause migration breakage for target-i386,
so x86_cpu_get_arch_id() should take care about it when used with old
machine types.

> 
> Signed-off-by: Chen Fan <address@hidden>
> ---
>  exec.c                  | 9 ++++-----
>  include/qom/cpu.h       | 9 +++++++++
>  include/sysemu/sysemu.h | 7 -------
>  3 files changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index cf12049..2948841 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -473,16 +473,15 @@ void cpu_exec_init(CPUArchState *env)
>  {
>      CPUState *cpu = ENV_GET_CPU(env);
>      CPUClass *cc = CPU_GET_CLASS(cpu);
> -    CPUState *some_cpu;
>      int cpu_index;
>  
>  #if defined(CONFIG_USER_ONLY)
>      cpu_list_lock();
>  #endif
> -    cpu_index = 0;
> -    CPU_FOREACH(some_cpu) {
> -        cpu_index++;
> -    }
> +    cpu_index = find_first_zero_bit(cc->cpu_present_mask,
> +                                    MAX_CPUMASK_BITS);
> +    set_bit(cpu_index, cc->cpu_present_mask);
> +
>      cpu->cpu_index = cpu_index;
>      cpu->numa_node = 0;
>      QTAILQ_INIT(&cpu->breakpoints);
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index df977c8..b8f46b1 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -70,6 +70,13 @@ typedef void (*CPUUnassignedAccess)(CPUState *cpu, hwaddr 
> addr,
>  
>  struct TranslationBlock;
>  
> +/* The following shall be true for all CPUs:
> + *   cpu->cpu_index < max_cpus <= MAX_CPUMASK_BITS
> + *
> + * Note that cpu->get_arch_id() may be larger than MAX_CPUMASK_BITS.
> + */
> +#define MAX_CPUMASK_BITS 255
> +
>  /**
>   * CPUClass:
>   * @class_by_name: Callback to map -cpu command line model name to an
> @@ -142,6 +149,8 @@ typedef struct CPUClass {
>      const struct VMStateDescription *vmsd;
>      int gdb_num_core_regs;
>      const char *gdb_core_xml_file;
> +
> +    DECLARE_BITMAP(cpu_present_mask, MAX_CPUMASK_BITS);
>  } CPUClass;
>  
>  #ifdef HOST_WORDS_BIGENDIAN
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index ba5c7f8..04edb8b 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -134,13 +134,6 @@ extern QEMUClockType rtc_clock;
>  
>  #define MAX_NODES 64
>  
> -/* The following shall be true for all CPUs:
> - *   cpu->cpu_index < max_cpus <= MAX_CPUMASK_BITS
> - *
> - * Note that cpu->get_arch_id() may be larger than MAX_CPUMASK_BITS.
> - */
> -#define MAX_CPUMASK_BITS 255
> -
>  extern int nb_numa_nodes;
>  extern uint64_t node_mem[MAX_NODES];
>  extern unsigned long *node_cpumask[MAX_NODES];