yamt commented on a change in pull request #3003: URL: https://github.com/apache/incubator-nuttx/pull/3003#discussion_r590871509
########## File path: arch/xtensa/include/esp32/memory_layout.h ########## @@ -72,9 +72,23 @@ * * When an internal heap is enabled this region starts at an offset equal to * the size of the internal heap. + * + * The QEMU bootloader image is slightly different than the chip's one. + * The ROM on PRO and APP uses different regions for static data. In QEMU, + * however, we load only one ROM binary, taken from the PRO CPU, and it is + * used by both CPUs. So, in QEMU, if we allocate this part early, it will + * be clobbered once the APP CPU starts. + * We can delay the allocation to when everything has started through the + * board_late_initiliaze hook, as is done for the APP data, however this + * should be fixed from QEMU side. The following macros, then, just skip + * PRO CPU's data when a QEMU image generation is enabled with SMP. */ -#define HEAP_REGION2_START 0x3ffe0450 +#if defined(CONFIG_ESP32_QEMU_IMAGE) && defined(CONFIG_SMP) Review comment: for me, a separate config is more convenient. * CONFIG_ESP32_WITH_QEMU without CONFIG_ESP32_QEMU_IMAGE makes sense when building qemu images manually from nuttx or nuttx.bin. (i almost always do this when using qemu for xtensa) * CONFIG_ESP32_QEMU_IMAGE without CONFIG_ESP32_WITH_QEMU might makes sense when using a fixed qemu in future. having said that, i can live with the single CONFIG_ESP32_QEMU_IMAGE config. it isn't a blocker of the PR at all. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org