From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55567) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFT46-0007M9-Cs for qemu-devel@nongnu.org; Mon, 17 Feb 2014 13:37:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFT41-0000SF-FY for qemu-devel@nongnu.org; Mon, 17 Feb 2014 13:37:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:42444) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFT41-0000S1-6X for qemu-devel@nongnu.org; Mon, 17 Feb 2014 13:37:09 -0500 Message-ID: <530256CD.20007@redhat.com> Date: Mon, 17 Feb 2014 11:37:01 -0700 From: Eric Blake MIME-Version: 1.0 References: <50f1d46e5a346a6788f8c4dd6c2bef7da34d7b6d.1389685621.git.chen.fan.fnst@cn.fujitsu.com> In-Reply-To: <50f1d46e5a346a6788f8c4dd6c2bef7da34d7b6d.1389685621.git.chen.fan.fnst@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="QSmjmk2BkcJk4NqgOnkk83dt1CW9f444k" Subject: Re: [Qemu-devel] [RFC 2/3] target-i386: add -smp X,apics=0x option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Chen Fan , qemu-devel@nongnu.org Cc: Igor Mammedov , =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --QSmjmk2BkcJk4NqgOnkk83dt1CW9f444k Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/14/2014 02:27 AM, Chen Fan wrote: > This option provides the infrastructure for specifying apicids when > boot VM, For example: >=20 > #boot with apicid 0 and 2: > -smp 2,apics=3D0xA,maxcpus=3D4 /* 1010 */ > #boot with apicid 1 and 7: > -smp 2,apics=3D0x41,maxcpus=3D8 /* 0100 0001 */ This syntax feels a bit odd when maxcpus is not a multiple of 8, and even harder when not a multiple of 4. I think part of my confusion stems from you treating the lsb as the left-most bit, but expect me to write in hex where I'm used to the right-most bit being lsb Wouldn't it be easier to express: msb .... lsb with leading 0s implied as needed, as in: 0x5 =3D> 0101 =3D> id 0 (lsb) and id 2 are enabled, regardless of whether= maxcpus=3D4 or maxcpus=3D8 0x82 =3D> 1000 0010 =3D> id 1 and id 7 are enabled, regardless of whether= maxcpus=3D8 or maxcpus=3D256 0x100000000 =3D> id 32 is enabled Or even better, why not reuse existing parsers that take cpu ids directly as numbers instead of making me compute a bitmap (as in maxcpus=3D4,id=3D0,id=3D2 - although I don't quite know QemuOpts well eno= ugh to know if you can repeat id=3D for forming a list of disjoint id numbers= ) > @@ -92,6 +93,14 @@ of @var{threads} per cores and the total number of @= var{sockets} can be > specified. Missing values will be computed. If any on the three values= is > given, the total number of CPUs @var{n} can be omitted. @var{maxcpus} > specifies the maximum number of hotpluggable CPUs. > +@var{apics} specifies the boot bitmap of existed apicid. s/existed/existing/ > + > +@example > +#specify the boot bitmap of apicid with 0 and 2: > +qemu-system-i386 -smp 2,apics=3D0xA,maxcpus=3D4 /* 1010 */ > +#specify the boot bitmap of apicid with 1 and 7: > +qemu-system-i386 -smp 2,apics=3D0x41,maxcpus=3D8 /* 0100 0001 */ > +@end example These examples would need updating to match my concerns. > @@ -1379,6 +1382,9 @@ static QemuOptsList qemu_smp_opts =3D { > }, { > .name =3D "maxcpus", > .type =3D QEMU_OPT_NUMBER, > + }, { > + .name =3D "apics", > + .type =3D QEMU_OPT_STRING, Why a string with your own ad-hoc parser? Can't we reuse some of the existing parsers that already know how to handle (possibly-disjoint) lists of cpu numbers? > + if (apics) { > + if (strstart(apics, "0x", &apics)) { Why not also allow 0X? > + if (*apics !=3D '\0') { > + int i, count; > + int64_t max_apicid =3D 0; > + uint32_t val; > + char tmp[2]; > + > + count =3D strlen(apics); > + > + for (i =3D 0; i < count; i++) { > + tmp[0] =3D apics[i]; > + tmp[1] =3D '\0'; > + sscanf(tmp, "%x", &val); sscanf is evil. It has undefined behavior on input overflow (that is, if I say 0x10000000000000000, there is no guarantee what sscanf will stick into val). All the more reason you should be using an existing parser which gracefully handles overflow. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --QSmjmk2BkcJk4NqgOnkk83dt1CW9f444k Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTAlbNAAoJEKeha0olJ0Nqs7wH/Rx8cYjBlBoQYb6EV+IcHxxg oxfFdUM+w2FbLU9+kDOK+n3U/yZ9eH3RnFsnmxSRuu83KepakF0h5OCsb3VTis/k 4tqG4kK+CQ4YaRh2bBPo1K+k6SvM5AbdoEwoeXYstDkHFkRrKRpyhVr4hfpXLs/P +nFAwP0m4ayrbcdGZmMxJPguxohyVFP9RUPByf+fkkYAylV+WlnBbBx4eI2NrESC +/bKwl3IH9Se/omPJBTdyvcNSKiO4eYcjxzq8PSqevHQ7L45CgV41H3GizyuY8Ej Pm79uNZMh6E8DAD0MESJPlkRmMDVQRn6bzE2YgqD24efAWtmKT0aTb0wCx1GmUo= =S0gt -----END PGP SIGNATURE----- --QSmjmk2BkcJk4NqgOnkk83dt1CW9f444k--