[POC | RFC | DNM] Introducing framework to configure MPU regions from DT-defined SRAM regions #40422

carlocaione · 2021-11-17T12:54:44Z

This is a first attempt to do what was suggested in #30403 (comment) and addressing the low hanging fruits from #40146.

That is: how to define SRAM regions in the DT with MPU attributes that can be automatically configured by the MPU driver and using this mechanism to define a non-cacheable region to be used for (among other things) DMA.

Automatic SRAM regions generation and MPU registering

In the DT we must define the SRAM regions with a new mpu-attr property, for example (refer to the sample)

       sram_attr_0: memory@20200000 {
               compatible = "mmio-sram";
               reg = <0x20200000 0x100000>;
               mpu-attr = <DT_SRAM_ATTR(DT_1MB)>;
               label = "region0";
       };

       sram_no_cache: memory@20300000 {
               compatible = "mmio-sram";
               reg = <0x20300000 0x100000>;
               mpu-attr = <DT_SRAM_NOCACHE_ATTR(DT_1MB)>;
               label = "region1";

We want these regions to be automatically mapped by the MPU driver with the provided attributes. To be able to do this we have to use the provided helpers.

In particular we have to tweak the linker script adding the new helpers for the creation of sections and regions:

MEMORY {
       ...
       LINKER_DT_SRAM_REGIONS()
}

SECTIONS {
       ...
       LINKER_DT_SRAM_SECTIONS()
}

To be able to configure the regions using the MPU, we leverage the mpu_config struct (when available) and use another new helper (DT_SRAM_INST_FOREACH) to cycle through all the SRAM regions with a provided MPU attribute. For example:

#define BUILD_MPU_REGION(p_name, p_base, p_size, p_attr)       \
       { .name = p_name,                                       \
         .base = p_base,                                       \
         .attr.rasr = p_attr,                                  \
       },

static const struct arm_mpu_region mpu_regions[] = {
       ...

       /* Generated SRAM regions */
       DT_SRAM_INST_FOREACH(BUILD_MPU_REGION)
};

const struct arm_mpu_config mpu_config = {
       .num_regions = ARRAY_SIZE(mpu_regions),
       .mpu_regions = mpu_regions,
};

The user can then allocated variables and buffers in the correct region using the usual GCC attributes and the label name for the region, for example:

uint32_t var_in_region0 __attribute((__section__("region0"))) = 0xaabbccdd;
uint32_t var_in_region1 __attribute((__section__("region1"))) = 0xddccbbaa;

non-cacheable memory

In this PR we also add a new chosen phandle that is used to reference the non-cacheable SRAM region. This region is then used when non-cacheable buffers must be allocated for (i.e.) DMA using a newly introduced attribute.

For example in the DT we can have something like this:

       chosen {
               zephyr,sram-no-cache = &sram_no_cache;
       };

       sram_no_cache: memory@20300000 {
               compatible = "mmio-sram";
               reg = <0x20300000 0x100000>;
               mpu-attr = <DT_SRAM_NOCACHE_ATTR(DT_1MB)>;
               label = "region1";
       };

When this phandle does exist, a new attribute __nocache_sram is automatically introduced, so that the user can easily place non-cacheable buffers in the non-cacheable regions doing (for example):

uint32_t buf_in_nocache[0x100] __nocache_sram;

Given a set of SRAM entries in the DT defined in the form (for example): sram0_with_attr: memory@0x20000000 { compatible = "mmio-sram"; reg = <0x20000000 0x1000>; mpu-atttr = <0xsomeattr>; label = "sram0"; }; This patch is introducing three helper macros: - LINKER_DT_SRAM_SECTIONS() to automatically generate linker sections from the SRAM nodes - LINKER_DT_SRAM_REGIONS() to automatically generate linker regions from the SRAM nodes - DT_SRAM_INST_FOREACH() to be used to cycle through all the SRAM nodes to statically generate the MPU regions to be passed to the MPU driver Also given a chosen phandle in the form: zephyr,sram-no-cache = &sram_no_cache a new __nocache_sram attribute is introduced to place buffers and variables in a non-cacheable SRAM region pointed by the phandle. Signed-off-by: Carlo Caione <[email protected]>

Introduce a simple sample to show how to use the newly introduced helpers (LINKER_DT_SRAM_* and DT_SRAM_INST_FOREACH) to automatically generate MPU regions from DT-defined SRAM regions. It also shows how to use the zephyr,sram-no-cache phandle to put buffers in non-cacheable memory. Signed-off-by: Carlo Caione <[email protected]>

nashif · 2021-11-17T13:38:59Z

[POC | RFC | DNM]

:)

mbolivar-nordic · 2021-11-19T01:12:16Z

No objections in general, sounds like a nice bit of infrastructure.

In this PR we also add a new chosen phandle that is used to reference the non-cacheable SRAM region

Are you sure about this one? You will only need one, not multiple, such regions?

carlocaione · 2021-11-19T08:13:22Z

Are you sure about this one? You will only need one, not multiple, such regions?

In theory yes, but with the chosen method only one region can be referenced (if we want to keep things simple).

For more complex cases where you have multiple non-cacheable regions my idea was to revisit a bit the shared_multi_heap allocator to support generic SRAM regions instead of relying only on reserved-memory (or maybe I can extend this PR to support reserved-memory regions).

KozhinovAlexander

Great improvements!

KozhinovAlexander · 2021-11-22T21:22:07Z

samples/memory/sram_regions/CMakeLists.txt

@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: Apache-2.0
+
+cmake_minimum_required(VERSION 3.20.0)


3.20 should be enough here. I think.

KozhinovAlexander · 2021-11-22T21:25:35Z

samples/memory/sram_regions/boards/dt_mpu_attr.h

+ * NORMAL_OUTER_INNER_WRITE_BACK_WRITE_READ_ALLOCATE_NON_SHAREABLE |
+ * MPU_RASR_XN_Msk | P_RW_U_NA_Msk
+ */
+#define DT_SRAM_ATTR(size) (0x110b0000 | size)


#define DT_SRAM_ATTR(size) ((MPU_RASR_XN_Msk | P_RW_U_NA_Msk) | size)

Wouldn't be it better to use mask defines with corresponding values?

yes but it's not easy. A lot of things in the .h files where the masks are defined doesn't get along well with DT, so you cannot just include the .h files into the DTS file. I tried to fix the header files where the masks are defined to be more compatible with DT but I gave up after a while and I just decided to go this way.

KozhinovAlexander · 2021-11-22T21:28:42Z

samples/memory/sram_regions/boards/mps2_an385.overlay

+	sram_attr_0: memory@20200000 {
+		compatible = "mmio-sram";
+		reg = <0x20200000 0x100000>;
+		mpu-attr = <DT_SRAM_ATTR(DT_1MB)>;


This looks for me that dt_mpu_attr.h header belongs somewhere else but not into samples directory or am I missing something?

Correct. This is a POC so I wanted to keep everything in one place.

carlocaione · 2021-11-23T08:38:49Z

Great improvements!

Thanks. Problem is that there are currently two people and two PRs trying to do the same thing. @JordanYates is working on #37279.

JordanYates

So my concern vs the approach in #37279 is that this only applies to mmio_sram.
My goal is a LINKER_DT_REGIONS macro which instantiates all the memory regions from devicetree, not just SRAM nodes. My original use case was for generating memory regions directly from flash partitions, as I want to place variables at specific addresses at compile time.

JordanYates · 2021-11-23T09:56:20Z

include/linker/devicetree_sram.h

+
+#define DT_DRV_COMPAT mmio_sram
+
+#define _DT_SRAM_INST_NAME(inst)	DT_STRING_TOKEN(DT_DRV_INST(inst), label)


The use of the label property for region naming was already discussed and rejected in #36365

KozhinovAlexander · 2021-11-23T19:11:09Z

@carlocaione @JordanYates can you somehow put your effort together or at least define who's changes should be merged first and who's afterwards if possible? I think even with two different approach it is possible to implement both. @nashif could you please coordinate proposed improvements or point on somebody responsible for it?

JordanYates · 2021-11-25T00:40:16Z

@carlocaione @JordanYates can you somehow put your effort together or at least define who's changes should be merged first and who's afterwards if possible? I think even with two different approach it is possible to implement both. @nashif could you please coordinate proposed improvements or point on somebody responsible for it?

At this point my effort is purely around generating region names from devicetree. My approach is the result of discussion in "dev-review" meetings, and unless something has changed, that is how names should be generated from devicetree.

This PR is about much more than just the region names, but that functionality has no conflict with my PR. It seems sensible to me to merge #37279 first.

carlocaione · 2021-11-25T08:13:14Z

At this point my effort is purely around generating region names from devicetree. My approach is the result of discussion in "dev-review" meetings, and unless something has changed, that is how names should be generated from devicetree.

This PR is about much more than just the region names, but that functionality has no conflict with my PR. It seems sensible to me to merge #37279 first.

Yes, agree on this point. #37279 is the first thing to merge. After that is merged we can start discussing about adapting this PR or opening an entirely new PR because AFAIU the use cases for @JordanYates are a bit more wide than what this PR is trying to address.

carlocaione · 2021-12-13T10:47:34Z

@JordanYates now that #37279 is merged can we discuss how to move in the direction of this PR?

github-actions · 2022-02-12T00:29:17Z

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.

carlocaione requested review from KozhinovAlexander, Navin-Sankar, aurel32, carlescufi, danieldegrasse, dleach02, erwango, galak, gmarull, hubertmis, mbolivar-nordic and ulfalizer November 17, 2021 12:54

carlocaione requested review from andyross, dcpleung and nashif as code owners November 17, 2021 12:54

carlocaione added DNM This PR should not be merged (Do Not Merge) RFC Request For Comments: want input from the community labels Nov 17, 2021

github-actions bot added area: API Changes to public APIs area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: Documentation area: Samples Samples labels Nov 17, 2021

carlocaione requested review from JordanYates, bbolen and nashif and removed request for JordanYates, Navin-Sankar, dcpleung and nashif November 17, 2021 13:19

carlocaione requested review from JordanYates and dcpleung and removed request for andyross November 17, 2021 13:20

carlescufi requested review from tejlmand and removed request for dleach02 and ulfalizer November 18, 2021 12:42

erwango mentioned this pull request Nov 22, 2021

linker: aarch32: automatic derivation of region names from devicetree nodes #37279

Merged

KozhinovAlexander requested changes Nov 22, 2021

View reviewed changes

JordanYates reviewed Nov 23, 2021

View reviewed changes

github-actions bot added the Stale label Feb 12, 2022

pdgendt mentioned this pull request Feb 18, 2022

Atmel SAM: Introduce SMC driver #42739

Merged

carlocaione closed this Feb 23, 2022

pdgendt mentioned this pull request Feb 23, 2022

Setup MPU for memory regions defined into the DT #43119

Merged

		@@ -0,0 +1,8 @@
		# SPDX-License-Identifier: Apache-2.0

		cmake_minimum_required(VERSION 3.20.0)


		#define DT_DRV_COMPAT mmio_sram

		#define _DT_SRAM_INST_NAME(inst) DT_STRING_TOKEN(DT_DRV_INST(inst), label)

[POC | RFC | DNM] Introducing framework to configure MPU regions from DT-defined SRAM regions #40422

[POC | RFC | DNM] Introducing framework to configure MPU regions from DT-defined SRAM regions #40422

Uh oh!

Conversation

carlocaione commented Nov 17, 2021

Automatic SRAM regions generation and MPU registering

non-cacheable memory

Uh oh!

nashif commented Nov 17, 2021

Uh oh!

mbolivar-nordic commented Nov 19, 2021

Uh oh!

carlocaione commented Nov 19, 2021

Uh oh!

KozhinovAlexander left a comment

Choose a reason for hiding this comment

Uh oh!

KozhinovAlexander Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

KozhinovAlexander Nov 22, 2021

Choose a reason for hiding this comment

Uh oh!

carlocaione Nov 23, 2021

Choose a reason for hiding this comment

Uh oh!

KozhinovAlexander Nov 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carlocaione Nov 23, 2021

Choose a reason for hiding this comment

Uh oh!

carlocaione commented Nov 23, 2021

Uh oh!

JordanYates left a comment

Choose a reason for hiding this comment

Uh oh!

JordanYates Nov 23, 2021

Choose a reason for hiding this comment

Uh oh!

KozhinovAlexander commented Nov 23, 2021

Uh oh!

JordanYates commented Nov 25, 2021

Uh oh!

carlocaione commented Nov 25, 2021

Uh oh!

carlocaione commented Dec 13, 2021

Uh oh!

github-actions bot commented Feb 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

KozhinovAlexander Nov 22, 2021 •

edited

Loading