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

[Greg Kurz <groug@kaod.org>]

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

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,


> -- 
> Andrea Bolognani / Red Hat / Virtualization


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]