Re: [Qemu-devel] [PATCH v2 00/23] target-arm: add Security Extensions for CPUs

[Peter Maydell]

On 16 May 2014 21:56, Greg Bellows <address@hidden> wrote:
> Overview:
> The approach delicately hashes all the SP registers just as Fabian's
> approach does.  The primary difference is that all the existing ARMCPRegInfo
> entries are left as-is and are blindly registered for both NS-bit settings
> (encoded in the hash key).  Once all of the existing registers have been
> added to the hash, the set of security specific registers are hashed.
> Hashing of these registers involves a single new type to indicate that the
> ARMCPRegInfo is a security update of an existing register.  On receipt of
> this type, a hash lookup is performed on the register and it is updated for
> the security specifics.  This primarily involves updating the fieldoffset to
> point at the secure bank.
>
> There are benefits to both approaches.  In the above approach it avoids to
> mark all register declarations as secure or banked and isolates the security
> registers to their own mechanism.  This can also be considered a detriment
> as the existing registers are not explicitly marked.

I definitely prefer the approach where we have all the information
about a particular register (including secure/nonsecure/AArch64/AArch32
variants) in a single place.

> @@@ -809,12 -760,6 +809,12 @@@ static inline uint64_t cpreg_to_kvm_id(
>   #define ARM_CP_OVERRIDE 16
>   #define ARM_CP_NO_MIGRATE 32
>   #define ARM_CP_IO 64
>  +/* Secure CP register type.
>  + * This flag indicates that the ARMCPRegInfo is a secure register entry.
>  + * This flag includes override as the registration may override the
>  + * previously registered attributes.
>  + */
>  +#define ARM_CP_SECURE (ARM_CP_OVERRIDE | 128)

This is definitely a bad idea -- we should be trying to reduce the
use of the OVERRIDE flag, because it pretty much always indicates
that something's wrong (mostly we use it to work around overly
wildcarded entries that we still have for legacy reasons).

thanks
-- PMM