Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PX4 protected build pub #16931

Merged
merged 12 commits into from Mar 17, 2022
Merged

PX4 protected build pub #16931

merged 12 commits into from Mar 17, 2022

Conversation

jlaitine
Copy link
Contributor

@jlaitine jlaitine commented Feb 23, 2021

This PR implements PX4 running on NuttX protected build. The split between kernel and user space libraries is in top level:

kernel:

  • All drivers
  • Full uORB
  • Parameters (but only loading at boot time)
  • high resolution timer
  • Anything that links directly to OS drivers or accesses hardware

user space:

  • Another uORB, mostly only uORBManager communicating with kernel side over ioctl
  • parameters
  • hrt interafce, using an own highest priority task for callbacks

build scripts:

  • Support for building nuttx kernel and user -side libraries, linking for this
  • Support for generating "builtins" for kernel side modules
  • Support for putting any module to eiither kernel or user side (adding "KERNEL" flag to CMakeLists.txt for the module). Restrictions apply; a module linking to nuttx kernel or accessing HW cannot be linked into user side..

@jlaitine jlaitine marked this pull request as draft February 23, 2021 07:26
@LorenzMeier
Copy link
Member

Thanks! I will set some quality time apart to go through it. This capability is one of the key reasons why we chose NuttX 10 years ago and why some of the PX4 fundamentals have been architected around file descriptors (to be able to go through kernel space cleanly). We have since optimized on various fronts, so I'm sure it needs some refresh and polishing.

@dagar
Copy link
Member

dagar commented Feb 23, 2021

I'm very interested to dig into this and see exactly where (and how) you've split things between kernel and user space. I actually gave this a try with NuttX a few years ago, but quickly came to the conclusion the value was minimal if nearly all of PX4 was simply in user space (able to clobber itself).

@dagar dagar requested review from dagar and davids5 February 26, 2021 17:44
@mrpollo mrpollo changed the title Px4 protected build pub PX4 protected build pub Mar 10, 2021
@jlaitine
Copy link
Contributor Author

If someone has had time to look into this bunch of stuff; I'd appreciate comments on it; basically which parts are implemented in a proper manner, and which could be done better or doesn't make sense at all. I could start chopping it in pieces and make individual smaller PR:s, for example

@dagar
Copy link
Member

dagar commented Apr 23, 2021

If someone has had time to look into this bunch of stuff; I'd appreciate comments on it; basically which parts are implemented in a proper manner, and which could be done better or doesn't make sense at all. I could start chopping it in pieces and make individual smaller PR:s, for example

I'm still very interested in this and would like to start getting pieces in incrementally as soon as we get v1.12 out.

@jlaitine
Copy link
Contributor Author

Rebased against current master, + added the following:
on px4_fmuv5_protected target:

  • removed the micrortps agent from the build, it is not relevant for this work
  • changed flash and sram allocation between kernel and user space, doubled the kernel memories so that it is easier to drop more stuff to kernel
  • moved "sensors" to kernel side, it saves a lot of CPU when all the sensor data doesn't need to go many times between kenel-userspace barrier
  • minor cleanups

Didn't have time to clean up all the things I had in mind yet though

@jlaitine jlaitine force-pushed the px4_protected_build_pub branch 16 times, most recently from 1a81cae to 4f64a3a Compare May 27, 2021 20:10
@jlaitine
Copy link
Contributor Author

jlaitine commented Sep 6, 2021

Just a re-base to keep this alive. The following items should be resolved (at least):

  • couple of "command not found" nsh errors on other targets than "protected-build"; "userspace_init" systemcmd called from rcS, which only exists for protected, and "kparam" for loading kernel side parameters at boot
  • all uORBs are allocated from user-side heap for protected-build, because of uORB performance optimization by inlining functions; these should be re-designed, especially the "advertised()" call. But I didn't want to change the functionality of uORB for now, it is complicated enough
  • one nuttx backport needed (protected build has broken in nuttx meanwhile)
  • need a nicer way to select what goes to kernel side and what to user side. Now the selection is done by adding keyword "KERNEL" into component's CMakeLists.txt. But it would be better to have a separate kernel side component list with the "board" configuration
  • fix the linking for other targets (again..)

@jlaitine jlaitine force-pushed the px4_protected_build_pub branch 3 times, most recently from e5f7438 to eec3194 Compare March 1, 2022 20:52
@jlaitine jlaitine marked this pull request as ready for review March 8, 2022 21:22
@jlaitine
Copy link
Contributor Author

jlaitine commented Mar 8, 2022

Dropped everything that is not absolutely needed to compile and run this; also tested flying it.

@jlaitine
Copy link
Contributor Author

jlaitine commented Mar 9, 2022

Just re-based to see if the weird CI fail goes away

@jlaitine jlaitine force-pushed the px4_protected_build_pub branch 2 times, most recently from 48d397b to 5fefddc Compare March 15, 2022 13:59
@jlaitine
Copy link
Contributor Author

@dagar , @bkueng , this PR now has left the following

  • initializing the userspace in boot
  • some random boardctl things like reading vbus status into userspace
  • boardctl call to launch modules in kernel side
  • px4_fmu-v5_protected target, which is tested to build, flash and fly

Would you like me to further split this into smaller pieces, or is this reviewable as is? This brings in a working target.

@@ -0,0 +1,20 @@
#include <drivers/drv_hrt.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing copyright header

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, added

The px4_userspace_init function is called from userspace
entrypoint before starting NSH

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
For protected/kernel builds the cxx static initializations
needs to be done also in kernel side, since px4 creates
c++ objects in kernel

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…utdown lockout

Add kernel side ioctls and handlers for vbus state and managing shutdown lockout

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Add launch_kmod command to start/execute px4 modules in kenel space
in NuttX protected build

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
This is needed for DMA capable memory for fat also in the user side;
the file system doesn't seem to work reliably without.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Add a target for nuttx protected build development

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
In memory protected builds these perform nuttx boardctl ioctl calls to kerenel

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Direct gpio read is not possible from user side applications, so use boardctl
interface instead.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
It accesses kernel internal structures directly; this needs to be
worked out with some proper userspace-kernel interface

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
…build

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
… protected build target

This info is needed on both kernel and user sides, and is just data.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
It is a performance issue to run it on user side

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've made it through... with a lot of patience :D
Nice work.

@bkueng bkueng merged commit 3d35929 into PX4:master Mar 17, 2022
@jlaitine
Copy link
Contributor Author

Thank you for all the reviews and help with this! This is now a good base for gradual improvements in small steps, for which we already have some ideas.

@jlaitine jlaitine deleted the px4_protected_build_pub branch April 22, 2022 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants