[RFC] System devicetree demo for mps2 an521 board #52272
Conversation
There was a problem hiding this comment.
Looks good to me, just a minor documentation nit.
I'd like to see how we'll express things like MPU partitioning across mps2_an521 and mps2_an521_ns with this approach, where the secure partition always has priority. Alternatively, ownership of something like the I2C/SPI bus. This at least gives us a starting point for expressing those relationships, though.
cmake/modules/dts.cmake
Outdated
| # - To users as a final devicetree source (DTS) file which can | ||
| # be used for debugging |
There was a problem hiding this comment.
Worth mentioning the filename here (build/zephyr/zephyr.dts)? All the other bullet points do so.
| * project has defined for that board - a single image boot is | ||
| * assumed. Please see the memory layout in: | ||
| * | ||
| * https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/platform/ext/target/mps2/an521/partition/flash_layout.h |
There was a problem hiding this comment.
Even though the flash layout described in Zephyr's fork of TF-M is wrong (it's been fixed upstream, but won't appear in Zephyr until we update to TF-M 1.7.0, which should be soon) ... we should probably point to the version of the file we actually use, which is: https://github.com/zephyrproject-rtos/trusted-firmware-m/blob/master/platform/ext/target/arm/mps2/an521/partition/flash_layout.h
|
@nordicjm @microbuilder thanks for the reviews. I will update as requested. For situational awareness, there are a couple of other changes incoming that aren't reflected in the discussion here:
@galak doesn't like the set up with the extra .dts files for targeting a particular core, and asked me to look into adding support for the 'execution domains' feature of system devicetree instead, and having each board provide some default domains. If I find this to be workable, the impact to the PR will be:
I've been raising issues and sending various pull requests to the system devicetree specification (issues: devicetree-org/lopper#133, devicetree-org/lopper#130, devicetree-org/lopper#129, devicetree-org/lopper#128, PRs: devicetree-org/lopper#131, devicetree-org/lopper#126), with at least one more big one incoming. This will change the semantics of the 'simple-bus' handling (I got this wrong, and simple-bus nodes need to be explicitly mapped into each CPU cluster's address-map). It will also result in more complete sysdtlib support. I have at least one more large-ish system DT spec PR incoming this week to make the language in there a bit more formal, etc. I will update this PR after I'm confident we have the spec issues sorted out. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
d7b5941 to
362a807
Compare
There was a problem hiding this comment.
@tejlmand seems this is not compatible with sysbuild: cmake -GNinja -DBOARD=sysdt_mps2_an521 -DSYSDT_EXECUTION_DOMAIN=/domains/domain_cpu0 -DAPP_DIR=.. -Dhello_world_SYSDT_EXECUTION_DOMAIN=/domains/domain_cpu0 /tmp/aa/zephyr/share/sysbuild/ gives:
*********************************
* Running CMake for hello_world *
*********************************
Loading Zephyr default modules (Zephyr repository).
-- Application: /tmp/aa/zephyr/samples/hello_world
-- CMake version: 3.25.2
-- Found Python3: /usr/bin/python3.10 (found suitable exact version "3.10.9") found components: Interpreter
-- Cache files will be written to: /home/jamie/.cache/zephyr
-- Zephyr version: 3.3.0-rc1 (/tmp/aa/zephyr)
-- Found west (found suitable version "0.14.0", minimum required is "0.7.1")
-- Board: sysdt_mps2_an521
-- ZEPHYR_TOOLCHAIN_VARIANT not set, trying to locate Zephyr SDK
-- Found host-tools: zephyr 0.15.0 (/opt/zephyr-sdk-0.15.0)
-- Found toolchain: zephyr 0.15.0 (/opt/zephyr-sdk-0.15.0)
-- Found Dtc: /opt/zephyr-sdk-0.15.0/sysroots/x86_64-pokysdk-linux/usr/bin/dtc (found suitable version "1.6.0", minimum required is "1.4.6")
CMake Error at /tmp/aa/zephyr/cmake/modules/dts.cmake:232 (message):
Found system devicetree file
/tmp/aa/zephyr/boards/arm/sysdt_mps2_an521/sysdt_mps2_an521.sysdts, but
SYSDT_EXECUTION_DOMAIN was not defined. Set this variable to the path to
the execution domain you wish to use.
Call Stack (most recent call first):
/tmp/aa/zephyr/cmake/modules/zephyr_default.cmake:113 (include)
/tmp/aa/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:66 (include)
/tmp/aa/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:117 (include_boilerplate)
/home/jamie/sysbuild/zephyr/share/zephyr-package/cmake/zephyr_package_search.cmake:117 (include)
/home/jamie/sysbuild/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:152 (check_zephyr_package)
CMakeLists.txt:5 (find_package)
-- Configuring incomplete, errors occurred!
CMake Error at cmake/modules/sysbuild_extensions.cmake:255 (message):
CMake configure failed for Zephyr project: hello_world
Location: /tmp/aa/zephyr/samples/hello_world
Call Stack (most recent call first):
CMakeLists.txt:59 (ExternalZephyrProject_Add)
cmake/modules/dts.cmake
Outdated
| # | ||
| # Run gen_defines.py to create a header file, zephyr.dts, and edt.pickle. | ||
| # | ||
| # TODO: how can we support board revisions in system devicetree? |
There was a problem hiding this comment.
If the revision is still provided with the board name, can it not check if <board>@<revision>.overlay exists and if so, include that with the main dts file?
There was a problem hiding this comment.
So far we are trying to be careful to use different extensions for system DT and standard DT files:
.sysdtsvs.dts.sysdtsivs.dtsi
So I guess this would be .sysoverlay?
| if(EXISTS ${DTS_SOURCE}) | ||
| # We found a devicetree. Check for a board revision overlay. | ||
| if(BOARD_REVISION AND EXISTS ${BOARD_DIR}/${BOARD}_${BOARD_REVISION_STRING}.overlay) | ||
| list(APPEND DTS_SOURCE ${BOARD_DIR}/${BOARD}_${BOARD_REVISION_STRING}.overlay) |
There was a problem hiding this comment.
(From above) similar to this, boards will either be v2 systemdt or v1 non-systemdt so should be able to have v2 specific handling
| 'compatible properties; unexpected address map ' | ||
| 'behavior may result') | ||
|
|
||
| def arm_v8m_address_map(): |
There was a problem hiding this comment.
Do we envisage similar requirements for other architectures that would need special handling here? I don't know if arc/mips/ppc/risc/xtensa have or would implement things similar to armv8 security. And also, how would this then support being used/extended out of tree?
There was a problem hiding this comment.
Do we envisage similar requirements for other architectures that would need special handling here?
I don't know. Anecdotally, this is the first time that the system DT maintainers ran into this; they had no idea this was a possiblility, and they're running on multiple architectures.
I think the answer is probably "yes", but I also think we should proceed incrementally and gather more data with case-by-case code like this until a pattern emerges. It's not like new architectures are appearing every day.
And also, how would this then support being used/extended out of tree?
Right now, the system devicetree specification itself is where the meaning of the execution level flags is defined, and the spec authors chose to mandate that the spec itself should be updated to add new architecture-specific definitions for this field. (I do plan on upstreaming this Arm v8-M convention into the spec as a replacement for a couple of previous, failed attempts to handle this case that we reverted in the spec; see devicetree-org/lopper@0da7cbd .)
TL;DR "you won't extend this out of tree for now". Let's gather data from publicly documented architectures that want to support this, then when the pattern emerges, we can extend the specification as needed.
So for now, this is a "heavyweight" operation
362a807 to
6790077
Compare
|
Last force push is just a rebase |
6790077 to
c40ee68
Compare
e2186ed to
dcdc2c6
Compare
dcdc2c6 to
49c09ac
Compare
These are now merged, so I've rebased their contents out of this PR to make it smaller. |
The name of each commented section should match the name in the "table of contents", for consistency and so people can jump from contents to implementations more easily with their editors' search functions. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Due to Hyrum's Law, there are users out in the wild depending on this directory existing to place their own generated files, so it'd break things unnecessarily to not do this if we don't have a DTS, as explained in a source code comment. However, this doesn't really have anything to do with DTS processing, so split it into its own module to keep things clean. This also paves the way for inserting another module in between the generated_file_directories and dts modules that itself depends on these directories existing. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Create a separate module that sets up all our devicetree handling, by setting up common variables that would apply to any and all DT processing. This is then included in the regular dts module that we include in zephyr_default.cmake. The separation of this code from dts.cmake is a useful cleanup, and is also groundwork for enabling system devicetree in Zephyr, which will need the same definitions included into its scope. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
When the build system was being split up into modules under cmake/modules, most of the resulting cmake modules had their inputs and outputs documented in top-of-file comments. The dts module is an exception, which makes it harder to use since its contracts aren't defined. Fix this by adding a contract. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This procedure will be useful elsewhere, so prep for not repeating ourselves. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Tighten up the interface boundaries by adding an extension function that separates *how* we preprocess the DTS from *what* DTS files we are preprocessing. This involves a functional change to the pre_dt module. This is useful cleanup but is also groundwork for relying on this helper function when adding system devicetree support. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
If DTS_SOURCE is already set, then it might not be a BOARD.dts, even though that is true basically always today. Make the output generation a bit more generic so that we can handle a pre-set DTS_SOURCE better, even though this is currently an edge case. (It won't be an edge case anymore when we have system devicetree support.) Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The change to edtlib.Node.unit_addr does break backwards compatibility, but there are no affected in-tree users Move these into dtlib from edtlib. Preserve edtlib-side APIs for this. Just like there are corresponding dtlib.Node and edtlib.Node classes, add new dtlib.Register and dtlib.Range classes to correspond to existing edtlib.Register and edtlib.Range classes, and move the parsing of these properties into dtlib. Nothing significant in the semantics for reg or ranges depends on devicetree bindings. These basic properties are defined in the devicetree specification and are central to the way addressing works. Their initialization therefore makes more sense in dtlib than edtlib. Putting it there will make it possible to use this data in dtlib clients as well as edtlib clients. There is a longstanding TODO comment in here (which dates back to the original introduction of the library) questioning whether we really ought to have edtlib.Node.unit_addr()'s value be an int, instead of a string. There does indeed seem little point to translating the unit address of a node recursively through the address maps of its parents. Make the return types match to keep things simple and expose the actual unit address, rather than this strange value. This keeps this conversion simpler by not duplicating address translation logic in two places, at the cost of breaking backwards compatibility. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
TODO: Various limitations and edge cases
see NotImplementedError() locations for details.
For more information about system devicetree, please see:
https://github.com/devicetree-org/lopper/
This module implements basic support for some system devicetree
concepts documented there. Specifically, add basic support for:
- cpu clusters
- indirect buses
- execution domains
This is done through a method that takes some target information gives
you back a "regular" DT object for that target. You can then use the
regular DT however you want in the zephyr build system.
As an extension to the system DT specification, sysdtlib supports
an 'address-map-secure' property that can be selected for Arm v8-M
CPUs with TrustZone support. This is a separate address map that can
be used for the different peripheral addressing that is typically
found on such CPUs when the CPU is in the secure state.
This extension is backwards compatible with the existing system DT
spec in the sense that existing system devicetrees with a single
address map property will not need to change to adopt this extension.
We'll work on upstreaming this into the system DT spec in parallel.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is a tiny glue script between the devicetree.sysdtlib Python module and the build system's dts.cmake CMake module. It can be used to convert a system devicetree (.sysdts) to a standard devicetree (.dts) file for use by the pre-existing "standard" devicetree tooling. The generated .dts file can be used as "normal". Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This contains system devicetree definitions for Arm v8-M CPUs. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
TODO: add interrupts-extended to map nodes to NVICs This extracts all of the common information in boards/arm/mps2_an521 into a new separate system devicetree include file (.sysdtsi) for the SoC. The intention here is that any "board" using this SoC should include this .sysdtsi file in that board's system devicetree file, i.e. <BOARD>.sysdts should look like this: #include <arm/mps2_an521.sysdtsi> /* board-specific system DTS contents go here ... */ A board definition using this system devicetree is in a separate commit. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
TODO: - add TF-M, mcuboot domains - boot on cpu0 ns once TF-M domain is available - boot on cpu1 This is a system devicetree-based board for the Arm MPS2 AN521, which should be equivalent to the various files in boards/arm/mps2_an521. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
These aren't allowed vendor prefixes, but system DT is using them as such. Work around it for now. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The intent of this commit is to make any directory with
${BOARD}.sysdts suitable as the value of the BOARD_DIR
variable.
I'm unsure about the consequences of doing it this way for hardware
model v1, and it's unclear to me that we even want to support system
devicetree in hardware model v1, which is why I'm calling it a hack.
It works for prototyping system DT support in HWMv1, though.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This module adds build-system support for boards with system devicetrees. This module runs before the dts module, and is responsible for converting a system devicetree to a regular devicetree suitable for consumption by the dts module if that is necessary. This prepares the dts module for its work. The DTS module in turn shouldn't have to do anything differently than normal, validating that system DT is possible to fit into an existing build system in an unobtrusive way. HACKS: It also is currently setting up the board defconfig based on a way to put domain-specific defconfigs inside the board directory, and doing similarly for a domain-specific overlay. This clearly won't scale to other situations, but it's enough for prototyping. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
49c09ac to
ef2c39b
Compare
|
This now separates the system DT portions into their own CMake module. The reviewable refactoring that enabled this is in a separate PR: #55293. This RFC is rebased on top of that PR. |
|
This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time. |
As discussed in architecture working group meetings related to #51830, here is a proof of concept for running a board definition using system devicetree in "hardware model v1" (for information on "v2" and why it will be needed, see #51831).
To test hello world booting on CPU0:
We don't have a TF-M target yet to build non-secure firmware, however,
and I haven't tested CPU1 yet. Those are still TODO.
Changelog: