CYG_HAL_TABLE_END alignment

CYG_HAL_TABLE_END alignment

[Peter Korsgaard]

Hi,

Why is CYG_HAL_TABLE_END aligned? It seems to be a conscious decission
according to hal/common/current/ChangeLog:

2000-09-04  Jonathan Larmour  <jlarmour@redhat.com>

        * include/hal_tables.h (CYG_HAL_TABLE_END): Use CYGARC_P2ALIGNMENT
        to align label

But this gives errors if the size of the table elements aren't a
multiple of the alignment (8 bytes).

I had an issue with that today using the I2C infrastructure, since the
cyg_i2c_bus structure is 44 bytes big with assertions enabled -
causing an never ending loop in cyg_i2c_init::cyg_i2c_init.

-- 
Bye, Peter Korsgaard

Re: CYG_HAL_TABLE_END alignment

[Bart Veer]

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

    Peter> Why is CYG_HAL_TABLE_END aligned? It seems to be a
    Peter> conscious decission according to
    Peter> hal/common/current/ChangeLog:

    Peter> 2000-09-04  Jonathan Larmour  <jlarmour@redhat.com>

    Peter> * include/hal_tables.h (CYG_HAL_TABLE_END): Use CYGARC_P2ALIGNMENT
    Peter>   to align label

    Peter> But this gives errors if the size of the table elements
    Peter> aren't a multiple of the alignment (8 bytes).

    Peter> I had an issue with that today using the I2C
    Peter> infrastructure, since the cyg_i2c_bus structure is 44 bytes
    Peter> big with assertions enabled - causing an never ending loop
    Peter> in cyg_i2c_init::cyg_i2c_init.

I had not seen that before, but I think I understand the issues.
Suppose you are putting a structure like the following in a table:

    struct fred {
        cyg_uint32	a;
	cyg_uint8	b[3];
    }

On most architectures the compiler and linker will arrange for the
start of this structure to be aligned to a 4-byte boundary, i.e. a
byte of padding gets inserted. When the compiler iterates through the
table it thinks it has a fred[] array with each entry properly
aligned. The linker does not see an array, it gets a number of
individual variables, but it will align each one's start to the
appropriate boundary. So no problems so far.

Linker alignment only applies to the start of a structure, not the
end. Therefore when the linker places the end of the table label this
would appear immediately after the last entry, without the padding.
That is a bad idea so the end is explicitly aligned in the code.

Now, ideally the table end would be aligned to the same boundary as a
struct fred. Unfortunately the CYG_HAL_TABLE_END() was not written to
take an alignment argument, and instead it uses CYGARC_P2ALIGNMENT. On
many architectures (including the ones I have run the I2C code on) the
natural alignment is 4 bytes and there are no problems. However
CYGARC_P2ALIGNMENT refers to the largest alignment that may be needed
on the current architecture. It is possible that a struct fred will
work happily with something smaller, and the compiler will assume the
smaller alignment. Hence the linker has inserted too much padding, the
table is too large, and the i2c init code will try to init a
non-existant entry and fail to detect the end of the table.

Ideally the table macros would take an extra argument to indicate the
actual alignment needed for the structures in each table, i.e.
sizeof(struct fred), but since the tables are defined via inline
assembler that could get tricky. Also changing the macros at this
stage is probably not a good idea, they get used in many places.

Instead I suspect the cyg_i2c_bus structure should have been given an
appropriate attribute to increase its alignment to the maximum, at the
cost of a small amount of memory. That way the compiler and the linker
will agree on how all the table entries and the end get aligned, and
things should work. The following would probably do the trick.

Index: i2c.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/i2c/current/include/i2c.h,v
retrieving revision 1.1
diff -u -r1.1 i2c.h
--- i2c.h	20 Apr 2005 12:25:30 -0000	1.1
+++ i2c.h	28 Apr 2005 19:24:08 -0000
@@ -92,7 +92,7 @@
     void                    (*i2c_stop_fn)(const cyg_i2c_device*);
     // A spare field for use by the driver
     void*                   i2c_extra;
-} cyg_i2c_bus;
+} CYG_HAL_TABLE_TYPE cyg_i2c_bus;
 
 #define CYG_I2C_BUS(_name_, _init_fn_, _tx_fn_, _rx_fn_, _stop_fn_, _extra_)    \
     cyg_i2c_bus _name_  CYG_HAL_TABLE_ENTRY( i2c_buses ) = {                    \

Bart

P.S. I'll take a look at your I2C patches when I get a chance,
probably this weekend.

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts

Re: CYG_HAL_TABLE_END alignment

[Peter Korsgaard]

>>>>> "Bart" == Bart Veer <bartv@ecoscentric.com> writes:

 Bart>     struct fred { 
 Bart>      cyg_uint32 a; 
 Bart>      cyg_uint8 b[3];
 Bart>     }

 Bart> On most architectures the compiler and linker will arrange for
 Bart> the start of this structure to be aligned to a 4-byte boundary,
 Bart> i.e. a byte of padding gets inserted. When the compiler
 Bart> iterates through the table it thinks it has a fred[] array with
 Bart> each entry properly aligned. The linker does not see an array,
 Bart> it gets a number of individual variables, but it will align
 Bart> each one's start to the appropriate boundary. So no problems so
 Bart> far.

Yes, but the padding byte is "inside" the structure,
E.G. sizeof(struct fred) == 8 - isn't it?

 Bart> Linker alignment only applies to the start of a structure, not
 Bart> the end. Therefore when the linker places the end of the table
 Bart> label this would appear immediately after the last entry,
 Bart> without the padding.  That is a bad idea so the end is
 Bart> explicitly aligned in the code.

But immediately after the last entry is correct, as that is where the
next element would be placed.

To me the alignment directive should simply be removed from the
CYG_HAL_TABLE_END macro - or am I missing something?

 Bart> P.S. I'll take a look at your I2C patches when I get a chance,
 Bart> probably this weekend.

Thanks!

-- 
Bye, Peter Korsgaard

Re: CYG_HAL_TABLE_END alignment

[Bart Veer]

>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

    Bart> On most architectures the compiler and linker will arrange
    Bart> for the start of this structure to be aligned to a 4-byte
    Bart> boundary, i.e. a byte of padding gets inserted. When the
    Bart> compiler iterates through the table it thinks it has a
    Bart> fred[] array with each entry properly aligned. The linker
    Bart> does not see an array, it gets a number of individual
    Bart> variables, but it will align each one's start to the
    Bart> appropriate boundary. So no problems so far.

    Peter> Yes, but the padding byte is "inside" the structure, E.G.
    Peter> sizeof(struct fred) == 8 - isn't it?

Normally yes. However when people start adding packed attributes etc.
into the picture things get messy. The HAL table macros are supposed
to work irrespective of the data types used. I do not know the
circumstances which caused the alignment to be added to the table end
macro, but assume it was something along the lines I described.

Forcing the alignment for CYG_HAL_TABLE_END and using
CYG_HAL_TABLE_TYPE on the structure definition eliminates any
ambiguity and is the correct fix for the i2c code. Removing the
alignment in the CYG_HAL_TABLE_END would presumably break whatever
scenario caused the alignment to be added in the first place.

Bart

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts