Support to run NuttX on ESP32 QEMU #437

maht · 2020-03-05T12:41:30Z

Some code changes to allow setup a build and run environment to for ESP32 QEMU.

It has code to setup the ESP-IDF and QEMU and compile some dependencies from a hello_world project (maybe this can be skipped somehow, further work is needed if desired).

Not sure if this kind of "extra code" is desired in main NuttX repository, so no problem if this PR is not allowed to be merged, but could be useful as reference for others.

patacongo · 2020-03-05T14:07:10Z

tools/Makefile.unix

+		esptool.py --chip esp32 elf2image --flash_mode dio --flash_size 4MB -o nuttx.bin nuttx && \
+		echo "Generated: $(NUTTXNAME).bin (ESP32 compatible)"; \
+	fi
+endif


CPU specific logic is FORBIDDEN outside of the appropriate arch/ and board/ directories. This cannot be committed.

A more appropriate place to do this would be arch/xtensa/src/Makefile

This is required to support a proper modular architecture. If people put compiler-specific, CPU-specific, or any other platform-specific logic outside of those directories, then we damage the architecture. That has never been permitted in the past and and will never be permitted in the future.

There are CPU specific tools under the tools/directory: pic32mx and zds. That is also a precedent for CPU-specific, build-related logic. I am not sure how you integrate it, however.

Thanks for the feedback. Yes, I figured out that this code was not good enough to be ready to merge, that is the reason I wanted to work more on it before uploading ... but some people requested some info about how to use QEMU, and it was easier to just create the pull request to share and accelerate the feedback loop. I will try to see if there is a way to move the code to proper location.

But now I remember: I did it because it seemed that there was a precedent some lines before in CONFIG_CXD56_BINARY (lines 483 to 495). Not that I want to repeat it if that is a "bad practice" or special case/rationale behind it.

I do have to fight on an almost daily basis to maintain good modularity in the OS, but I feel like I am losing the battle. I am being overwhelmed by self-serving, ill-conceived changes that keep the submitter from having do the amount of work that is necessary to maintain the modularity.

Sorry to hear that, I don't wanted to cause trouble. But in my defense, if something is not good in the first place, it generates a dilemma on new additions: should we care more about consistency with existing practices (even bad ones) or should we do the correct one despite that will create inconsistencies. The ideal solution should migrate everything, but that puts a burden on the people contributing that was not involved on the original bad practice. So, I will suggest that we should identify that bad practices and at least, if not fix them, mark them as bad in the code (like some FIXME commment). That way the new contributors could be more aware about the preference of other merits over inconsistency in that particular case.

Sorry to hear that, I don't wanted to cause trouble. But in my defense, if something is not good in the first place, it generates a dilemma on new additions: should we care more about consistency with existing practices (even bad ones) or should we do the correct one despite that will create inconsistencies.

Yes, that is exactly the dilemma. If one bad example is permitted into the architecture, then it provides a precedent and justification for perpetuating that bad example. The only solution is to be more diligent in reviewing changes in the first place. But that is not humanly possible, at least not with the people involved now. Bad things will slip in unnoticed and once they have slipped in, then the problems really begin.

I don't have a good solution to that. But we also can't allow one bad implementation to justify its propagation.

Ok, then try to move the other examples of bad practice of this case or at least leave a message in the code if the efforts spans more than a week or so.

I think once there is no "bad example" on place, it would be easier to anyone to point out what is accepted and what is not, so the human review effort would be less, hopefully.

And by the way, from my part, thanks for being so strict :) It pays off on the long term.

I will try to port some of the other cases if I have some time or contribute as needed.

PR #457 provides a resolution for the CXD56xx. If moves the CXD56xx-specific logic out of tools/Makefile.unix&win and into boards/arm/cxd56xx/scripts. This is the same solution that was used for another architecture so has a solid precedence, works very well, and is very simple.

OK.. PR #457 has been merged. Can you please follow the example there there to implement the POSTBUILD hook for the ESP32. You only need to look at:

https://github.com/apache/incubator-nuttx/blob/master/boards/arm/cxd56xx/scripts/Config.mk - New file needed containing the POSTBUILD operations

and

https://github.com/apache/incubator-nuttx/blob/master/boards/arm/cxd56xx/spresense/scripts/Make.defs - Add only like to to include the above file AFTER including tools/Conflg.mk

And that is it!

patacongo · 2020-03-05T14:12:30Z

tools/Makefile.unix

+		esptool.py --chip esp32 elf2image --flash_mode dio --flash_size 4MB -o nuttx.bin nuttx && \
+		echo "Generated: $(NUTTXNAME).bin (ESP32 compatible)"; \
+	fi
+endif


This is required to support a proper modular architecture. If people put compiler-specific, CPU-specific, or any other platform-specific logic outside of those directories, then we damage the architecture. That has never been permitted in the past and and will never be permitted in the future.

There are CPU specific tools under the tools/directory: pic32mx and zds. That is also a precedent for CPU-specific, build-related logic. I am not sure how you integrate it, however.

xiaoxiang781216 · 2020-03-05T14:42:09Z

tools/Makefile.unix

@@ -501,6 +501,23 @@ ifeq ($(CONFIG_UBOOT_UIMAGE),y)
 		cp -f uImage /tftpboot/uImage; \
 	fi
 endif
+ifeq ($(CONFIG_ESP32_BINARY),y)
+	@echo "MKIMAGE: ESP32 binary"
+	$(Q) if ! esptool.py version | grep "v2[.]"; then \


Could we let QEMU directly load ELF image instead binary? So we don't include the speicial version of gcc from silicon vendor.

I tried without success, but maybe there is some way, since I am not familiar with it, sorry.

I am thinking that a more systematic solution is needed both for CXD56xx and ESP32 and for any future platforms that require custom binary generation after the final linking step. I have started some more general discussion in #442

I am thinking that perhaps we should support some Makefiles and tools in directories under tools/. Then perhaps we could do a wildcard check for:

(TOPDIR)/tools/$(CONFIG_ARCH)/Makefile

And if it exists, then run it. So in this case, it would check for tools/esp32/Makefile and would execture tools/esp32/Makefile to generate the binary using the changes in this PR.

This is more effort, but would serve the needs of all architectures, existing and future.

Loading an ELF file directly should be possible, provided that the ELF file includes the necessary CPU startup code and exception vectors. This doesn't happen when you build an ESP-IDF application — currently it always relies on the ROM code to provide the startup code and the exception vectors. However i think that NuttX includes such an option: https://github.com/apache/incubator-nuttx/blob/119a38ce10d8be96b8373b5a19145eda231edd93/boards/xtensa/esp32/esp32-core/README.txt#L491-L499 (i haven't tried it myself).

The document mentioned loading the code using OpenOCD, but same logic applies to loading the code over GDB (using the qemu built-in GDB server).

Thanks for the info, I will try, it could be a simpler way than the current approach.

masayuki2009 · 2020-03-06T03:07:15Z

@maht, thanks for sending this PR. I confirmed that nuttx works on ESP32 QEMU. (SMP configuration also works!)

However, I noticed that the difference between nsh/defconfig and qemu-nsh/defconfig is just the following setting.

CONFIG_SUPPRESS_UART_CONFIG=y

Actually I noticed that this setting works with ESP32-DevKitC board. (Perhaps, bootloader already set UART clock correctly). So I think we should apply the above settings to all configurations so that we can run all configurations on QEMU and also remove qemu-nsh/defconfig.

What do you think?

xiaoxiang781216 · 2020-03-06T18:10:10Z

@maht you can define POSTBUILD function to provide the special post process with this patch now:
#457
Of course, it's better to make ELF image loadable in QEMU if it's possible.

maht · 2020-03-06T19:27:10Z

@maht, thanks for sending this PR. I confirmed that nuttx works on ESP32 QEMU. (SMP configuration also works!)

However, I noticed that the difference between nsh/defconfig and qemu-nsh/defconfig is just the following setting.

CONFIG_SUPPRESS_UART_CONFIG=y

Actually I noticed that this setting works with ESP32-DevKitC board. (Perhaps, bootloader already set UART clock correctly). So I think we should apply the above settings to all configurations so that we can run all configurations on QEMU and also remove qemu-nsh/defconfig.

What do you think?

Yes, I agree. If it does not interfere with running on the real board it would be simpler to include in all of them and remove special cases. Thanks for trying it and the feedback.

maht · 2020-03-06T19:30:53Z

@maht you can define POSTBUILD function to provide the special post process with this patch now:
#457
Of course, it's better to make ELF image loadable in QEMU if it's possible.

Yes, I will try to modify accordingly to #457 and ELF image if possible. I just need to find some spare time, but if there some hurry on this let me know.

patacongo · 2020-03-16T17:55:01Z

@maht What is the status of this PR? It it your intention to complete the requsted changes? Or did you plan to just close this PR?

maht · 2020-03-17T09:21:18Z

@maht What is the status of this PR? It it your intention to complete the requsted changes? Or did you plan to just close this PR?

@patacongo Sorry for the lack of progress on this. There were personal issues related with the current emergency situation in Europe that made me be less focused than usual. My intention is to complete the requested changes, but maybe it takes some extra days. If there is someone else to complete them so they can be merged earlier, it would be ok to me though.

maht · 2020-03-21T19:31:12Z

@patacongo I have perform the requested changed except the direct usage of ELF binary: I was unable to make it run properly, so I am leaving it for further improvement. Please check the code and aside of that, maybe you know the reason for the error bin/Make.dep:6371: *** missing separator. Stop. in sim/udgram. Should be related to #468 but I can't see the same solution applies to my code. I did a simple try to reproduce it but the error does not appear. Before trying to reproduce using the exact CI scripts, which would take more effort, I ask you in case it rings a bell to you. If not I will try to explore deeper. Thanks in advance.

patacongo · 2020-03-21T19:58:14Z

The failed test does not seem to have anything to do with ESP32 Qemu. Should I go ahead and merge anyway?

patacongo · 2020-03-21T20:02:15Z

Yes, I will go ahead and merge. This change effects only ESP32 and not sim or mips or arm or riscv or x86. I am breaking the rules but I think it is the correct thing to do in this case. It seems that something is broken in the sim build and that should not block your change.

patacongo

LGTM

maht · 2020-03-21T22:57:46Z

Yes, I will go ahead and merge. This change effects only ESP32 and not sim or mips or arm or riscv or x86. I am breaking the rules but I think it is the correct thing to do in this case. It seems that something is broken in the sim build and that should not block your change.

Thanks Greg

xiaoxiang781216 · 2020-03-22T05:32:19Z

@maht this PR break the nightly build:
https://builds.apache.org/blue/organizations/jenkins/NuttX-Nightly-Build/detail/NuttX-Nightly-Build/71/pipeline
please help to fix the problem.

xiaoxiang781216 · 2020-03-22T08:18:36Z

@maht @btashton provide a fix here:
apache/nuttx-testing#18

…and-mutliphy-support ESP32s3 t3pw-dev add ADC and Multiphy support

maht mentioned this pull request Mar 5, 2020

esp32-core: Fix BOARD_CLOCK_FREQUENCY #413

Merged

patacongo reviewed Mar 5, 2020

View reviewed changes

patacongo requested changes Mar 5, 2020

View reviewed changes

xiaoxiang781216 reviewed Mar 5, 2020

View reviewed changes

patacongo mentioned this pull request Mar 5, 2020

tools/Makefile.unix: Remove FORBIDDEN CXD56 code in common Makefile. #442

Closed

patacongo mentioned this pull request Mar 6, 2020

Build System: Resolve CPU-specific logic in common build files #457

Merged

Support to run NuttX on ESP32 QEMU

11b86f0

maht force-pushed the esp32_qemu branch from 010876f to 11b86f0 Compare March 19, 2020 14:44

maht requested a review from patacongo March 21, 2020 19:24

patacongo approved these changes Mar 21, 2020

View reviewed changes

patacongo merged commit d410a29 into apache:master Mar 21, 2020

xiaoxiang781216 mentioned this pull request Mar 22, 2020

Add Dockerfile for CI system and hook into GitHub Actions apache/nuttx-testing#17

Merged

Ouss4 mentioned this pull request Mar 22, 2020

Configure python environment and install esptool apache/nuttx-testing#18

Merged

cwespressif pushed a commit to cwespressif/incubator-nuttx that referenced this pull request Apr 3, 2024

Merge pull request apache#437 from midokura/feature/t3pw-dev-add-adc-…

107468b

…and-mutliphy-support ESP32s3 t3pw-dev add ADC and Multiphy support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support to run NuttX on ESP32 QEMU #437

Support to run NuttX on ESP32 QEMU #437

maht commented Mar 5, 2020

patacongo Mar 5, 2020

patacongo Mar 5, 2020

patacongo Mar 5, 2020

maht Mar 5, 2020

maht Mar 5, 2020

maht Mar 5, 2020

patacongo Mar 5, 2020 •

edited

maht Mar 5, 2020

patacongo Mar 6, 2020

patacongo Mar 6, 2020

patacongo Mar 5, 2020

xiaoxiang781216 Mar 5, 2020

maht Mar 5, 2020

patacongo Mar 5, 2020

igrr Mar 5, 2020 •

edited

maht Mar 6, 2020

masayuki2009 commented Mar 6, 2020

xiaoxiang781216 commented Mar 6, 2020

maht commented Mar 6, 2020

maht commented Mar 6, 2020

patacongo commented Mar 16, 2020

maht commented Mar 17, 2020

maht commented Mar 21, 2020

patacongo commented Mar 21, 2020

patacongo commented Mar 21, 2020

patacongo left a comment

maht commented Mar 21, 2020

xiaoxiang781216 commented Mar 22, 2020

xiaoxiang781216 commented Mar 22, 2020

Support to run NuttX on ESP32 QEMU #437

Support to run NuttX on ESP32 QEMU #437

Conversation

maht commented Mar 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patacongo Mar 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igrr Mar 5, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

masayuki2009 commented Mar 6, 2020

xiaoxiang781216 commented Mar 6, 2020

maht commented Mar 6, 2020

maht commented Mar 6, 2020

patacongo commented Mar 16, 2020

maht commented Mar 17, 2020

maht commented Mar 21, 2020

patacongo commented Mar 21, 2020

patacongo commented Mar 21, 2020

patacongo left a comment

Choose a reason for hiding this comment

maht commented Mar 21, 2020

xiaoxiang781216 commented Mar 22, 2020

xiaoxiang781216 commented Mar 22, 2020

patacongo Mar 5, 2020 •

edited

igrr Mar 5, 2020 •

edited