qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 09/15] target-arm: Add ARMCPU secure property


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2 09/15] target-arm: Add ARMCPU secure property
Date: Mon, 15 Dec 2014 17:21:57 +0000

On 15 December 2014 at 17:18, Greg Bellows <address@hidden> wrote:
>
>
> On 15 December 2014 at 11:01, Peter Maydell <address@hidden>
> wrote:
>>
>> On 11 December 2014 at 23:29, Greg Bellows <address@hidden>
>> wrote:
>> > +static Property arm_cpu_has_el3_property =
>> > +            DEFINE_PROP_BOOL("has_el3", ARMCPU, has_el3, false);
>>
>> I think the default value here should be "true": we want
>> CPUs to default to having all their features turned on
>> unless the board specifically disables it.
>>
>> (This won't affect behaviour til we get to patch 15 because
>> before then ARM_FEATURE_EL3 isn't set and we never add
>> the property here.)
>
>
> I went with false in the initialization because it matches how features are
> initialized (disabled unless otherwise enabled).  In the case of EL3, it is
> disabled on CPUs by default unless specifically enabled.  This is also the
> case of the has_el3 property, disabled by default unless we see that the EL3
> feature is enabled.

This is correct...

> As-is, the tunable is set to true once we discover that
> the EL3 feature is set

...but this isn't. When you call qdev_property_add_static()
it will call object_property_set_bool() to set the flag to
the default value as specified in the Property struct, so
we'll end up with has_el3 set to false by default on CPUs
which can support EL3, which is not what we want.

In fact you're manually setting cpu->has_el3 = true in
the arm_cpu_post_init(). You should just set the property
struct's default value correctly in the first place...

thanks
-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]