Re: [Qemu-devel] [PATCH v2 1/6] Add cfgend parameter for ARM CPU selection.

[Julian Brown]

On Thu, 5 Jan 2017 17:17:13 +0000
Peter Maydell <address@hidden> wrote:

> > +    qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
> > +                             &error_abort);
> > +
> > +    qdev_prop_set_globals(DEVICE(obj));
> > +
> > +    if (object_property_get_bool(obj, "cfgend", NULL)) {
> > +        if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> > +            cpu->reset_sctlr |= SCTLR_EE;
> > +        } else {
> > +            cpu->reset_sctlr |= SCTLR_B;
> > +        }
> > +    }  
> 
> Can we just implement this property the same way we do all
> our existing ones, ie just call qdev_property_add_static()
> here, and then look at the property value in arm_cpu_realizefn() ?
> I'm not clear what the call to qdev_prop_set_globals() is
> needed for.

...

> > @@ -765,15 +782,20 @@ static ObjectClass
> > *arm_cpu_class_by_name(const char *cpu_model) return NULL;
> >      }
> >
> > -    cpuname = g_strsplit(cpu_model, ",", 1);
> > +    cpuname = g_strsplit(cpu_model, ",", 2);
> >      typename = g_strdup_printf("%s-" TYPE_ARM_CPU, cpuname[0]);
> >      oc = object_class_by_name(typename);
> > -    g_strfreev(cpuname);
> > -    g_free(typename);
> >      if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU) ||
> >          object_class_is_abstract(oc)) {
> > +        g_strfreev(cpuname);
> > +        g_free(typename);
> >          return NULL;
> >      }
> > +
> > +    cc = CPU_CLASS(oc);
> > +    cc->parse_features(typename, cpuname[1], &error_fatal);
> > +    g_strfreev(cpuname);
> > +  
> 
> I'm also not clear why this needs to change -- the existing code works
> for all of our current properties.

I'm a little confused, I think -- these changes seemed to be necessary
to allow the parsing of the command-line syntax you suggested earlier
(-mcpu=foo,cfgend=bar):

http://lists.nongnu.org/archive/html/qemu-devel/2016-11/msg00660.html

The qdev_prop_set_globals function calls back into arm/cpu.c to do the
actual parsing, IIRC.

Can the existing properties be set like that via the command line?
AFAICT they're only used to communicate settings from the machine
models (integratorcp, and so on) to the CPU initialisation code, and
are never exposed to the user.

Did I miss something? (I think the g_strsplit call with 1 as its third
argument is a no-op. Actually maybe that was supposed to be -1?)

Thanks,

Julian