| Message ID | 52AC939E.9060706@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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.
--- 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; }
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