On 3 December 2014 at 20:06, Greg Bellows <greg.bell...@linaro.org> wrote:
> Added a "secure" state property to the ARMCPU descriptor. This property
> indicates whether the ARMCPU is enabled for secure state or not. By default
> it
> is disabled at this time.
Shouldn't this feature be "has_el3" ? It's the configurable
version of the ARM_FEATURE_EL3.
> Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
> ---
> target-arm/cpu-qom.h | 2 ++
> target-arm/cpu.c | 24 ++++++++++++++++++++++++
> 2 files changed, 26 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index dcfda7d..8dab91b 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -100,6 +100,8 @@ typedef struct ARMCPU {
> bool start_powered_off;
> /* CPU currently in PSCI powered-off state */
> bool powered_off;
> + /* CPU secure state enabled */
> + bool secure;
>
> /* PSCI conduit used to invoke PSCI methods
> * 0 - disabled, 1 - smc, 2 - hvc
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 01afed2..0e660f9 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -388,6 +388,9 @@ static Property arm_cpu_reset_hivecs_property =
> static Property arm_cpu_rvbar_property =
> DEFINE_PROP_UINT64("rvbar", ARMCPU, rvbar, 0);
>
> +static Property arm_cpu_secure_property =
> + DEFINE_PROP_BOOL("secure", ARMCPU, secure, false);
> +
> static void arm_cpu_post_init(Object *obj)
> {
> ARMCPU *cpu = ARM_CPU(obj);
> @@ -407,6 +410,14 @@ static void arm_cpu_post_init(Object *obj)
> qdev_property_add_static(DEVICE(obj), &arm_cpu_rvbar_property,
> &error_abort);
> }
> +
> + if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
> + /* Add the secure state CPU property only if EL3 is allowed. This
> will
> + * prevent "secure" from existing on non EL3 enabled machines.
"on CPUs which cannot support EL3".
> + */
> + qdev_property_add_static(DEVICE(obj), &arm_cpu_secure_property,
> + &error_abort);
> + }
> }
>
> static void arm_cpu_finalizefn(Object *obj)
> @@ -476,6 +487,19 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
> **errp)
> cpu->reset_sctlr |= (1 << 13);
> }
>
> + if (arm_feature(env, ARM_FEATURE_V6) && !cpu->secure) {
> + /* The security extension and ID_PFR1 only apply to ARMv6 and up. IF
> + * this is the case and secure state has not been enabled then we
> + * disable the security extension feature.
> + */
> + unset_feature(env, ARM_FEATURE_EL3);
You don't need to conditionalize this on V6 being set;
just do
if (!cpu->secure) {
unset_feature(...);
}
to reflect the property setting into the feature bits. The
property won't exist in the first place on pre-v6 cores.
> +
> + /* Disable the security extension feature bits in the processor
> feature
> + * register as well. This is id_pfr1[7:4].
> + */
> + cpu->id_pfr1 &= ~0xf0;
This is a bit of a tricky area, because we don't in general
mess with our ID register values to match the features we do or
don't happen to implement, so this would be an odd special case.
Can we get away without touching PFR1 here?
> + }
> +
> register_cp_regs_for_features(cpu);
> arm_cpu_register_gdb_regs_for_features(cpu);
thanks
-- PMM
