Move to and extend `zephyr,memory-attr` #60049

carlocaione · 2023-07-05T14:15:27Z

The zephyr,memory-region-mpu property was added in #43119 as a convenient way to create and configure MPU regions using information coming from DT. It has been used a lot since it was introduced so I guess we can consider it a Zephyr success story ™ .

Unfortunately it has been proved to be a bit limited and with some important limitations:

It was introduced as a property of the compatible zephyr,memory-region that is used to create linker regions and sections from DT data. This means that we can actually create MPU regions only for DT-defined regions and sections.
The naming is unfortunate because it is implying that it is used only for MPU.
It is misplaced being in include/zephyr/linker/devicetree_regions.h and still it has nothing to do with the linker at all.
It is exporting a function called LINKER_DT_REGION_MPU that again has nothing to do with the linker.

Point (1) is also particularly limiting because it is preventing us to characterize memory regions that are not generated using the zephyr,memory-region compatible, like generic mmio-sram regions.

While we fix all the issues, we also want to extend a bit the range of usefulness of this property. We are renaming it zephyr,memory-attr and it is now carrying information about the type of memory the property is attached to (cacheable, non-cacheable, IO, eXecutable, etc...). The user can use this property and the DT API coming with it to act on the memory node it is accompanied by.

We are still providing the DT_MEMORY_ATTR_APPLY() macro that can be used to create the MPU regions as before, but we are adding also a DT_MEMORY_ATTR_FOREACH_NODE() macro that can be used to cycle through the memory nodes and act on those.

Some use-cases:

We want to be able to cycle and identify memory regions with some attributes.
We want to be able to check whether a buffer is inside a memory region with some attribute (see for example RFC: drivers: stm32: SPI: Check that SPI buffers are in a nocache region #57786).
We want to be able to generate MMU regions from DT.

In general the idea is being able to detect and work with memory regions and their attributes in an easy way instead of resorting to custom code.

microbuilder · 2023-07-07T09:59:39Z

@carlocaione Maybe we should add this change to the release notes now to make sure the change doesn't get lost?

manuargue · 2023-07-14T13:01:37Z

Hi @carlocaione , changes for NXP S32 affected platforms LGTM. Approved that part.

Btw, I'm working on a pr to extend this feature to Cortex-A/R Aarch32, but I haven't had time to file a PR yet. And I also saw there's some big refactoring going on for those arch in another pr, so I think better I'll wait for that to get merged. My intention is to "normalize" the memory attribute names across all the Arm Aarch32 archs and to document (similarly to #60049 (review)) in a table what are the memory attributes assigned. Are you already working in something like this or would be fine to continue in this direction and raise a pr?

carlocaione · 2023-07-14T13:04:37Z

Btw, I'm working on a pr to extend this feature to Cortex-A/R Aarch32, but I haven't had time to file a PR yet. And I also saw there's some big refactoring going on for those arch in another pr, so I think better I'll wait for that to get merged. My intention is to "normalize" the memory attribute names across all the Arm Aarch32 archs and to document (similarly to #60049 (review)) in a table what are the memory attributes assigned. Are you already working in something like this or would be fine to continue in this direction and raise a pr?

I'm not working on it but it sounds like a very welcome improvement and I'm not currently working on it. So feel free to keep working on it if you are willing to, that definitely would help a lot.

doc/build/dts/api/api.rst

dts/bindings/base/zephyr,memory-attr.yaml

include/zephyr/devicetree/memory-attr.h

tests/lib/devicetree/api/src/main.c

doc/releases/release-notes-3.5.rst

dts/bindings/base/zephyr,memory-region.yaml

dts/bindings/test/vnd,memory-attr.yaml

include/zephyr/devicetree/memory-attr.h

The 'zephyr,memory-region-mpu' property was addede gqas a convenient way to create and configure MPU regions using information coming from DT. It has been used a lot since it was introduced so I guess we can consider it a Zephyr success story ™ . Unfortunately it has been proved to be a bit limited and with some important limitations: 1. It was introduced as a property of the compatible zephyr,memory-region that is used to create linker regions and sections from DT data. This means that we can actually create MPU regions only for DT-defined regions and sections. 2. The naming is unfortunate because it is implying that it is used only for MPU. 3. It is misplaced being in include/zephyr/linker/devicetree_regions.h and still it has nothing to do with the linker at all. 4. It is exporting a function called LINKER_DT_REGION_MPU that again has nothing to do with the linker. Point (1) is also particularly limiting because it is preventing us to characterize memory regions that are not generated using the 'zephyr,memory-region' compatible, like generic mmio-sram regions. While we fix all the issues, we also want to extend a bit the range of usefulness of this property. We are renaming it 'zephyr,memory-attr' and it is now carrying information about the type of memory the property is attached to (cacheable, non-cacheable, IO, eXecutable, etc...). The user can use this property and the DT API coming with it to act on the memory node it is accompanied by. We are still providing the DT_MEMORY_ATTR_APPLY() macro that can be used to create the MPU regions as before, but we are adding also a DT_MEMORY_ATTR_FOREACH_NODE() macro that can be used to cycle through the memory nodes and act on those. Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Move to 'zephyr,memory-attr' and use the newly introduced helpers. Signed-off-by: Carlo Caione <ccaione@baylibre.com>

Add DT changes to the 3.5 release notes. Signed-off-by: Carlo Caione <ccaione@baylibre.com>

carlescufi

This looks very reasonable to me

microbuilder

Thanks for the release notes.

dts/bindings/test/vnd,memory-attr.yaml

dgastonochoa · 2023-07-25T09:17:21Z

tests/lib/devicetree/api/src/main.c

+	zassert_true(!strcmp(val_apply[1].name, "memory@44332211"), "");
+	zassert_equal(val_apply[1].base, 0x44332211, "");
+	zassert_equal(val_apply[1].size, 0x2000, "");
+	zassert_equal(val_apply[1].attr, 0xCACA, "");


Shouldn't this 0xCACA be REGION_RAM_NOCACHE_ATTR?

yeah, maybe for readability but after all this is a test, so it doesn't really matter?

carlocaione force-pushed the mem-attr branch from 56f317b to 83a1626 Compare July 5, 2023 14:29

microbuilder self-requested a review July 6, 2023 08:58

microbuilder mentioned this pull request Jul 6, 2023

arch: arm: aarch32: cortex_m: fault: ARMv6-M illegal instruction fetch triggers lockup before any fault debug output #57412

Closed

carlocaione changed the title ~~Mem attr~~ Move to and extend zephyr,memory-attr Jul 6, 2023

carlocaione force-pushed the mem-attr branch 3 times, most recently from 7ffb953 to c8ccfd0 Compare July 7, 2023 09:43

carlocaione marked this pull request as ready for review July 7, 2023 09:44

carlocaione requested review from nashif, dcpleung, andyross, ioannisg, stephanosio, galak, carlescufi, MaureenHelm, mmahadevan108, dleach02, erwango, povergoing, ABOSTM and FRASTM as code owners July 7, 2023 09:44

carlocaione requested review from gmarull and removed request for FRASTM July 7, 2023 09:44

zephyrbot assigned erwango Jul 7, 2023

carlocaione requested review from mbolivar-nordic and removed request for ioannisg and MaureenHelm July 7, 2023 09:45

povergoing previously approved these changes Jul 7, 2023

View reviewed changes

zephyrbot requested a review from gautierg-st July 14, 2023 10:24

hubertmis previously approved these changes Jul 14, 2023

View reviewed changes

carlocaione dismissed hubertmis’s stale review via 07be64e July 14, 2023 11:24

carlocaione force-pushed the mem-attr branch from 1d99063 to 07be64e Compare July 14, 2023 11:24

carlocaione requested a review from manuargue as a code owner July 14, 2023 11:24

gmarull reviewed Jul 18, 2023

View reviewed changes

carlocaione force-pushed the mem-attr branch from 07be64e to 714e7bb Compare July 18, 2023 09:11

gmarull reviewed Jul 18, 2023

View reviewed changes

dts/bindings/base/zephyr,memory-region.yaml Show resolved Hide resolved

gmarull reviewed Jul 18, 2023

View reviewed changes

dts/bindings/test/vnd,memory-attr.yaml Show resolved Hide resolved

gmarull reviewed Jul 18, 2023

View reviewed changes

include/zephyr/devicetree/memory-attr.h Outdated Show resolved Hide resolved

carlocaione added 3 commits July 18, 2023 12:01

dts: Move to 'zephyr,memory-attr'

6ab7a17

Move to 'zephyr,memory-attr' and use the newly introduced helpers. Signed-off-by: Carlo Caione <ccaione@baylibre.com>

doc: release: Add DT changes

cd4542d

Add DT changes to the 3.5 release notes. Signed-off-by: Carlo Caione <ccaione@baylibre.com>

carlocaione force-pushed the mem-attr branch from 714e7bb to cd4542d Compare July 18, 2023 10:02

gmarull approved these changes Jul 18, 2023

View reviewed changes

carlocaione requested review from povergoing and hubertmis July 25, 2023 07:21

carlescufi approved these changes Jul 25, 2023

View reviewed changes

hubertmis approved these changes Jul 25, 2023

View reviewed changes

microbuilder approved these changes Jul 25, 2023

View reviewed changes

erwango approved these changes Jul 25, 2023

View reviewed changes

dts/bindings/test/vnd,memory-attr.yaml Show resolved Hide resolved

dgastonochoa reviewed Jul 25, 2023

View reviewed changes

carlescufi merged commit aab0735 into zephyrproject-rtos:main Jul 25, 2023

carlocaione deleted the mem-attr branch July 25, 2023 09:22

This was referenced Jul 28, 2023

mem_attr: Introduce library to query memory attributes #60916

Closed

Add the DT_MEMORY_ATTR_ defines #60874

Closed

carlocaione added the treewide 🧹 label Aug 1, 2023

carlocaione mentioned this pull request Aug 4, 2023

[RFC] Make zephyr,memory-attr a capabilities bitmask #61009

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move to and extend `zephyr,memory-attr` #60049

Move to and extend `zephyr,memory-attr` #60049

carlocaione commented Jul 5, 2023 •

edited

Loading

microbuilder commented Jul 7, 2023

manuargue commented Jul 14, 2023

carlocaione commented Jul 14, 2023

carlescufi left a comment

microbuilder left a comment

dgastonochoa Jul 25, 2023

carlocaione Jul 25, 2023

Move to and extend zephyr,memory-attr #60049

Move to and extend zephyr,memory-attr #60049

Conversation

carlocaione commented Jul 5, 2023 • edited Loading

microbuilder commented Jul 7, 2023

manuargue commented Jul 14, 2023

carlocaione commented Jul 14, 2023

carlescufi left a comment

Choose a reason for hiding this comment

microbuilder left a comment

Choose a reason for hiding this comment

dgastonochoa Jul 25, 2023

Choose a reason for hiding this comment

carlocaione Jul 25, 2023

Choose a reason for hiding this comment

Move to and extend `zephyr,memory-attr` #60049

Move to and extend `zephyr,memory-attr` #60049

carlocaione commented Jul 5, 2023 •

edited

Loading