From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35206) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VeQ9X-0000ef-7C for qemu-devel@nongnu.org; Thu, 07 Nov 2013 09:01:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VeQ9P-0005Lz-OX for qemu-devel@nongnu.org; Thu, 07 Nov 2013 09:01:43 -0500 Message-ID: <527B9D3B.3060407@suse.de> Date: Thu, 07 Nov 2013 15:01:31 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <5278F84A.2010904@suse.de> <1383815511-21996-1-git-send-email-aik@ozlabs.ru> In-Reply-To: <1383815511-21996-1-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] spapr: add "compat" machine option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , qemu-devel@nongnu.org Cc: Paolo Bonzini , qemu-ppc@nongnu.org, Alexander Graf , Igor Mammedov Am 07.11.2013 10:11, schrieb Alexey Kardashevskiy: > On 11/06/2013 12:53 AM, Andreas F=C3=A4rber wrote:> Am 05.11.2013 10:52= , schrieb Paolo Bonzini: >>> Il 05/11/2013 10:16, Alexander Graf ha scritto: >>>> >>>> On 05.11.2013, at 10:06, Paolo Bonzini wrote: >>>> >>>>> Il 30/09/2013 14:57, Alexey Kardashevskiy ha scritto: >>>>>>>> Why is the option under -machine instead of -cpu? >>>>>> Because it is still the same CPU and the guest will still read the= real >>>>>> PVR from the hardware (which it may not support but this is why we= need >>>>>> compatibility mode). >>>>> >>>>> How do you support migration from a newer to an older CPU then? I = think >>>>> the guest should never see anything about the hardware CPU model. >>>> >>>> POWER can't model that. It always leaks the host CPU information int= o the guest. It's the guest kernel's responsibility to not expose that ch= ange to user space. >>>> >>>> Yes, it's broken :). I'm not even sure there is any sensible way to = do live migration between different CPU types. >>> >>> Still in my opinion it should be "-cpu", not "-machine". Even if it'= s >>> just a "virtual" CPU model. >> >> PowerPC currently does not have -cpu option parsing. If you need to >> implement it, I would ask for a generic hook in CPUClass set by >> TYPE_POWERPC_CPU, so that the logic does not get hardcoded in cpu_init= , >> and for the p=3Dv parsing logic to be so generic as to just set proper= ty p >> to value v on the CPU instance. I.e. please make the compatibility >> settings a static property or dynamic property of the CPU. >> >> Maybe the parsing code could even live in generic qom/cpu.c, overridde= n >> by x86/sparc and reused for arm? Somewhere down my to-do list but >> patches appreciated... >=20 >=20 > I spent some time today trying to digest what you said, still having pr= oblems > with understanding of what you meant and what Igor meant about global v= ariables > (I just do not see the point in them). >=20 > Below is the result of my excercise. At the moment I would just like to= know > if I am going in the right direction or not. The overall direction is good ... see below. >=20 > And few question while we are here: > 1. the proposed common code handles both static and dynamic properties. > What is the current QEMU trend about choosing static vs. dynamic? Can d= o > both in POWERPC, both have benifits. Static properties have mostly served to set a field to a value before the object is realized. You can set a default value there. The setters are usually no-op (error out) for realized objects. Dynamic properties allow you (more easily) to implement any logic for storing/retrieving the value and can serve to inspect or set a value at runtime. We were told on a KVM call that discovery of properties should not be a decision factor towards static properties - management tools need to inspect an object instance via QMP (and handle a property getting dropped or renamed). > 2. The static powerpc_properties array only works if defined with POWER= 7 family > but not POWER family. Both families are abstract so I did not expect an= y > difference but it is there. Any clue before I continue debugging? :) There is no hierarchy among families. So POWER7 is not a POWER, it's a powerpc at the bottom of the file. If you want power_properties rather than powerpc_properties then you need to assign them individually for POWER, ..., POWER5, ..., POWER7, POWER8 - or tweak the type hierarchy. >=20 > Thanks! >=20 > --- >=20 > This adds suboptions support for -cpu and demonstrates the use ot this > by adding a static "compat1" and a dynamic "compat" options to POWERPC > CPUs. Unfortunately that approach won't work. Both x86 and sparc, as I mentioned, need special handling, so you can't generalize it. Either we need #ifdef'fery to rule out the exceptions, or better, what I suggested was something along the lines of struct CPUClass { ... void (*parse_options)(CPUState *cpu, const char *str); } with cpu_common_parse_options() as the default implementation assigned via cc->parse_options =3D cpu_common_parse_options; rather than called ou= t of common code. You could have a trivial (inline?) function to obtain cc and call cc->parse_options though, for use in cpu_ppc_init(). I also think you can use the object_property_* API rather than qdev_prop_* for parsing and setting the value, compare Igor's code in target-i386/cpu.c. Please do separate these global preparations from the actual new ppc property. Elsewhere it was discussed whether to use a readable string value, which might hint at a dynamic property of type string or maybe towards an enum (/me no experience with those yet and whether that works better with dynamic or static). Regards, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3=BC= rnberg