tmedicci commented on code in PR #20125: URL: https://github.com/apache/nuttx/pull/20125#discussion_r4008718221
########## arch/risc-v/src/common/riscv_exception_common.S: ########## @@ -279,6 +286,52 @@ return_from_exception: REGLOAD s0, REG_EPC(sp) /* restore sepc */ csrw CSR_EPC, s0 +#ifdef REG_MCAUSE + /* Restore mcause with the frame. On a CLIC part mcause holds mpp, mpie and + * mpil; mret restores privilege from mpp and mintstatus.mil from mpil. + * Leaving it as whatever the last trap wrote makes mret restore the wrong + * privilege or the wrong interrupt level -- mil stuck at 63 + * masks every interrupt and deadlocks the system. The frame's + * value is by construction what the hardware latched for this trap, so it is + * correct at any nesting depth. + * + * Restored BEFORE mstatus, deliberately. mstatus.MPP/MPIE are aliased into + * mcause[29:28]/[27], so whichever is written last wins for those fields. + * Synthesised frames -- up_initial_state(), riscv_jump_to_user() -- leave + * this slot zero, and writing that after mstatus dropped kernel threads to + * U-mode and faulted on the first instruction fetch of kernel text. Writing + * mcause first lets mstatus stay authoritative for privilege while mcause + * still supplies mpil, which lives nowhere else. + */ + + REGLOAD s1, REG_MCAUSE(sp) Review Comment: Could you please review the synthetic frame paths before restoring this slot? [`common/supervisor/riscv_syscall.S`](https://github.com/apache/nuttx/blob/master/arch/risc-v/src/common/supervisor/riscv_syscall.S#L87-L130) builds a frame with `save_ctx`, status, EPC, and SP, but does not initialize `REG_MCAUSE`. `SYS_switch_context` can save that frame for later restoration. Please also check the other frame constructors. ########## arch/risc-v/src/common/espressif/esp_userspace.c: ########## @@ -0,0 +1,458 @@ +/**************************************************************************** + * arch/risc-v/src/common/espressif/esp_userspace.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <assert.h> +#include <stdint.h> +#include <string.h> + +#include <nuttx/compiler.h> +#include <nuttx/userspace.h> + +#include <arch/board/board_memorymap.h> + +#include "riscv_internal.h" + +#include "esp_userspace.h" + +#include "esp_rom_sys.h" +#include "soc/soc.h" +#include "hal/cache_hal.h" +#include "hal/cache_ll.h" +#include "hal/mmu_hal.h" +#include "hal/mmu_types.h" +#include "spi_flash_mmap.h" +#include "bootloader_flash_priv.h" + +#if defined(CONFIG_ESPRESSIF_SPIRAM) && \ + defined(CONFIG_ESPRESSIF_SPIRAM_USER_HEAP) +# include "esp_psram.h" +# include "esp_private/esp_psram_extram.h" +#endif + +#ifdef CONFIG_BUILD_PROTECTED + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define USER_IMAGE_OFFSET CONFIG_ESPRESSIF_USER_IMAGE_OFFSET + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/* Emitted at the start of the user image by user-space.ld. It tells the + * kernel where the user image expects its flash-mapped regions to live, so + * that the flash MMU can be programmed before any user code is reachable. + */ + +struct user_image_load_header_s +{ + uintptr_t drom_vma; /* Destination address (VMA) for DROM region */ + uintptr_t drom_lma; /* Flash offset (LMA) for start of DROM region */ + uintptr_t drom_size; /* Size of DROM region */ + uintptr_t irom_vma; /* Destination address (VMA) for IROM region */ + uintptr_t irom_lma; /* Flash offset (LMA) for start of IROM region */ + uintptr_t irom_size; /* Size of IROM region */ +}; + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static struct user_image_load_header_s g_header; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: load_header + * + * Description: + * Read the user image load header out of flash. + * + * Unlike the ESP32-C3 protected port, which temporarily mapped a scratch + * flash window to reach the header, this reads it directly. The SPI flash + * driver is already initialised by the time esp_userspace() runs, and + * esp_start() uses the same call to parse the kernel image header, so no + * scratch mapping is needed. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void load_header(void) +{ + int ret = bootloader_flash_read(USER_IMAGE_OFFSET, &g_header, + sizeof(g_header), true); + + if (ret != 0) + { + esp_rom_printf("ERROR: user image header read failed: rc=%d at 0x%x\n", + ret, (unsigned int)USER_IMAGE_OFFSET); + PANIC(); + } +} + +/**************************************************************************** + * Name: configure_mmu + * + * Description: + * Map the user image's flash-resident code and read-only data into the + * virtual address space. + * + * This is deliberately additive. esp_start()'s map_rom_segments() calls + * mmu_hal_unmap_all() before installing the kernel's own mappings; doing + * that here would unmap the kernel while it is executing. Only the two + * user regions are added. + * + * The cache is disabled across the update, so this function -- and + * everything it calls -- must be resident in IRAM. The linker script + * places this object there by name. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void noinline_function configure_mmu(void) +{ + uint32_t actual_mapped_len = 0; + uint32_t drom_lma = USER_IMAGE_OFFSET + g_header.drom_lma; + uint32_t irom_lma = USER_IMAGE_OFFSET + g_header.irom_lma; + uint32_t drom_lma_aligned = drom_lma & MMU_FLASH_MASK; + uint32_t drom_vma_aligned = g_header.drom_vma & MMU_FLASH_MASK; + uint32_t irom_lma_aligned = irom_lma & MMU_FLASH_MASK; + uint32_t irom_vma_aligned = g_header.irom_vma & MMU_FLASH_MASK; + cache_bus_mask_t bus_mask; + + cache_hal_disable(CACHE_LL_LEVEL_EXT_MEM, CACHE_TYPE_ALL); + + mmu_hal_map_region(0, MMU_TARGET_FLASH0, + drom_vma_aligned, drom_lma_aligned, + g_header.drom_size, &actual_mapped_len); + + mmu_hal_map_region(0, MMU_TARGET_FLASH0, + irom_vma_aligned, irom_lma_aligned, + g_header.irom_size, &actual_mapped_len); + + bus_mask = cache_ll_l1_get_bus(0, drom_vma_aligned, g_header.drom_size); + cache_ll_l1_enable_bus(0, bus_mask); + bus_mask = cache_ll_l1_get_bus(0, irom_vma_aligned, g_header.irom_size); + cache_ll_l1_enable_bus(0, bus_mask); + +#if CONFIG_ESPRESSIF_NUM_CPUS > 1 + bus_mask = cache_ll_l1_get_bus(1, drom_vma_aligned, g_header.drom_size); + cache_ll_l1_enable_bus(1, bus_mask); + bus_mask = cache_ll_l1_get_bus(1, irom_vma_aligned, g_header.irom_size); + cache_ll_l1_enable_bus(1, bus_mask); +#endif + +#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE + cache_ll_invalidate_addr(CACHE_LL_LEVEL_ALL, CACHE_TYPE_ALL, + CACHE_LL_ID_ALL, irom_vma_aligned, + actual_mapped_len); +#endif + + cache_hal_enable(CACHE_LL_LEVEL_EXT_MEM, CACHE_TYPE_ALL); +} + +/**************************************************************************** + * Name: initialize_data + * + * Description: + * Copy the user image's initialised data from flash into user RAM. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void initialize_data(void) +{ + size_t length = USERSPACE->us_dataend - USERSPACE->us_datastart; + int ret; + + uintptr_t src = USER_IMAGE_OFFSET + USERSPACE->us_datasource; + + ret = bootloader_flash_read(src, (void *)USERSPACE->us_datastart, + length, true); + if (ret != 0) + { + /* The usual cause is alignment: this read wants its flash offset and + * length aligned, and .data's load address is whatever the end of + * .rodata left it at. user-space.ld pads both to 16 for that reason. + */ + + esp_rom_printf("ERROR: user .data read failed: rc=%d src=0x%x " + "dst=0x%x len=0x%x\n", + ret, (unsigned int)src, + (unsigned int)USERSPACE->us_datastart, + (unsigned int)length); + PANIC(); + } +} + +/**************************************************************************** + * Name: configure_mpu + * + * Description: + * Establish the PMP regions that separate kernel from user. + * + * Every entry is reset first. By the time this runs the HAL has already + * programmed eleven regions from bootloader_init(), several of which would + * grant user mode access to kernel memory -- entry 5 in particular covers + * the whole of SRAM as read/write. They are only reprogrammable because + * CONFIG_ESPRESSIF_KERNEL_OWNS_PMP built them without the lock bit. + * + * Only user-accessible regions are then described. Per the RISC-V + * privileged specification an access matching no PMP entry is permitted in + * machine mode and denied in user mode, so the kernel needs no entries of + * its own and everything not listed below is automatically inaccessible to + * user code. + * + * Regions are expressed as TOR pairs because the linker-defined bounds are + * not naturally aligned powers of two. Every boundary must respect the + * 128-byte PMP granularity of this SoC; user-space.ld is responsible for + * that alignment. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void configure_mpu(void) +{ + const uintptr_t r = PMPCFG_R; + const uintptr_t rw = PMPCFG_R | PMPCFG_W; + const uintptr_t rx = PMPCFG_R | PMPCFG_X; + + /* Drop everything the HAL left behind. Machine mode is unaffected: an + * unlocked entry never constrains it, and an address matching no entry is + * permitted in machine mode. + */ + + riscv_config_pmp_region(0, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(1, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(2, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(3, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(4, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(5, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(6, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(7, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(8, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(9, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(10, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(11, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(12, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(13, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(14, PMPCFG_A_OFF, 0, 0); + riscv_config_pmp_region(15, PMPCFG_A_OFF, 0, 0); Review Comment: This clears only entries 0–15, but the ESP32-P4 revision-3+ HAL layout uses entries through 31. Could you guard this protected path with `CONFIG_ESP32P4_SELECTS_REV_LESS_V3` for now, or extend the common PMP API and reset all implemented entries? Otherwise entries 16–31 may retain HAL permissions. ########## boards/risc-v/esp32p4/common/scripts/esp32p4_protected_memory.ld: ########## @@ -0,0 +1,183 @@ +/**************************************************************************** + * boards/risc-v/esp32p4/common/scripts/esp32p4_protected_memory.ld + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * ESP32-P4 memory layout for the protected build. + * + * kernel-space.ld and user-space.ld link the kernel and user images into + * these blocks respectively. + * + * Three properties of this SoC shape the layout: + * + * 1. Instruction and data share one virtual window for external flash + * (SOC_MMU_DI_VADDR_SHARED). Unlike parts with separate IROM and DROM + * windows, all four flash regions are carved out of 0x40000000 upwards + * and are distinguished only by address range. + * + * 2. On silicon older than v3.0 the internal SRAM is two banks with a + * hardware gap. That maps neatly onto the kernel/user split: the kernel + * takes sram_low, user data takes sram_high, and the boundary needs no + * artificial alignment because the banks are already far apart. + * + * 3. PMP granularity is 128 bytes, so every region boundary that the PMP + * describes must be 128-byte aligned. This is why the metadata block + * below is 0x80 rather than the 0x18 used by parts with 4-byte + * granularity: UDROM starts immediately after it, and that start address + * becomes a PMP TOR boundary. + * + * Flash offsets must also satisfy the cache MMU constraint + * paddr % 64KB == vaddr % 64KB. All four flash regions are placed on 64 KB + * boundaries, with the metadata offset applied on top for UDROM. + ****************************************************************************/ + +#include <nuttx/config.h> + +/* common.ld would supply RESERVE_RTC_MEM and MSPI_WORKAROUND_SIZE, but it + * also pulls in the HAL's sdkconfig.h, which is not on the include path when + * the user image is linked in pass 1. This file is shared by both links, so + * the two constants are reproduced here instead. Values are the ESP32-P4 + * ones: 24 bytes of RTC timer retention, and no MSPI workaround because that + * applies to revision v3 silicon only. + */ + +#ifndef RESERVE_RTC_MEM +# define RESERVE_RTC_MEM 24 +#endif + +#ifndef MSPI_WORKAROUND_SIZE +# define MSPI_WORKAROUND_SIZE 0x0 +#endif + +/* Internal SRAM. Bank boundaries come from the flat layout so that the two + * stay in step; see esp32p4_flat_memory.ld. + */ + +#define SRAM_LOW_START 0x4ff00000 +#define SRAM_LOW_END 0x4ff2cbd0 /* 2nd stage bootloader iram_loader_seg */ +#define SRAM_LOW_SIZE (SRAM_LOW_END - SRAM_LOW_START) + +#define SRAM_HIGH_START 0x4ff40000 +/* CONFIG_CACHE_L2_CACHE_SIZE is a HAL sdkconfig alias that is not visible + * here; the NuttX-side symbol it is derived from is used directly instead. + */ + +#define SRAM_HIGH_SIZE (0x80000 - CONFIG_ESPRESSIF_CACHE_L2_CACHE_SIZE) + +/* Flash, mapped into the shared instruction/data window. The 0x20 offset on + * KIROM is the usual Espressif convenience: the image has a 0x18 byte file + * header and each segment an 0x08 byte header, so starting 0x20 in keeps + * paddr % 64KB == vaddr % 64KB satisfied for the first segment. + */ + +#define IDROM_SEG_SIZE (CONFIG_MMU_PAGE_SIZE << 10) Review Comment: `CONFIG_MMU_PAGE_SIZE` is not available while the pass-1 linker script is preprocessed, so this currently produces an invalid region length. Since this layout assumes 1024 64-KiB MMU pages, could we make that value explicit here? ```suggestion #define IDROM_SEG_SIZE 0x04000000 ``` ########## arch/risc-v/src/common/espressif/esp_userspace.c: ########## @@ -0,0 +1,458 @@ +/**************************************************************************** + * arch/risc-v/src/common/espressif/esp_userspace.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <assert.h> +#include <stdint.h> +#include <string.h> + +#include <nuttx/compiler.h> +#include <nuttx/userspace.h> + +#include <arch/board/board_memorymap.h> + +#include "riscv_internal.h" + +#include "esp_userspace.h" + +#include "esp_rom_sys.h" +#include "soc/soc.h" +#include "hal/cache_hal.h" +#include "hal/cache_ll.h" +#include "hal/mmu_hal.h" +#include "hal/mmu_types.h" +#include "spi_flash_mmap.h" +#include "bootloader_flash_priv.h" + +#if defined(CONFIG_ESPRESSIF_SPIRAM) && \ + defined(CONFIG_ESPRESSIF_SPIRAM_USER_HEAP) +# include "esp_psram.h" +# include "esp_private/esp_psram_extram.h" +#endif + +#ifdef CONFIG_BUILD_PROTECTED + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define USER_IMAGE_OFFSET CONFIG_ESPRESSIF_USER_IMAGE_OFFSET + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/* Emitted at the start of the user image by user-space.ld. It tells the + * kernel where the user image expects its flash-mapped regions to live, so + * that the flash MMU can be programmed before any user code is reachable. + */ + +struct user_image_load_header_s +{ + uintptr_t drom_vma; /* Destination address (VMA) for DROM region */ + uintptr_t drom_lma; /* Flash offset (LMA) for start of DROM region */ + uintptr_t drom_size; /* Size of DROM region */ + uintptr_t irom_vma; /* Destination address (VMA) for IROM region */ + uintptr_t irom_lma; /* Flash offset (LMA) for start of IROM region */ + uintptr_t irom_size; /* Size of IROM region */ +}; + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static struct user_image_load_header_s g_header; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: load_header + * + * Description: + * Read the user image load header out of flash. + * + * Unlike the ESP32-C3 protected port, which temporarily mapped a scratch + * flash window to reach the header, this reads it directly. The SPI flash + * driver is already initialised by the time esp_userspace() runs, and + * esp_start() uses the same call to parse the kernel image header, so no + * scratch mapping is needed. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void load_header(void) +{ + int ret = bootloader_flash_read(USER_IMAGE_OFFSET, &g_header, + sizeof(g_header), true); Review Comment: Could `load_header()` validate this metadata before `configure_mmu()` uses it? Please check expected UIROM/UDROM VMAs, nonzero sizes, overflow in `USER_IMAGE_OFFSET + lma`, flash bounds, and MMU alignment. Before clearing BSS or copying data, also validate the `USERSPACE` entry/trampoline and data/BSS/heap ranges against UIROM/UDRAM. These should be runtime checks, not only `DEBUGASSERT`; please check other `USERSPACE` consumers too. ########## arch/risc-v/src/common/espressif/esp_userspace.c: ########## @@ -0,0 +1,458 @@ +/**************************************************************************** + * arch/risc-v/src/common/espressif/esp_userspace.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <assert.h> +#include <stdint.h> +#include <string.h> + +#include <nuttx/compiler.h> +#include <nuttx/userspace.h> + +#include <arch/board/board_memorymap.h> + +#include "riscv_internal.h" + +#include "esp_userspace.h" + +#include "esp_rom_sys.h" +#include "soc/soc.h" +#include "hal/cache_hal.h" +#include "hal/cache_ll.h" +#include "hal/mmu_hal.h" +#include "hal/mmu_types.h" +#include "spi_flash_mmap.h" +#include "bootloader_flash_priv.h" + +#if defined(CONFIG_ESPRESSIF_SPIRAM) && \ + defined(CONFIG_ESPRESSIF_SPIRAM_USER_HEAP) +# include "esp_psram.h" +# include "esp_private/esp_psram_extram.h" +#endif + +#ifdef CONFIG_BUILD_PROTECTED + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +#define USER_IMAGE_OFFSET CONFIG_ESPRESSIF_USER_IMAGE_OFFSET + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +/* Emitted at the start of the user image by user-space.ld. It tells the + * kernel where the user image expects its flash-mapped regions to live, so + * that the flash MMU can be programmed before any user code is reachable. + */ + +struct user_image_load_header_s +{ + uintptr_t drom_vma; /* Destination address (VMA) for DROM region */ + uintptr_t drom_lma; /* Flash offset (LMA) for start of DROM region */ + uintptr_t drom_size; /* Size of DROM region */ + uintptr_t irom_vma; /* Destination address (VMA) for IROM region */ + uintptr_t irom_lma; /* Flash offset (LMA) for start of IROM region */ + uintptr_t irom_size; /* Size of IROM region */ +}; + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +static struct user_image_load_header_s g_header; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: load_header + * + * Description: + * Read the user image load header out of flash. + * + * Unlike the ESP32-C3 protected port, which temporarily mapped a scratch + * flash window to reach the header, this reads it directly. The SPI flash + * driver is already initialised by the time esp_userspace() runs, and + * esp_start() uses the same call to parse the kernel image header, so no + * scratch mapping is needed. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void load_header(void) +{ + int ret = bootloader_flash_read(USER_IMAGE_OFFSET, &g_header, + sizeof(g_header), true); + + if (ret != 0) + { + esp_rom_printf("ERROR: user image header read failed: rc=%d at 0x%x\n", + ret, (unsigned int)USER_IMAGE_OFFSET); + PANIC(); + } +} + +/**************************************************************************** + * Name: configure_mmu + * + * Description: + * Map the user image's flash-resident code and read-only data into the + * virtual address space. + * + * This is deliberately additive. esp_start()'s map_rom_segments() calls + * mmu_hal_unmap_all() before installing the kernel's own mappings; doing + * that here would unmap the kernel while it is executing. Only the two + * user regions are added. + * + * The cache is disabled across the update, so this function -- and + * everything it calls -- must be resident in IRAM. The linker script + * places this object there by name. + * + * Input Parameters: + * None. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static void noinline_function configure_mmu(void) +{ + uint32_t actual_mapped_len = 0; + uint32_t drom_lma = USER_IMAGE_OFFSET + g_header.drom_lma; + uint32_t irom_lma = USER_IMAGE_OFFSET + g_header.irom_lma; + uint32_t drom_lma_aligned = drom_lma & MMU_FLASH_MASK; + uint32_t drom_vma_aligned = g_header.drom_vma & MMU_FLASH_MASK; + uint32_t irom_lma_aligned = irom_lma & MMU_FLASH_MASK; + uint32_t irom_vma_aligned = g_header.irom_vma & MMU_FLASH_MASK; + cache_bus_mask_t bus_mask; + + cache_hal_disable(CACHE_LL_LEVEL_EXT_MEM, CACHE_TYPE_ALL); + + mmu_hal_map_region(0, MMU_TARGET_FLASH0, + drom_vma_aligned, drom_lma_aligned, + g_header.drom_size, &actual_mapped_len); + + mmu_hal_map_region(0, MMU_TARGET_FLASH0, + irom_vma_aligned, irom_lma_aligned, + g_header.irom_size, &actual_mapped_len); + + bus_mask = cache_ll_l1_get_bus(0, drom_vma_aligned, g_header.drom_size); + cache_ll_l1_enable_bus(0, bus_mask); + bus_mask = cache_ll_l1_get_bus(0, irom_vma_aligned, g_header.irom_size); + cache_ll_l1_enable_bus(0, bus_mask); + Review Comment: Small scope reminder: PMP CSRs are hart-local, while `configure_mpu()` currently runs only during CPU0 startup. Since SMP is not supported here yet, could this protected configuration be constrained or documented as single-core until PMP is initialized on every hart? ########## arch/risc-v/src/common/espressif/esp_region_protect.c: ########## @@ -0,0 +1,107 @@ +/**************************************************************************** + * arch/risc-v/src/common/espressif/esp_region_protect.c + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +/**************************************************************************** + * Unlocked build of the HAL's esp_cpu_configure_region_protection(). + * + * The HAL implementation programs every PMP entry with the lock bit set. It + * runs very early, from esp_start() -> bootloader_init() -> + * bootloader_init_mem(), and that path is not gated by + * CONFIG_ESPRESSIF_REGION_PROTECTION -- that option guards only a second, + * redundant call later in esp_start(). + * + * PMP lock bits cannot be cleared without the Smepmp extension, which the + * ESP32-P4 does not implement (reading mseccfg raises an illegal + * instruction). Once the HAL has run, 14 of the 16 entries are dead for the + * remainder of the boot, and one of the survivors grants U-mode read/write + * across the whole kernel data region. A protected build therefore has to + * stop the entries being locked in the first place. + * + * The same function also programs the 16 PMA entries that mark the invalid + * address ranges and make external flash/RAM, ROM and L2MEM cacheable. That + * work is required for the SoC to run at all: replacing this function with + * an empty stub boot-loops inside bootloader_init(). + * + * So rather than reimplement any of it, the HAL translation unit is dropped + * from the build (see hal_<chip>.mk / hal_<chip>.cmake) and its source is + * included below with PMP_L defined to zero. Including "riscv/csr.h" first + * means its include guard is already set when the HAL source includes it + * again, so the redefinition survives. Everything else -- the PMA setup, + * the chip-revision variants, the PSRAM handling -- stays byte for byte + * identical to the vendored source and tracks it when the HAL is updated. + * + * The result is the HAL's own region layout with every PMP entry left + * unlocked, so the kernel can re-describe them for a kernel/user split. + * + * NOTE: unlocked PMP entries do not constrain machine mode, so this reduces + * the protection a flat build gets. It is meant to be paired with a + * protected build, whose userspace initialisation re-establishes the + * boundaries. + ****************************************************************************/ + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +/* Pulled in first so that its include guard is already set when the HAL + * source below includes it again, keeping the redefinition that follows. + */ + +#include "riscv/csr.h" + +#undef PMP_L +#define PMP_L 0 + +/* The HAL implementation itself, compiled with the lock bit cleared. The + * chip directory is spelled out rather than derived from + * CONFIG_ESPRESSIF_CHIP_SERIES because an #include directive cannot + * concatenate string literals; CONFIG_ESPRESSIF_KERNEL_OWNS_PMP depends on + * ARCH_CHIP_ESP32P4, so this file is only ever built for that chip. + */ + +#include "../../chip/esp-hal-3rdparty/components/esp_hw_support/port/esp32p4/cpu_region_protect.c" Review Comment: A common source should not directly include an ESP32-P4 implementation. Since this Kconfig currently depends on `ARCH_CHIP_ESP32P4`, the simplest fix is to move this wrapper under `src/esp32p4/`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
