I fixed the issue by adding naming and a couple of macros for short-cutting
the name. Interestingly and somewhat baffling, the only fields that needed
the fix were the bank_fieldoffsets and fieldoffsets. It appears to be
related to static initialization using these fields as all the CP15
anonymous unions/structs don't throw errors.
Greg
On 17 October 2014 10:20, Greg Bellows <greg.bell...@linaro.org> wrote:
> So, I believe I reproduced the issue with gcc-4.4. 4.6 and 4.7 appear to
> work fine. I tried adding the "-fms-extensions" flag through configure's
> "--extra-cflags" but it dow not appear to work. Not sure if this is a
> configure/make issue or if anonymous unions/structs are still disallowed.
>
> I'll look into other options in case we deem it important to support older
> compilers.
>
> Greg
>
> On 17 October 2014 08:37, Greg Bellows <greg.bell...@linaro.org> wrote:
>
>> Hmmm, I had not encountered this as I am using a new compiler which is
>> presumeably using -std=c11. Here is what I am using:
>>
>> > gcc --version
>> gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1
>>
>> A quick search on gcc 4.4 shows that a different flag may be
>> needed (-fms-extensions). There is also a special flag for 4.6 apparently.
>>
>> One question I have is how old of a toolchain do we support? Peter?
>>
>> In the meantime, I'll try and reproduce on my end and try the additional
>> flags.
>>
>> Thanks for pointing this out.
>>
>> Greg
>>
>> On 16 October 2014 20:32, Edgar E. Iglesias <edgar.igles...@gmail.com>
>> wrote:
>>
>>> On Fri, Oct 10, 2014 at 11:03:22AM -0500, Greg Bellows wrote:
>>> > From: Fabian Aggeler <aggel...@ethz.ch>
>>> >
>>> > Prepare ARMCPRegInfo to support specifying two fieldoffsets per
>>> > register definition. This will allow us to keep one register
>>> > definition for banked registers (different offsets for secure/
>>> > non-secure world).
>>>
>>> Hi Greg,
>>>
>>> I gave the series a try through my auto-tester and it fails on this
>>> patch with gcc-4.4:
>>> $ gcc-4.4 --version
>>> gcc-4.4 (Ubuntu/Linaro 4.4.7-8ubuntu1) 4.4.7
>>>
>>> We might need to pass additional options to gcc for the
>>> anonymous structs/unions or use a different approach.
>>>
>>> Cheers,
>>> Edgar
>>>
>>>
>>>
>>> >
>>> > Also added secure state tracking field and flags. This allows for
>>> > identification of the register info secure state.
>>> >
>>> > Signed-off-by: Fabian Aggeler <aggel...@ethz.ch>
>>> > Signed-off-by: Greg Bellows <greg.bell...@linaro.org>
>>> >
>>> > ==========
>>> >
>>> > v5 -> v6
>>> > - Separate out secure CPREG flags
>>> > - Add convenience macro for testing flags
>>> > - Removed extraneous newline
>>> > - Move add_cpreg_to_hashtable() functionality to a later commit for
>>> which it is
>>> > dependent on.
>>> > - Added comment explaining fieldoffset padding
>>> >
>>> > v4 -> v5
>>> > - Added ARM CP register secure and non-secure bank flags
>>> > - Added setting of secure and non-secure flags furing registration
>>> > ---
>>> > target-arm/cpu.h | 42 +++++++++++++++++++++++++++++++++++++++---
>>> > 1 file changed, 39 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>>> > index 59414f3..4d8de9e 100644
>>> > --- a/target-arm/cpu.h
>>> > +++ b/target-arm/cpu.h
>>> > @@ -985,6 +985,24 @@ enum {
>>> > ARM_CP_STATE_BOTH = 2,
>>> > };
>>> >
>>> > +/* ARM CP register secure state flags. These flags identify security
>>> state
>>> > + * attributes for a given CP register entry.
>>> > + * The existence of both or neither secure and non-secure flags
>>> indicates that
>>> > + * the register has both a secure and non-secure hash entry. A
>>> single one of
>>> > + * these flags causes the register to only be hashed for the specified
>>> > + * security state.
>>> > + * Although definitions may have any combination of the S/NS bits,
>>> each
>>> > + * registered entry will only have one to identify whether the entry
>>> is secure
>>> > + * or non-secure.
>>> > + */
>>> > +enum {
>>> > + ARM_CP_SECSTATE_S = (1 << 0), /* bit[0]: Secure state register
>>> */
>>> > + ARM_CP_SECSTATE_NS = (1 << 1), /* bit[1]: Non-secure state
>>> register */
>>> > +};
>>> > +
>>> > +/* Convenience macro for checking for a specific bit */
>>> > +#define ARM_CP_SECSTATE_TEST(_ri, _flag) (((_ri)->secure & (_flag))
>>> == (_flag))
>>> > +
>>> > /* Return true if cptype is a valid type field. This is used to try to
>>> > * catch errors where the sentinel has been accidentally left off the
>>> end
>>> > * of a list of registers.
>>> > @@ -1119,6 +1137,8 @@ struct ARMCPRegInfo {
>>> > int type;
>>> > /* Access rights: PL*_[RW] */
>>> > int access;
>>> > + /* Security state: ARM_CP_SECSTATE_* bits/values */
>>> > + int secure;
>>> > /* The opaque pointer passed to define_arm_cp_regs_with_opaque()
>>> when
>>> > * this register was defined: can be used to hand data through to
>>> the
>>> > * register read/write functions, since they are passed the
>>> ARMCPRegInfo*.
>>> > @@ -1128,12 +1148,28 @@ struct ARMCPRegInfo {
>>> > * fieldoffset is non-zero, the reset value of the register.
>>> > */
>>> > uint64_t resetvalue;
>>> > - /* Offset of the field in CPUARMState for this register. This is
>>> not
>>> > - * needed if either:
>>> > + /* Offsets of the fields (secure/non-secure) in CPUARMState for
>>> this
>>> > + * register. The array will be accessed by the ns bit which means
>>> the
>>> > + * secure instance has to be at [0] while the non-secure instance
>>> must be
>>> > + * at [1]. If a register is not banked .fieldoffset can be used,
>>> which maps
>>> > + * to the non-secure bank.
>>> > + *
>>> > + * Extra padding is added to align the default fieldoffset field
>>> with the
>>> > + * non-secure bank_fieldoffsets entry. This is necessary for
>>> maintaining
>>> > + * the same storage offset when AArch64 and banked AArch32 are
>>> seperately
>>> > + * defined.
>>> > + *
>>> > + * This is not needed if either:
>>> > * 1. type is ARM_CP_CONST or one of the ARM_CP_SPECIALs
>>> > * 2. both readfn and writefn are specified
>>> > */
>>> > - ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */
>>> > + union { /* offsetof(CPUARMState, field) */
>>> > + struct {
>>> > + ptrdiff_t _fieldoffset_padding;
>>> > + ptrdiff_t fieldoffset;
>>> > + };
>>> > + ptrdiff_t bank_fieldoffsets[2];
>>> > + };
>>> > /* Function for making any access checks for this register in
>>> addition to
>>> > * those specified by the 'access' permissions bits. If NULL, no
>>> extra
>>> > * checks required. The access check is performed at runtime, not
>>> at
>>> > --
>>> > 1.8.3.2
>>> >
>>>
>>
>>
>
