risc-v/esp32p4: BUILD_PROTECTED support, with PSRAM in the user heap

Documentation/platforms/risc-v/esp32p4/boards/esp32p4-tab5/index.rst

@@ -200,6 +200,20 @@ We can use the interrupt pin to send a signal when the interrupt fires::
 The pin is configured as a rising edge interrupt, so after issuing the
 above command, connect it to 3.3V.
 
+knsh
+----
+
+This is identical to the ``nsh`` configuration below except that NuttX is
+built in protected mode (``CONFIG_BUILD_PROTECTED``): the kernel and the user
+applications are linked as two separate images, the kernel runs in machine
+mode and user code in user mode, and the PMP enforces the split.
+
+The console is UART0 rather than the USB Serial/JTAG port, so connect to the
+board's UART adapter rather than ``/dev/ttyACM0``.
+
+The build produces two images, ``nuttx.bin`` and ``nuttx_user.bin``; see
+`Building and flashing`_ for the offsets.
+
 lvgl_demo
 ---------
 
@@ -301,3 +315,19 @@ Then open the console:
 
    $ picocom -b 115200 /dev/ttyACM0
    nsh>
+
+Protected build
+---------------
+
+The ``knsh`` configuration produces a second image for user space, which is
+flashed at ``CONFIG_ESPRESSIF_USER_IMAGE_OFFSET`` (``0x110000`` by default):
+
+.. code-block:: console
+
+   $ ./tools/configure.sh esp32p4-tab5:knsh
+   $ make -j
+   $ esptool.py -c esp32p4 -p /dev/ttyACM0 -b 921600 write_flash \
+       0x2000 nuttx.bin 0x110000 nuttx_user.bin
+
+``make flash ESPTOOL_PORT=<port>`` does both in one step.  The console for this
+configuration is UART0 at 115200, not the USB Serial/JTAG port.

arch/risc-v/Kconfig

@@ -222,6 +222,7 @@ config ARCH_CHIP_ESP32P4
 	select ARCH_RV_ISA_M
 	select ARCH_RV_ISA_A
 	select ARCH_RV_ISA_C
+	select ARCH_RV_HAVE_CLIC
 	select ARCH_VECNOTIRQ
 	select ARCH_HAVE_BOOTLOADER if !ESPRESSIF_SIMPLE_BOOT
 	select ARCH_HAVE_I2CRESET
@@ -776,6 +777,15 @@ config ARCH_RV_HAVE_APLIC
 		Controller (APLIC) to provide flexible interrupt control. This device
 		is not backward compatible with PLIC.
 
+config ARCH_RV_HAVE_CLIC
+	bool
+	default n
+	---help---
+		RISC-V defines Core-Level Interrupt Controller (CLIC) to provide
+		flexible interrupt control. When enabled, CLIC uses interrupt
+		threshold (MINTTHRESH/SINTTHRESH) CSRs for interrupt control
+		instead of the standard IE bit in STATUS register.
+
 config ARCH_RV_EXT_AIA
 	bool "Enable RISC-V SxAIA support"
 	default n

arch/risc-v/include/irq.h

@@ -201,7 +201,30 @@
 
 #define REG_INT_CTX_NDX     32
 
-#ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+/* On a CLIC part, mcause carries mpp / mpie / mpil / interrupt -- state that
+ * mret consults -- so it must be saved and restored with the frame, not just
+ * read for dispatch.  Uses the ARCH_RISCV_INTXCPT_EXTREGS extension slot,
+ * so REG_INT_CTX and everything below it keep their existing offsets.
+ *
+ * Only a protected build needs this: a flat build never returns to a lower
+ * privilege level, so nothing consumes the saved mpp.  Keeping the frame at
+ * its original size there leaves every flat RISC-V target untouched.
+ */
+
+#if defined(CONFIG_ARCH_RV_HAVE_CLIC) && !defined(CONFIG_BUILD_FLAT)
+#  define REG_MCAUSE_NDX    33
+#endif
+
+#ifdef REG_MCAUSE_NDX
+
+/* The frame carries mcause as well; keep it sized to match. */
+
+#  ifdef CONFIG_ARCH_RISCV_INTXCPT_EXTREGS
+#    define INT_XCPT_REGS   (34 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
+#  else
+#    define INT_XCPT_REGS   34
+#  endif
+#elif defined(CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
 #  define INT_XCPT_REGS     (33 + CONFIG_ARCH_RISCV_INTXCPT_EXTREGS)
 #else
 #  define INT_XCPT_REGS     33
@@ -347,6 +370,9 @@
 #  define REG_X30           (INT_REG_SIZE*REG_X30_NDX)
 #  define REG_X31           (INT_REG_SIZE*REG_X31_NDX)
 #  define REG_INT_CTX       (INT_REG_SIZE*REG_INT_CTX_NDX)
+#ifdef REG_MCAUSE_NDX
+#  define REG_MCAUSE        (INT_REG_SIZE*REG_MCAUSE_NDX)
+#endif
 
 #ifdef CONFIG_ARCH_FPU
 #  define REG_F0            (INT_REG_SIZE*REG_F0_NDX)
@@ -426,6 +452,9 @@
 #  define REG_X30           REG_X30_NDX
 #  define REG_X31           REG_X31_NDX
 #  define REG_INT_CTX       REG_INT_CTX_NDX
+#ifdef REG_MCAUSE_NDX
+#  define REG_MCAUSE        REG_MCAUSE_NDX
+#endif
 
 #ifdef CONFIG_ARCH_FPU
 #  define REG_F0            REG_F0_NDX

arch/risc-v/src/common/espressif/CMakeLists.txt

@@ -23,6 +23,14 @@
 set(SRCS)
 
 # Head/startup file
+if(CONFIG_ESPRESSIF_KERNEL_OWNS_PMP)
+  list(APPEND SRCS esp_region_protect.c)
+endif()
+
+if(CONFIG_BUILD_PROTECTED)
+  list(APPEND SRCS esp_userspace.c)
+endif()
+
 list(APPEND SRCS esp_head.S)
 
 # Custom vector table (skip common RISC-V vector table)

arch/risc-v/src/common/espressif/Kconfig

@@ -34,6 +34,19 @@ config ESPRESSIF_FLASH_32M
 
 endchoice # ESPRESSIF_FLASH
 
+config ESPRESSIF_P4DBG
+	bool "ESP32-P4 protected-build debug counters"
+	default n
+	depends on ARCH_CHIP_ESP32P4 && !BUILD_FLAT
+	---help---
+		Maintain counters and CLIC state snapshots in the interrupt, idle and
+		timer paths. Emit progress markers from board bring-up using the
+		ROM printf rather than syslog.
+
+		The counters should be read with a debugger.
+
+		Debug aid, do not activate in a production build.
+
 config ESPRESSIF_DONT_USE_ROM_LIBC
 	bool "Don't use ROM libc functions"
 	default n
@@ -238,6 +251,84 @@ config ESPRESSIF_REGION_PROTECTION
 	---help---
 		Configure the MPU to disable access to invalid memory regions.
 
+		Must be disabled in a protected build; esp_start.c fails the build
+		if it is not.  It only adds a second, redundant call to
+		esp_cpu_configure_region_protection() late in esp_start() -- the
+		regions have already been programmed from bootloader_init() by then
+		-- and in a protected build the kernel owns the PMP and re-describes
+		it during userspace initialisation, so the extra call is pointless
+		at best and racy against that setup at worst.
+
+		This is enforced with a compile-time check rather than
+		"depends on !BUILD_PROTECTED".  This symbol selects ARCH_USE_MPU and
+		BUILD_PROTECTED depends on ARCH_USE_MPU, so a dependency on the
+		build type closes a loop that Kconfig resolves by making
+		BUILD_PROTECTED unsatisfiable -- which silently collapses the whole
+		"Memory organization" choice for every board in the tree.
+
+config ESPRESSIF_KERNEL_OWNS_PMP
+	bool "Kernel owns the PMP configuration"
+	depends on ARCH_CHIP_ESP32P4
+	default n
+	select ARCH_USE_MPU
+	---help---
+		Drop the HAL's esp_cpu_configure_region_protection() from the build
+		and supply a NuttX build of the same code with the PMP lock bit
+		cleared.  The region layout, and the PMA setup the SoC needs in
+		order to run at all, are unchanged; only the lock bit differs.
+
+		The HAL version sets the lock bit on every entry it writes, and runs
+		from bootloader_init() before NuttX can intervene.  PMP lock bits
+		are irreversible without the Smepmp extension, which the ESP32-P4
+		does not implement, so a protected build has to prevent the locking
+		rather than undo it.
+
+		A protected build requires this and must set it explicitly in its
+		defconfig; esp_start.c fails the build if the two disagree.  It is
+		deliberately not "default y if BUILD_PROTECTED": this symbol selects
+		ARCH_USE_MPU, BUILD_PROTECTED depends on ARCH_USE_MPU, and a default
+		conditioned on BUILD_PROTECTED closes that loop.  Kconfig resolves
+		the circularity by making BUILD_PROTECTED unsatisfiable, which
+		silently collapses the whole "Memory organization" choice for every
+		board in the tree.
+
+		Enabling it on its own is legitimate only for bring-up: unlocked
+		entries do not constrain machine mode, so a flat build ends up with
+		less protection than it would otherwise have.
+
+if BUILD_PROTECTED
+
+config ESPRESSIF_USER_IMAGE_OFFSET
+	hex "User image offset"
+	default 0x110000
+	---help---
+		Offset in SPI Flash for flashing the User application firmware
+		image (nuttx_user.bin).
+
+		Must be aligned to the flash MMU page size (64 KB on ESP32-P4) so
+		that the cache MMU constraint paddr % 64KB == vaddr % 64KB can be
+		met, and must be beyond the end of the kernel image.
+
+endif # BUILD_PROTECTED
+
+config ESPRESSIF_PMP_EARLY_SNAPSHOT
+	bool "Dump PMP state during early boot (diagnostic)"
+	default n
+	---help---
+		Bring-up diagnostic.  Prints the raw PMP configuration via the ROM
+		printf immediately before and after the bootloader_init() call in
+		esp_start(), long before the console driver exists.
+
+		Use it to determine whether PMP entries are already locked when
+		NuttX gains control, or whether the lock bits are set by
+		bootloader_init() -> bootloader_init_mem() ->
+		esp_cpu_configure_region_protection().  PMP lock bits cannot be
+		cleared without the Smepmp extension, so a protected-mode port has
+		to prevent the locking rather than undo it, and that requires
+		knowing which code sets it.
+
+		This is a debug aid only; leave it disabled in normal builds.
+
 config ESPRESSIF_RUN_IRAM
 	bool "Run from IRAM"
 	default n

arch/risc-v/src/common/espressif/Make.defs

@@ -39,6 +39,14 @@ CHIP_CSRCS += esp_irq.c esp_gpio.c esp_rtc_gpio.c esp_libc_stubs.c
 CHIP_CSRCS += esp_lowputc.c esp_serial.c
 CHIP_CSRCS += esp_systemreset.c
 
+ifeq ($(CONFIG_ESPRESSIF_KERNEL_OWNS_PMP),y)
+  CHIP_CSRCS += esp_region_protect.c
+endif
+
+ifeq ($(CONFIG_BUILD_PROTECTED),y)
+  CHIP_CSRCS += esp_userspace.c
+endif
+
 ifeq ($(CONFIG_ARCH_HAVE_EXTRA_HEAPS),y)
 	CHIP_CSRCS += esp_extraheaps.c
 	ifeq ($(CONFIG_ESPRESSIF_RETENTION_HEAP),y)

arch/risc-v/src/common/espressif/esp_allocateheap.c

@@ -31,6 +31,8 @@
 
 #include <arch/board/board.h>
 #include <nuttx/arch.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/userspace.h>
 #include <nuttx/board.h>
 #include <nuttx/mm/mm.h>
 
@@ -103,7 +105,20 @@ void up_allocate_heap(void **heap_start, size_t *heap_size)
 
   board_autoled_on(LED_HEAPALLOCATE);
 
-#if defined(CONFIG_MM_KERNEL_HEAP) && \
+#if defined(CONFIG_BUILD_PROTECTED) && defined(CONFIG_MM_KERNEL_HEAP)
+  /* In a protected build this call describes the USER heap, not the
+   * kernel's.  It runs from the end of the user image's .bss to the top of
+   * the user RAM region, both of which the user image published in its
+   * userspace structure.  esp_userspace() has already granted user mode
+   * read/write over that span in the PMP.
+   *
+   * Using the kernel's own SRAM here -- as the flat path below does -- would
+   * hand user mode memory it cannot touch and would overlap the kernel heap.
+   */
+
+  *heap_start = (void *)USERSPACE->us_bssend;
+  *heap_size  = USERSPACE->us_heapend - USERSPACE->us_bssend;
+#elif defined(CONFIG_MM_KERNEL_HEAP) && \
     defined(CONFIG_ESPRESSIF_SPIRAM) && \
     defined(CONFIG_ESPRESSIF_SPIRAM_USER_HEAP)
   DEBUGASSERT(esp_psram_is_initialized());
@@ -182,9 +197,14 @@ void up_allocate_kheap(void **heap_start, size_t *heap_size)
 #if CONFIG_MM_REGIONS > 1
 void riscv_addregion(void)
 {
-#if defined(CONFIG_ESP32P4_SELECTS_REV_LESS_V3)
+#if defined(CONFIG_ESP32P4_SELECTS_REV_LESS_V3) && \
+    !defined(CONFIG_BUILD_PROTECTED)
   /* ESP32-P4 rev < v3 has non-contiguous SRAM: sram_low + sram_high.
    * The primary heap is in sram_low. Add sram_high as a second region.
+   *
+   * Not in a protected build: there sram_high is the user RAM region
+   * (see esp32p4_protected_memory.ld), so adding it to the kernel heap
+   * would hand the kernel memory that belongs to user space.
    */
 
   extern uint8_t _sram_high_heap_start[];
@@ -202,8 +222,21 @@ void riscv_addregion(void)
     }
 #endif
 
-#if !defined(CONFIG_MM_KERNEL_HEAP)
-#  if defined(CONFIG_ESPRESSIF_SPIRAM_USER_HEAP)
+  /* External PSRAM is user memory.  kumm_addregion() resolves to
+   * mm_addregion(USR_HEAP, ...), and USR_HEAP is the single heap in a flat
+   * build and (*USERSPACE->us_data->us_heap) in the kernel phase of a
+   * protected one, so the same call reaches user-accessible memory in both.
+   *
+   * configure_mpu() has already granted user mode read/write over this
+   * span.  The two must agree: a region added here without the grant is in
+   * the heap but faults on first touch from user code.
+   *
+   * A flat build that keeps a separate kernel heap is deliberately left as
+   * it was; changing where PSRAM lands there is not part of this change.
+   */
+
+#if defined(CONFIG_ESPRESSIF_SPIRAM_USER_HEAP) && \
+    (defined(CONFIG_BUILD_PROTECTED) || !defined(CONFIG_MM_KERNEL_HEAP))
   if (esp_psram_is_initialized())
     {
       uintptr_t start = esp_psram_extram_vaddr_start();
@@ -214,7 +247,6 @@ void riscv_addregion(void)
           kumm_addregion((void *)start, end - start);
         }
     }
-#  endif
 #endif
 }
 #endif