Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing

[Igor Mammedov]

On Tue, 20 Jan 2015 16:45:19 -0600
Greg Bellows <address@hidden> wrote:

> On Tue, Jan 20, 2015 at 10:25 AM, Igor Mammedov <address@hidden> wrote:
> > On Tue, 20 Jan 2015 16:08:09 +0000
> > Peter Maydell <address@hidden> wrote:
> >
> >> On 20 January 2015 at 15:59, Igor Mammedov <address@hidden> wrote:
> >> > On Tue, 20 Jan 2015 15:34:23 +0000
> >> > Peter Maydell <address@hidden> wrote:
> >> >
> >> >> On 20 January 2015 at 15:22, Igor Mammedov <address@hidden> wrote:
> >> >> > Please do not use legacy +-feature format and support only foo=val 
> >> >> > format.
> >> >> > Other targets have it only for to being able support legacy setups
> >> >> > which use +- format.
> >> >>
> >> >> I thought this was the standard format for CPU features. Do you
> >> >> have an example of a CPU feature being set using foo=val format?
> >> > Currently on x86 we can use either legacy +foo1,-foo2,foo3 and
> >> > in addition to it we ca use canonized format for generic properties
> >> > like, foo1=on,foo2=off,foo3=on
> >> >
> >> > We try to move out of legacy format, so that it would be possible
> >> > to reuse generic property parsing infrastructure like with any
> >> > device object. That would allow to use -device/device_add for CPUs.
> >>
> >> -device/-device_add for CPUs is pretty fraught in the general
> >> case because there's no obvious place to plug them and have
> >> them be wired up properly.
> > That depends on CPU of-cause, but we are close to having device_add
> > working with x86 CPUs.
> >
> >> You'd need to use -global for CPU
> >> properties, which is a nightmare...
> > mine thoughts on it were that '-cpu type,feat...' would  eventually
> > do conversion of features to global properties transparently for
> > user using target specific cc->parse_features() callback. Which
> > Greg could actually do here. We would happy to reuse it with x86 CPUs.
> >
> >>
> >> Anyway, I'm not particularly attached to the exact command
> >> line syntax we've used here -- I was just looking for "we have
> >> a CPU property, and use the same syntax for specifying CPU
> >> feature enable/disable that other CPUs do"...
> > Then '-cpu arm_foo,featX=on/off' should do the job.
> >
> >
> >>
> >> -- PMM
> >
> 
> For now I went with the "-cpu arm_foo,aarch64=off" approach so as to
> not complicate it right now with adding full-blown CPU properties.
I see that there is quite a mess with feature bits and properties
on ARM target already, so I won't ask to rewrite all of it to.
But since you need control only one feature than make it property
and set related feature bit from property setter or like it's done
for arm_cpu_has_el3_property.
Then in arm_cpu_parse_features() you can just do generic setting:

 object_property_parse(OBJECT(cpu), val, featurename, &local_err);

for foo=on/off format and forget about touching it again,
further down the road one would just need to add a property to
mange needed feature bits.