Re: [Qemu-devel] [PATCH for-4.2 12/24] target/arm: Add VHE system register redirection and aliasing

[Richard Henderson]

On 7/25/19 7:01 AM, Alex Bennée wrote:
>> +    union {
>> +        /*
>> +         * Offsets of the secure and non-secure fields in CPUARMState for
>> +         * the register if it is banked.  These fields are only used during
>> +         * the static registration of a register.  During hashing the bank
>> +         * associated with a given security state is copied to fieldoffset
>> +         * which is used from there on out.
>> +         *
>> +         * It is expected that register definitions use either fieldoffset
>> +         * or bank_fieldoffsets in the definition but not both.  It is also
>> +         * expected that both bank offsets are set when defining a banked
>> +         * register.  This use indicates that a register is banked.
>> +         */
>> +        ptrdiff_t bank_fieldoffsets[2];
> 
> It seems a bit off that we are compressing this structure into a union
> when we didn't bother with the above fieldoffset despite the statement
> that only one or the other is used.
...
> Is 2 pointers enough saving? Will we never see a re-directed register
> that was using the banked fieldoffsets? Can we protect against that?

It's because bank_fieldoffsets[] is only used before instantiation.

Before the structure is given to add_cpreg_to_hashtable, we have copied the
structure, copied bank_fieldoffsets into fieldoffset, and adjusted the secure
state flag.

After instantiation, those two pointers are unused, and I thought I could
reclaim it.  However, you may have a point re total memory savings not making
the complication worthwhile.


r~