arch/avr/avrdx: do not copy const variables into RAM

AVR uses Hardward architecture with separate address space for program
memory (flash) and data memory (RAM). Normal program flow can only
access data memory which means that all variables - including const
variables - have to be copied into RAM to be accessible. (This happens
automatically during startup.)

It is possible to work around this limitation in software but that
can have severe impact on performance and/or API complexity. It is hardly
feasible to change NuttX interfaces in a way that would allow to make use
of this workaround.

On newer AVR families, there is an alternative option enabled by this patch.
These chips map part of their program memory (a 32kB window) into data
memory address space. This patch leverages this feature and adds support
for placing const variables into the mapped window. No copy to RAM is done
for them.

Const variables are therefore loaded directly from flash (not consuming
RAM) while still being available to be used by any NuttX interface.

Linker script of breadxavr board is changed to make use of these changes.

Tested by verifying string addresses - parameters in printf call
in a custom application (and also by running the application and verifying
its output.)

Documentation tested by build.

Signed-off-by: Kerogit <[email protected]>

Author: Kerogit

Documentation/platforms/avr/avrdx/boards/breadxavr/index.rst

@@ -1,3 +1,5 @@
+.. _breadxavr_board:
+
 ========================
 AVR128DA28 on breadboard
 ========================

Documentation/platforms/avr/common/constants-in-progmem.rst

@@ -178,3 +178,48 @@ Note that both ``IOBJ`` and ``IPTR`` need to be activated by
 If this configuration option is not set, both macros are defined
 to be empty and all strings will be copied to RAM (performance penalty
 discussed above is therefore removed as well.)
+
+Using memory-mapped flash
+=========================
+
+Newer AVR devices - tinyAVR and AVR DA/DB family - have their program
+memory mapped into upper 32kB half of data memory address space.
+(If the program memory size exceeds 32kB, only a 32kB-sized window
+is mapped. This is controlled by NVM peripheral within the chip.
+On current chips, the top window is mapped by default.)
+
+This can be leveraged in a way that makes these AVR devices behave
+as a von Neumann architecture. With proper configuration in a linker
+script, all constants can be placed into the mapped program memory
+region where they will be accessible for both load from program memory
+instructions and load from data address space instructions.
+
+As long as these constants fit into the 32kB window, this is a best
+available option on devices that support it. It combines advantages
+of all previous options and doesn't have any of their drawbacks.
+The performance penalty is negligible (flash read is few cycles slower
+than RAM read), RAM is not consumed and all variables are fully
+available to be used as parameters for any kernel interface.
+
+Unlike previous options, using this one is fully controlled by board's
+linker script. The linker script needs to place the constants
+(eg. ``rodata`` section) to appropriate memory location.
+
+Despite that, there is still a configuration option
+:menuselection:`System Type --> Use memory-mapped access to flash`,
+which is selected by default on devices that support this method
+of not copying data from program memory to RAM. Setting it unlocks
+additional configuration options
+:menuselection:`Size of .rodata FLMAP section` and
+:menuselection:`Offset of .rodata FLMAP section` which may be used
+to further configure section sizes. Note that these values are
+only made available to the linker and board's linker script needs
+to be designed to obey them.
+
+To have these configuration options available, the board needs
+to select ``AVR_HAVE_BOARD_FLMAP`` in its configuration. It declares
+that its linker script will obey ``__RODATA_SIZE__`` and
+``__RODATA_OFFSET__`` symbols (which are set by the above-mentioned
+configuration options.)
+
+See the linker script of :ref:`breadxavr_board` for an example.

arch/avr/Kconfig

@@ -28,6 +28,7 @@ config ARCH_CHIP_AVRDX
 	select ARCH_FAMILY_AVR
 	select MM_SMALL
 	select ARCH_HAVE_TICKLESS
+	select AVR_HAVE_FLMAP
 	---help---
 		Atmel/Microchip AVR 32/64/128 DA/DB core family.
 

arch/avr/src/avr/Kconfig

@@ -68,22 +68,38 @@ config AVR_BUILDROOT_TOOLCHAIN
 
 endchoice # Toolchain
 
+choice
+	prompt "Const variable placement"
+	default AVR_CONST_TO_FLMAP if AVR_HAVE_BOARD_FLMAP && AVR_HAVE_FLMAP
+	default AVR_HAS_MEMX_PTR if ARCH_DEBUG_H
+	default AVR_CONST_TO_RAM
+
+config AVR_CONST_TO_RAM
+	bool "No special handling, copy to RAM"
+	---help---
+		Initialization code will copy const variables into RAM.
+		This is a standard option available for all compilers and it is
+		fully supported in the kernel (because there are no special
+		requirements for such support.)
+
+		Main disadvantage of this option is that it may severely reduce
+		RAM available for the application and it may even be impossible
+		to fit all data into the RAM.
+
 config AVR_HAS_MEMX_PTR
-	bool "Enable in-flash static const strings"
+	bool "Mark const variables with __memx"
 	depends on AVR_ATMEL_AVR_TOOLCHAIN || AVR_LINUXGCC_TOOLCHAIN
-	default y if ARCH_DEBUG_H
-	default n
 	---help---
 		Enabling this option activates IOBJ and IPTR qualifiers
 		for pointers in the source code. Compiler will then be allowed
-		to place constants into program memory without copying it to RAM,
+		to place constants into program memory without copying them to RAM,
 		reducing amount of RAM needed to hold static data.
 
 		The compiler then extends pointers with these qualifiers enabled
 		to 24bit length with highest bit set for data that reside in RAM.
 		Based on this bit, it will then read the data using instructions
 		appropriate for the underlying storage. As such, there is
-		a performance tradeoff.
+		a potentially significant performance tradeoff.
 
 		Additionally, if this is enabled, all constant strings used
 		for debugging and assertion are placed into program memory,
@@ -94,8 +110,87 @@ config AVR_HAS_MEMX_PTR
 		pointers in arbitrary interaction with the kernel. Not all API
 		functions have these qualifiers added to their parameters.
 
+config AVR_CONST_TO_FLMAP
+	bool "Use memory-mapped access to flash"
+	depends on AVR_HAVE_BOARD_FLMAP && AVR_HAVE_FLMAP
+	---help---
+		Newer AVR chips - namely tinyAVR and AVR families - have (part of)
+		their program memory (flash) mapped into data memory address space.
+		The mapping is limited to 32kB window.
+
+		With this option enabled, const variables are kept in the program
+		memory and no copy to RAM is done. Yet it is still possible to use
+		such variables in any interaction with the kernel as they are
+		visible in data memory address space.
+
+		Note that this option only triggers some basic configuration
+		in the init function. It is the linker script of the board that needs
+		to ensure variables are placed correctly.
+
+		Beware that FLMAP bits in NVMCTRL.CTRLB I/O register which select
+		the segment of program memory to be mapped may not be changed freely
+		by the application. If the application needs to change the mapping,
+		it may only do so while observing these rules:
+
+		1. No kernel function must be called until the original value
+		is restored.
+
+		2. Interrupts must be disabled for as long as the value is changed.
+
+endchoice # Const variable placement
+
+config AVR_FLMAP_RODATA_SIZE
+	int "Size of .rodata FLMAP section"
+	depends on AVR_CONST_TO_FLMAP
+	default 4096
+	---help---
+		Specify size of .rodata memory section, ie. the section that stores
+		const variables. This will be passed as a parameter to the linker
+		to be used by the board's linker script.
+
+		Value must be divisible by 512 and no more than 32 kilobytes
+		is possible.
+
+config AVR_FLMAP_RODATA_OFFSET
+	int "Offset of .rodata FLMAP section"
+	depends on AVR_CONST_TO_FLMAP
+	default 0
+	---help---
+		Specify size of memory block between end of the .rodata section
+		(counting its full size as defined in AVR_FLMAP_RODATA_SIZE)
+		and the end of flash.
+
+		This value is intended to leave the end of the flash unused,
+		presumably for the purpose of placing APPDATA section in there
+		(see the chip documentation for details about subdividing
+		the program flash to BOOT, APPCODE and APPDATA sections.)
+
+		Note that this value is only passed to the linker to be used
+		by the linker script - the script then needs to place
+		the .rodata section accordingly.
+
+		Value must be divisible by 512 and no more than 31 kilobytes
+		is possible. Sum of this value and AVR_FLMAP_RODATA_SIZE must
+		also not exceed 32kB.
 
 config AVR_HAS_RAMPZ
 	bool
 
+config AVR_HAVE_BOARD_FLMAP
+	bool
+	---help---
+		This configuration option is supposed to be selected by board's
+		Kconfig if the board's linker script responds to __RODATA_SIZE__
+		and __RODATA_OFFSET__ passed by the linker and if it configures
+		.rodata section's size and position accordingly. Configuration
+		options that allow the user to configure values in these symbols
+		are unlocked if this is set and if the chip has support
+		for memory-mapped flash
+
+config AVR_HAVE_FLMAP
+	bool
+	---help---
+		This configuration option is set by chips that have (at least
+		some part of their) flash mapped into data memory address space.
+
 endif # ARCH_FAMILY_AVR

arch/avr/src/avr/Toolchain.defs

@@ -106,6 +106,16 @@ ifeq ($(CONFIG_DEBUG_OPT_UNUSED_SECTIONS),y)
   ARCHOPTIMIZATION += -ffunction-sections -fdata-sections
 endif
 
+# .rodata size for FLMAP configuration
+ifeq ($(CONFIG_AVR_CONST_TO_FLMAP),y)
+  LDFLAGS += --defsym=__RODATA_SIZE__=$(CONFIG_AVR_FLMAP_RODATA_SIZE)
+endif
+
+# .rodata offset for FLMAP configuration
+ifeq ($(CONFIG_AVR_CONST_TO_FLMAP),y)
+  LDFLAGS += --defsym=__RODATA_OFFSET__=$(CONFIG_AVR_FLMAP_RODATA_OFFSET)
+endif
+
 ifeq ($(CONFIG_DEBUG_LINK_MAP),y)
   LDFLAGS += -Map=$(call CONVERT_PATH,$(TOPDIR)$(DELIM)nuttx.map)
 endif

arch/avr/src/avrdx/avrdx_head.S

@@ -492,6 +492,61 @@ __start:
 	out		_SFR_IO_ADDR(SPH), r29
 	out		_SFR_IO_ADDR(SPL), r28
 
+
+#ifdef CONFIG_AVR_HAVE_FLMAP
+
+	/* Configure FLMAP bits in NVMCTRL.CTRLB in case this was software
+	 * reset and they are not in default state.
+	 *
+	 * Don't care if the application actually changes the register. These
+	 * bits are not under configuration protection (CCP) and runaway
+	 * application can possibly change them.
+	 *
+	 * This is executed if the chip supports it, regardless of if the board
+	 * declared it is using the feature by setting AVR_HAVE_BOARD_FLMAP
+	 * in Kconfig (it may be using it without setting that option.)
+	 */
+
+	ldi		r26, lo8(NVMCTRL_CTRLB)
+	ldi		r27, hi8(NVMCTRL_CTRLB)
+	ld		r16, X
+
+	/* Default value (on current chips anyway) uses all bits set to one. */
+
+#  if !defined(NVMCTRL_FLMAP_1_bm)
+
+	/* Might be there won't be such (supported) device ever. */
+
+#    error Chips without FLMAP bit 1 are not supported
+
+#  elif !defined(NVMCTRL_FLMAP_2_bm)
+
+	/* Expected case */
+
+	ldi		r17, NVMCTRL_FLMAP_0_bm | NVMCTRL_FLMAP_1_bm;
+
+#  else
+
+	/* Future device with more than 128kB RAM. The default is expected
+	 * to be all bits set to one but it needs to be verified when
+	 * support for such chip is added. Issue a warning.
+	 */
+
+#    warning Support for more than 128kB flash was done blindly here
+
+	ldi		r17, NVMCTRL_FLMAP_0_bm | NVMCTRL_FLMAP_1_bm | NVMCTRL_FLMAP_2_bm;
+
+#  endif
+
+	/* As long as we are always setting bits to one, we don't need
+	 * to clear them in the original value
+	 */
+
+	or		r16, r17
+	st		X, r16
+
+#endif
+
 	/* Copy initial global data values from FLASH into RAM */
 
 	.global	__do_copy_data;			/* Required to suppress dragging in logic from libgcc */

boards/Kconfig

@@ -94,6 +94,7 @@ config ARCH_BOARD_AVRDX_BREADXAVR
 	select ARCH_HAVE_LEDS
 	select ARCH_HAVE_BUTTONS
 	select ARCH_HAVE_IRQBUTTONS
+	select AVR_HAVE_BOARD_FLMAP
 	---help---
 		This is a board used to make something use an AVRnDx core
 		so it can be developed and tested.

boards/avr/avrdx/breadxavr/scripts/breadxavr.ld

@@ -30,11 +30,55 @@
  * *Memory configuration A
  */
 
+/* Use this instead of repeated magical number */
+__CHIP_FLASH_SIZE__ = 128K;
+
+/* VMA above 0x800000, must not collide with anything else,
+ * arbitrary value otherwise.
+ */
+__RODATA_VMA__ = 0xa00000;
+/* Application configuration is supposed to pass this value
+ * to linker so the default should not be used.
+ */
+__RODATA_SIZE__ = DEFINED(__RODATA_SIZE__) ? __RODATA_SIZE__ : 4K;
+/* Size increment matches chip's memory organization (boot, application
+ * code and application data can be sized with 512B increments.)
+ */
+ASSERT (__RODATA_SIZE__ % 512 == 0,
+        "__RODATA_SIZE__ must be a multiple of 512")
+/* Only 32kB of flash is mapped to data memory */
+ASSERT (__RODATA_SIZE__ <= 32K,
+        "__RODATA_SIZE__ must not be larger than 32kB")
+
+/* Same as for __RODATA_SIZE__ */
+__RODATA_OFFSET__ = DEFINED(__RODATA_OFFSET__) ? __RODATA_OFFSET__ : 0;
+/* Size increment matches chip's memory organization (boot, application
+ * code and application data can be sized with 512B increments.)
+ */
+ASSERT (__RODATA_OFFSET__ % 512 == 0,
+        "__RODATA_OFFSET__ must be a multiple of 512")
+/* Only 32kB of flash is mapped to data memory */
+ASSERT (__RODATA_OFFSET__ <= 31K,
+        "__RODATA_OFFSET__ must not be larger than 31kB")
+
+/* Also verify sum of size and offset */
+ASSERT (__RODATA_SIZE__ + __RODATA_OFFSET__ <= 32K,
+        "Sum of __RODATA_SIZE__ and __RODATA_OFFSET__ must not be larger than 32kB")
+
+/* Set the origin to the very end of flash. 64K is a magical number
+ * that constitutes of 32K (start of memory-mapped flash in data
+ * memory address space) and 32K (end of the area, size of the rodata
+ * section is subtracted from it.)
+ */
+__RODATA_ORIGIN__ = __RODATA_VMA__ + 64K - __RODATA_SIZE__ - __RODATA_OFFSET__;
+__RODATA_FLASH_START = __CHIP_FLASH_SIZE__ - __RODATA_SIZE__ - __RODATA_OFFSET__;
+
 MEMORY
 {
-  flash    (rx) : ORIGIN = 0, LENGTH = 128K
+  flash    (rx) : ORIGIN = 0, LENGTH = __CHIP_FLASH_SIZE__
   sram   (rw!x) : ORIGIN = 0x804000, LENGTH = 16K
   eeprom (rw!x) : ORIGIN = 0x801400, LENGTH = 512
+  rodata  (r!x) : ORIGIN = __RODATA_ORIGIN__, LENGTH = __RODATA_SIZE__
 }
 
 ENTRY(__start)
@@ -129,8 +173,6 @@ SECTIONS
   {
     _sdata = ABSOLUTE(.);
     *(.data .data.*)
-    *(.rodata)
-    *(.rodata*)
     *(.gnu.linkonce.d.*)
     CONSTRUCTORS
     _edata = ABSOLUTE(.);
@@ -154,6 +196,14 @@ SECTIONS
     _enoinit = ABSOLUTE(.);
   } > sram
 
+  .rodata ABSOLUTE(__RODATA_ORIGIN__) : AT (ABSOLUTE(__RODATA_FLASH_START))
+  {
+    *(.rodata)
+    *(.rodata*)
+    *(.gnu.linkonce.r*)
+    _rodata_end = ABSOLUTE(.);
+  } > rodata
+
   .eeprom     :
   {
     _seeprom = ABSOLUTE(.);