Patchwork qemu will core dump with "-smp 254, sockets=2, cores=3, threads=2"

login
register
mail settings
Submitter lijun
Date Dec. 14, 2013, 5:21 p.m.
Message ID <52AC939E.9060706@gmail.com>
Download mbox | patch
Permalink /patch/301272/
State New
Headers show

Comments

lijun - Dec. 14, 2013, 5:21 p.m.
Hi all,
     As qemu core dump cause by "sockets=2,cores=3,threads=2", so add 
this patch to check whether cores and threads is a power of 2.
     The following is the realization of apicid_from_topo_ids function 
in file target-i386/topology.h. It uses shift to get the values of 
pkg_id and core_id. nr_cores and nr_threads is related to this shift.
static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
                                              unsigned nr_threads,
                                              unsigned pkg_id,
                                              unsigned core_id,
                                              unsigned smt_id)
{
     return (pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
            (core_id << apicid_core_offset(nr_cores, nr_threads)) |
            smt_id;
}
----
So should add a check for smp_cores and smp_threads in smp_parse 
function in file vl.c. Check whether smp_cores and smp_threads is a 
power of 2, so nr_cores and nr_threads is a power of 2.  When return 
from apicid_from_topo_ids function, apic_id and id could get the correct 
values(apic_id is in file "hw/i386/acpi-build.c" and id is in file 
"hw/acpi/piix4.c") .

Without this check for smp_cores and smp_threads, specify "-smp 
160,sockets=2,cores=3,threads=2", qemu will core dump too.




Best Regards,
Jun Li
Peter Maydell - Dec. 14, 2013, 7:10 p.m.
On 14 December 2013 17:21, lijun <junmuzi@gmail.com> wrote:
> Hi all,
>     As qemu core dump cause by "sockets=2,cores=3,threads=2", so add this
> patch to check whether cores and threads is a power of 2.
>     The following is the realization of apicid_from_topo_ids function in
> file target-i386/topology.h. It uses shift to get the values of pkg_id and
> core_id. nr_cores and nr_threads is related to this shift.
> static inline apic_id_t apicid_from_topo_ids(unsigned nr_cores,
>                                              unsigned nr_threads,
>                                              unsigned pkg_id,
>                                              unsigned core_id,
>                                              unsigned smt_id)
> {
>     return (pkg_id  << apicid_pkg_offset(nr_cores, nr_threads)) |
>            (core_id << apicid_core_offset(nr_cores, nr_threads)) |
>            smt_id;
> }
> ----
> So should add a check for smp_cores and smp_threads in smp_parse function in
> file vl.c. Check whether smp_cores and smp_threads is a power of 2, so
> nr_cores and nr_threads is a power of 2.  When return from
> apicid_from_topo_ids function, apic_id and id could get the correct
> values(apic_id is in file "hw/i386/acpi-build.c" and id is in file
> "hw/acpi/piix4.c") .

vl.c is cross platform code so I'm not sure it's the right place to make
this check. x86 ACPI may not be able to handle a non-power-of-2
number of cores in a socket, but it's a v alid ARM SMP configuration,
for instance.

thanks
-- PMM
Eric Blake - Dec. 16, 2013, 2:43 p.m.
On 12/14/2013 10:21 AM, lijun wrote:
> Hi all,
>     As qemu core dump cause by "sockets=2,cores=3,threads=2", so add
> this patch to check whether cores and threads is a power of 2.

> +/**
> + * This function will return whether @num is power of 2.
> + *
> + * Returns: 1 indicate @num is power of 2, 0 indicate @num is not.
> + */
> +static int is_2_power(int num)
> +{
> +    if (num < 0 || num > 256)
> +        return 1;
> +
> +    return !(num & (num - 1));
> +}

Please don't reinvent qemu-common.h's is_power_of_2.  Furthermore, your
function is more than just a power-of-2 check, it is also a range check.

Patch

--- a/vl.c    2013-12-14 23:46:58.991076467 +0800
+++ b/vl.c    2013-12-15 00:40:31.653800907 +0800
@@ -1384,6 +1384,19 @@ 
      },
  };

+/**
+ * This function will return whether @num is power of 2.
+ *
+ * Returns: 1 indicate @num is power of 2, 0 indicate @num is not.
+ */
+static int is_2_power(int num)
+{
+    if (num < 0 || num > 256)
+        return 1;
+
+    return !(num & (num - 1));
+}
+
  static void smp_parse(QemuOpts *opts)
  {
      if (opts) {
@@ -1418,6 +1431,12 @@ 

      }

+    /* check whether smp_cores and smp_threads is a power of 2 */
+    if (!is_2_power(smp_cores) || !is_2_power(smp_threads)) {
+        smp_cores = 1;
+        smp_threads = 1;
+    }
+
      if (max_cpus == 0) {
          max_cpus = smp_cpus;
      }