Re: [Qemu-devel] [PATCH] qemu will core dump with "-smp 254, , sockets=2, cores=3, threads=2"

[Eduardo Habkost]

On Tue, Dec 17, 2013 at 11:16:30PM +0800, lijun wrote:
> As Eric and Eduardo's suggestions, use is_power_of_2 to check
> whether nr_cores
> and nr_threads is the power of 2 in function x86_apicid_from_cpu_idx in file
> target-i386/topology.h. This check is very simple, I prefer add it in a
> function to write a new function. Thanks for Eric and Eduardo.
> 
> Best Regards,
> Jun Li
> 
> Signed-off-by: Jun Li <address@hidden>
> ---
>  target-i386/topology.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/target-i386/topology.h b/target-i386/topology.h
> index 07a6c5f..ff21a98 100644
> --- a/target-i386/topology.h
> +++ b/target-i386/topology.h
> @@ -126,6 +126,14 @@ static inline apic_id_t
> x86_apicid_from_cpu_idx(unsigned nr_cores,
>                                                  unsigned cpu_index)
>  {
>      unsigned pkg_id, core_id, smt_id;
> +
> +    /* Check whether nr_cores and nr_threads is a power of 2.
> +     * If not, 1 is assigned to them.
> +     */
> +    nr_cores = is_power_of_2(nr_cores) > 0 ? nr_cores : 1;
> +    nr_threads = is_power_of_2(nr_threads) > 0 ? nr_threads : 1;

I still have the same questions from my previous messages. The whole
point of topology.h is to calculate correct APIC IDs when nr_cores or
nr_threads are not powers of 2. You are simply reintroducing the bugs
that were fixed when topology.h was created.

In other words: x86_apicid_from_cpu_idx(3, 42, 253) must return 513, and
you are breaking it.

See:
  http://article.gmane.org/gmane.comp.emulators.qemu/190390
  Message-Id: <address@hidden>
and:
  http://article.gmane.org/gmane.comp.emulators.qemu/164044
  Message-Id: <address@hidden>



> +
> +
>      x86_topo_ids_from_idx(nr_cores, nr_threads, cpu_index,
>                            &pkg_id, &core_id, &smt_id);
>      return apicid_from_topo_ids(nr_cores, nr_threads, pkg_id,
> core_id, smt_id);
> -- 
> 1.8.3.1
> 

-- 
Eduardo