Re: [avr-libc-dev] [Patch] Include a device info note section in gcrt1.S

[Senthil Kumar Selvaraj]

On Mon, Aug 11, 2014 at 04:26:22PM +0200, Joerg Wunsch wrote:
> As Senthil Kumar Selvaraj wrote:
> 
> > I figured it's easier and less error-prone to embed this information
> > here and then consume it everywhere else. I have a patch for
> > binutils, for example, that uses the information in the note section
> > to print percentage of memory used - like what the WinAVR port of
> > avr-size does, but without the hardcoding.
> 
> I like that idea much more than Eric Weddington's hack.
> 
> I think the AVR is ultimately not the only device which might this be
> a concern of; all flash-based controllers can probably benefit from
> it.  Thinking a little more about the layout of that device
> information (so it can e.g. be applied to all flash-based ARM
> controllers as well) would IMHO much increase the likelihood of this
> being accepted as a general solution by the binutils folks.  After
> all, Atmel could go ahead, and apply it to their own ARM devices as
> a first example. ;-)

Yes, it makes sense to make it as general as possible.
> 
> > +    .section .note.gnu.avr.deviceinfo, "", @note
> > +#define NOTE_NAME "AVR"
> > +#ifdef __AVR_DEVICE_NAME__
> > +    #define DEVICE_NAME STR(__AVR_DEVICE_NAME__)
> > +#else
> > +    #define DEVICE_NAME ""
> > +#endif
> > +    .long .L__note_name_end - .L__note_name_start
> > +    .long .L__desc_end - .L__desc_start
> > +    .long 1 ; Type 1 - this is the only known note type for AVR.
> > +.L__note_name_start:
> > +    .asciz NOTE_NAME
> > +.L__note_name_end:
> > +.L__desc_start:
> > +    .long .L__device_name_end - .L__device_name_start
> > +.L__device_name_start:
> > +    .asciz DEVICE_NAME
> > +.L__device_name_end:
> > +#ifdef PROGMEM_SIZE
> > +    .long PROGMEM_SIZE
> > +#elif FLASHEND > 0
> > +    .long FLASHEND + 1
> > +#else
> > +    .long FLASHEND
> > +#endif
> > +#ifdef RAMSIZE
> > +    .long RAMSIZE
> > +#elif RAMEND > 0
> > +    .long RAMEND - RAMSTART + 1
> > +#else
> > +    .long RAMEND
> > +#endif
> > +#ifdef EEPROM_SIZE
> > +    .long EEPROM_SIZE
> > +#elif E2END > 0
> > +    .long E2END + 1
> > +#else
> > +    .long E2END
> > +#endif
> > +.L__desc_end:
> > +    .balign 4
> > +
> 
> Few things come to mind:
> 
> * Start out with those parts of the information that have a fixed
>   size, i.e. the sizes of the memory regions, and move the variable
>   parts behind.  That way, the initial part can be described as a C
>   structure.
> 
> * Wouldn't we also need start addresses?  In particular, if you think
>   beyond the horizon towards ARMs, their flash usually doesn't start
>   at 0, but even for the AVR, the RAM starts at a device-dependent
>   address.  Providing on the the size information without the start is
>   not very useful.  (OTOH, information that can already be derived
>   from other sources in the ELF file doesn't need to be duplicated.)
> 
> * For the strings, add an offset (or index) table.  That way, in order
>   to read the second string, you don't have to traverse the first one.
>   Thus, the actual string array somewhat resembles an ELF string
>   section.
> 
> * Please add a patch that explains the layout in the documentation,
>   too.

How does the below patch look? Once you guys find it ok, I'll follow it
up with a documentation patch. Note that the part above the
L__desc_start label (and the .balign directive at the end) are there as
per the ELF spec for a note section.

diff --git avr-libc/crt1/gcrt1.S avr-libc/crt1/gcrt1.S
index 2d341a4..721573b 100644
--- avr-libc/crt1/gcrt1.S
+++ avr-libc/crt1/gcrt1.S
@@ -304,3 +304,70 @@ __do_copy_data:
 #endif /* __AVR_ASM_ONLY__ */
 ;      .endfunc
 
+    .section .note.gnu.avr.deviceinfo, "", @note
+#define NOTE_NAME "AVR"
+#ifdef __AVR_DEVICE_NAME__
+    #define DEVICE_NAME STR(__AVR_DEVICE_NAME__)
+#else
+    #define DEVICE_NAME ""
+#endif
+
+    .long .L__note_name_end - .L__note_name_start
+    .long .L__desc_end - .L__desc_start
+    .long 1 ; Type 1 - this is the only known note type for AVR.
+.L__note_name_start:
+    .asciz NOTE_NAME
+.L__note_name_end:
+.L__desc_start:
+#ifdef FLASHSTART
+    .long FLASHSTART
+#else
+    .long 0
+#endif
+#ifdef PROGMEM_SIZE
+    .long PROGMEM_SIZE
+#elif FLASHEND > 0
+    .long FLASHEND + 1
+#else
+    .long FLASHEND
+#endif
+    .long RAMSTART
+#ifdef RAMSIZE
+    .long RAMSIZE
+#elif RAMEND > 0
+    .long RAMEND - RAMSTART + 1
+#else
+    .long RAMEND
+#endif
+#ifdef E2START
+    .long E2START
+#else
+    .long 0
+#endif
+#ifdef EEPROM_SIZE
+    .long EEPROM_SIZE
+#elif E2END > 0
+    .long E2END + 1
+#else
+    .long E2END
+#endif
+    /* String offsets table.
+    Index 0 - Size of offset table in bytes
+    Index 1 - Device name byte offset
+    */
+.L__stroffsettab_start:
+.long .L__stroffsettab_end - .L__stroffsettab_start /* Size of index table in 
bytes */
+.long .L__device_name_start - .L__strtab_start /* Byte offset of device name */
+.L__stroffsettab_end:
+    /* String table for storing arbitrary strings.
+    Offsets are stored in the string offset table above */
+.L__strtab_start:
+    .byte 0
+.L__device_name_start:
+    .asciz DEVICE_NAME
+.L__device_name_end:
+    .byte 0
+.L__strtab_end:
+.L__desc_end:
+    .balign 4
+

> 
> -- 
> cheers, Joerg               .-.-.   --... ...--   -.. .  DL8DTL
> 
> http://www.sax.de/~joerg/
> Never trust an operating system you don't have sources for. ;-)