From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c4L3h-0005Fd-Mp for qemu-devel@nongnu.org; Wed, 09 Nov 2016 00:04:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c4L3g-000429-AY for qemu-devel@nongnu.org; Wed, 09 Nov 2016 00:04:25 -0500 Date: Wed, 9 Nov 2016 15:41:30 +1100 From: David Gibson Message-ID: <20161109044130.GF427@umbus.fritz.box> References: <1477825928-10803-1-git-send-email-david@gibson.dropbear.id.au> <1477825928-10803-14-git-send-email-david@gibson.dropbear.id.au> <20161108052631.GU28688@umbus.fritz.box> <711cf9b0-1233-c66b-eccd-a0acfccf7e96@ozlabs.ru> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Wb5NtZlyOqqy58h0" Content-Disposition: inline In-Reply-To: <711cf9b0-1233-c66b-eccd-a0acfccf7e96@ozlabs.ru> Subject: Re: [Qemu-devel] [RFC 13/17] pseries: Move CPU compatibility property to machine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: nikunj@linux.vnet.ibm.com, mdroth@linux.vnet.ibm.com, thuth@redhat.com, lvivier@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org --Wb5NtZlyOqqy58h0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Nov 08, 2016 at 04:56:10PM +1100, Alexey Kardashevskiy wrote: > On 08/11/16 16:26, David Gibson wrote: > > On Fri, Nov 04, 2016 at 06:43:52PM +1100, Alexey Kardashevskiy wrote: > >> On 30/10/16 22:12, David Gibson wrote: > >>> Server class POWER CPUs have a "compat" property, which is used to se= t the > >>> backwards compatibility mode for the processor. However, this only m= akes > >>> sense for machine types which don't give the guest access to hypervis= or > >>> privilege - otherwise the compatibility level is under the guest's co= ntrol. > >>> > >>> To reflect this, this removes the CPU 'compat' property and instead > >>> creates a 'max-cpu-compat' property on the pseries machine. Strictly > >>> speaking this breaks compatibility, but AFAIK the 'compat' option was > >>> never (directly) used with -device or device_add. > >>> > >>> The option was used with -cpu. So, to maintain compatibility, this p= atch > >>> adds a hack to the cpu option parsing to strip out any compat options > >>> supplied with -cpu and set them on the machine property instead of th= e new > >>> removed cpu property. > >>> > >>> Signed-off-by: David Gibson > >>> --- > >>> hw/ppc/spapr.c | 6 +++- > >>> hw/ppc/spapr_cpu_core.c | 47 +++++++++++++++++++++++++++-- > >>> hw/ppc/spapr_hcall.c | 2 +- > >>> include/hw/ppc/spapr.h | 10 +++++-- > >>> target-ppc/compat.c | 65 +++++++++++++++++++++++++++++++++++= +++++ > >>> target-ppc/cpu.h | 6 ++-- > >>> target-ppc/translate_init.c | 73 -----------------------------------= ---------- > >>> 7 files changed, 127 insertions(+), 82 deletions(-) > >>> > >>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > >>> index 6c78889..b983faa 100644 > >>> --- a/hw/ppc/spapr.c > >>> +++ b/hw/ppc/spapr.c > >>> @@ -1849,7 +1849,7 @@ static void ppc_spapr_init(MachineState *machin= e) > >>> machine->cpu_model =3D kvm_enabled() ? "host" : smc->tcg_def= ault_cpu; > >>> } > >>> =20 > >>> - ppc_cpu_parse_features(machine->cpu_model); > >>> + spapr_cpu_parse_features(spapr); > >>> =20 > >>> spapr_init_cpus(spapr); > >>> =20 > >>> @@ -2191,6 +2191,10 @@ static void spapr_machine_initfn(Object *obj) > >>> " place of standard EPOW events = when possible" > >>> " (required for memory hot-unplu= g support)", > >>> NULL); > >>> + > >>> + object_property_add(obj, "max-cpu-compat", "str", > >>> + ppc_compat_prop_get, ppc_compat_prop_set, > >>> + NULL, &spapr->max_compat_pvr, &error_fatal); > >>> } > >>> =20 > >>> static void spapr_machine_finalizefn(Object *obj) > >>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > >>> index ee5cd14..0319516 100644 > >>> --- a/hw/ppc/spapr_cpu_core.c > >>> +++ b/hw/ppc/spapr_cpu_core.c > >>> @@ -18,6 +18,49 @@ > >>> #include "target-ppc/mmu-hash64.h" > >>> #include "sysemu/numa.h" > >>> =20 > >>> +void spapr_cpu_parse_features(sPAPRMachineState *spapr) > >>> +{ > >>> + /* > >>> + * Backwards compatibility hack: > >>> + > >>> + * CPUs had a "compat=3D" property which didn't make sense for > >>> + * anything except pseries. It was replaced by "max-cpu-compa= t" > >>> + * machine option. This supports old command lines like > >>> + * -cpu POWER8,compat=3Dpower7 > >>> + * By stripping the compat option and applying it to the machi= ne > >>> + * before passing it on to the cpu level parser. > >>> + */ > >>> + gchar **inpieces, **outpieces; > >>> + int n, i, j; > >>> + gchar *compat_str =3D NULL; > >>> + gchar *filtered_model; > >>> + > >>> + inpieces =3D g_strsplit(MACHINE(spapr)->cpu_model, ",", 0); > >>> + n =3D g_strv_length(inpieces); > >>> + outpieces =3D g_new0(gchar *, g_strv_length(inpieces)); > >>> + > >>> + /* inpieces[0] is the actual model string */ > >>> + for (i =3D 0, j =3D 0; i < n; i++) { > >>> + if (g_str_has_prefix(inpieces[i], "compat=3D")) { > >>> + compat_str =3D inpieces[i]; > >>> + } else { > >>> + outpieces[j++] =3D g_strdup(inpieces[i]); > >>> + } > >>> + } > >>> + > >>> + if (compat_str) { > >>> + char *val =3D compat_str + strlen("compat=3D"); > >>> + object_property_set_str(OBJECT(spapr), val, "max-cpu-compat", > >>> + &error_fatal); > >> > >> This part is ok. > >> > >>> + } > >>> + > >>> + filtered_model =3D g_strjoinv(",", outpieces); > >>> + ppc_cpu_parse_features(filtered_model); > >> > >> > >> Rather than reducing the CPU parameters string from the command line, = I'd > >> keep "dc->props =3D powerpc_servercpu_properties" and make them noop += warn > >> to use the machine option instead. One day QEMU may start calling the = CPU > >> features parser itself and somebody will have to hack this thing > >> again. > >=20 > > Hrm. A deprecation message like that only works if a human is reading > > it. Usually qemu will be invoked by libvirt and the message will > > probably disappear into some log file to scare someone unnecessarily. > >=20 > > Meanwhile, what will the actual behaviour be? Pulling the CPU's > > property value into the machine instead would be really ugly. > > Ignoring it would break users with existing libvirt. >=20 >=20 > I only suggested instead of removing "compat=3D" from the model string, > - pass the model as is to ppc_cpu_parse_features() with no changes; > - change powerpc_set_compat() to print a message and do nothing else, and > add a comment there saying why it is so. Ah, right, now I understand. Yes, that's a good idea. It will greatly simplify the rather hideous string mangling, too. > > The hack above is nasty, but I'm not really seeing a better option. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --Wb5NtZlyOqqy58h0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJYIqj3AAoJEGw4ysog2bOSSjUQANyz868x4cDMQZokoVK2HQIq dT8vwT6Ys2qELMVipwddHrCg/K9JSGgDxdDmh9byH221yMeh67BO4K0MO7q1KX8K CsIeRjph2wK6CYi8bVexMOjK4pB51iWa0F0aKg/gVLJN00wThCaoOoeGFKBqErt3 MFTKJxzRoF83l9XI0dEik5MyoaGsiaPJbuzb0nlCJnsHXiUtPrgiv07vQMY1dsyA /caIi3ODa4sqgbtIKhRQDogPdcxmHHFAY7XDI2kJWZyCbLcXdR7zLmhIjnan61eJ iCduS4RpFvJ18VUUpbHi12E78I2UEtaGlH8OaLp2Ay99oh5wyQoEKexUxM4WEnWO h1a/JP6CpUr5GtUJEIchjnZ8vF9+Z1PRf7hGU6+eRzTUfG0eUo7nB5czAYiRG/mO F6C62X9XEP6Ui89BEaEhju9IVCfAwUUuP/XC5g346J3CxcPFsh5GxDs6zrtT7tzx wxt2XCwrJo8M193SXHZJIRP5fSPD1GNE2qF5bqs5pP5f9RyN1flqrABvAuH4Lg3J /Xex7Ch3/a8wSsVoEsViHybFLGhwcO3xTqIkd6wJF7UFthzmUdlBMgI37fFrto0I 08QQ75wY82TM/MaLYbXqvDl38luDbY51L9qMfSJJkcgerJXoz0KiaAsq7VkJjUC2 FF4V69d2Dny3dOpt//yk =+UFT -----END PGP SIGNATURE----- --Wb5NtZlyOqqy58h0--