Re: [Qemu-devel] [PATCHv3 2/4] pseries: Move CPU compatibility property to machine

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On Thu, May 04, 2017 at 08:50:47PM +0200, Greg Kurz wrote:
> On Thu, 04 May 2017 19:09:11 +0200
> Andrea Bolognani <abologna@redhat.com> wrote:
> 
> > On Thu, 2017-04-27 at 17:28 +1000, David Gibson wrote:
> > > @@ -2480,6 +2480,10 @@ static void spapr_machine_initfn(Object *obj)
> > >                                      " place of standard EPOW events when possible"
> > >                                      " (required for memory hot-unplug 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);  
> > 
> > I'm not familiar with QEMU's object system, but shouldn't
> > you be using object_property_add_str() instead? It looks
> > like you're doing more than the straightforward wrapper
> > would do, so maybe that's just not possible.
> > 
> > 
> > In any case, all other string properties look like
> > 
> >   pseries-2.10.kvm-type=string
> > 
> > whereas this one ends up looking like
> > 
> >   pseries-2.10.max-cpu-compat=str
> > 
> > which I think should be fixed - object_property_add_str()
> > passes "string" instead of "str" to object_property_add().
> > 
> > You should also add a sensible description for the property,
> > preferably spelling out all the accepted values.
> > 
> > 
> > Speaking of properties...
> > 
> >   $ qemu-system-ppc64 -cpu host,compat=whatever
> >   Segmentation fault
> > 
> > You might want to look into that ;)
> > 
> 
> This happens because patch 2 is missing a change for the recently added POWER9:
> 
>          .max_threads = 8,
>      },
>      { /* POWER9, ISA3.00 */
> +        .name = "power9",
>          .pvr = CPU_POWERPC_LOGICAL_3_00,
>          .pcr = PCR_COMPAT_3_00,
>          .pcr_level = PCR_COMPAT_3_00,
> 

Right, I have that fixed in my tree so it will be in the next spin.

-- 
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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]