Skip to content

Conversation

@carlocaione
Copy link
Contributor

@carlocaione carlocaione commented Jul 31, 2023

This is the final step in making the zephyr,memory-attr property actually useful.

The problem with the current implementation is that zephyr,memory-attr is an enum type, this is making very difficult to use that to actually describe the memory capabilities. The solution proposed in this PR is to use the zephyr,memory-attr property as an OR-ed bitmask of memory attributes.

Important

With the change proposed in this PR it is possible in the DeviceTree to mark the memory regions with a bitmask of attributes by using the zephyr,memory-attr property. This property and the related memory region can then be retrieved at run-time by leveraging a provided helper library or the usual DT helpers.

The set of general attributes that can be specified in the property are defined and explained in include/zephyr/dt-bindings/memory-attr/memory-attr.h (the list can be extended when needed).

https://github.com/zephyrproject-rtos/zephyr/blob/5664796d1f5b525dc27bb6dfc862c35e7c5b7b13/include/zephyr/dt-bindings/memory-attr/memory-attr.h#L16-L20

For example, to mark a memory region in the DeviceTree as volatile, non-cacheable, out-of-order:

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-attr = <( DT_MEM_VOLATILE | DT_MEM_NON_CACHEABLE | DT_MEM_OOO )>;
   };

Important

The zephyr,memory-attr property can also be used to set architecture-specific custom attributes that can be interpreted at run time. This is leveraged, among other things, to create MPU regions out of DeviceTree defined memory regions on ARM, for example:

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-region = "NOCACHE_REGION";
       zephyr,memory-attr = <( DT_ARM_MPU(ATTR_MPU_RAM_NOCACHE) )>;
   };

See include/zephyr/dt-bindings/memory-attr/memory-attr-mpu.h to see how an architecture can define its own special memory attributes (in this case ARM MPU).

Important

The property can also be used to set custom software-specific attributes. For example we can think of marking a memory region as available to be used for memory allocation (not yet implemented):

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-attr = <( DT_MEM_NON_CACHEABLE | DT_MEM_SW_ALLOCATABLE )>;
   };

Or maybe we can leverage the property to specify some alignment requirements for the region:

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-attr = <( DT_MEM_CACHEABLE | DT_MEM_SW_ALIGN(32) )>;
   };

Important

The conventional and recommended way to deal and manage with memory regions marked with attributes is by using the provided mem-attr helper library by enabling CONFIG_MEM_ATTR (or by using the usual DT helpers).

When this option is enabled the list of memory regions and their attributes are compiled in a user-accessible array and a set of functions is made available that can be used to query, probe and act on regions and attributes, see include/zephyr/mem_mgmt/mem_attr.h

Note

Note that the zephyr,memory-attr property is only a descriptive property of the capabilities of the associated memory region, but it does not result in any actual setting for the memory to be set. The user, code or subsystem willing to use this information to do some work (for example creating an MPU region out of the property) must use either the provided mem-attr library or the usual DeviceTree helpers to perform the required work / setting.

Note: how to review this PR

Warning

Unfortunately this PR cannot be divided into smaller PRs to not break bisectability.

In general this PR we are:

  • Removing the previous DT-based helpers for zephyr,memory-attr, these were recently introduced but proved to not be up to the task
  • Adding the int version of the DT property, see dts/bindings/base/zephyr,memory-attr.yaml, include/zephyr/dt-bindings/memory-attr/memory-attr.h and include/zephyr/dt-bindings/memory-attr/memory-attr-mpu.h
  • Adding the mem-attr library, see include/zephyr/mem_mgmt/mem_attr.h and subsys/mem_mgmt/mem_attr.c
  • Reworking the ARM MPU drivers to use the mem-attr to create the DT-defined MPU regions: see arch/arm/core/aarch32/mpu/arm_mpu.c and arch/arm64/core/cortex_r/arm_mpu.c
  • Fixing all the DTS and moving from enum to int
  • Adding documentation in doc/services/mem_mgmt/index.rst

@carlocaione carlocaione changed the title wip Move zephyr,memory-attr to a capabilities bitmask Jul 31, 2023
@carlocaione carlocaione changed the title Move zephyr,memory-attr to a capabilities bitmask Make zephyr,memory-attr a capabilities bitmask Jul 31, 2023
@carlocaione carlocaione changed the title Make zephyr,memory-attr a capabilities bitmask [RFC] Make zephyr,memory-attr a capabilities bitmask Aug 1, 2023
@carlocaione carlocaione marked this pull request as ready for review August 1, 2023 09:50
@hubertmis
Copy link
Member

I'm a little confused. I understand I can use zephyr,memory-attr = <( DT_MEM_VOLATILE | DT_MEM_NON_CACHEABLE | DT_MEM_OOO )>; or zephyr,memory-attr = <( DT_ARM_MPU(ATTR_MPU_RAM_NOCACHE) )>;.

But would these result in the same MPU entry if added to an ARM's .dts file?

@carlocaione
Copy link
Contributor Author

I'm a little confused. I understand I can use zephyr,memory-attr = <( DT_MEM_VOLATILE | DT_MEM_NON_CACHEABLE | DT_MEM_OOO )>; or zephyr,memory-attr = <( DT_ARM_MPU(ATTR_MPU_RAM_NOCACHE) )>;.

But would these result in the same MPU entry if added to an ARM's .dts file?

No, you can create an MPU entry only using the DT_ARM_MPU(...) macro and that is to be considered legacy and it is there only to support the current way we have in Zephyr to create MPU regions.

zephyr,memory-attr = <( DT_ARM_MPU(ATTR_MPU_RAM_NOCACHE) )>; is creating an MPU region because in the ARM MPU driver I added the code to do that to be backward compatible with the current way we have to create MPU regions.

zephyr,memory-attr = <( DT_MEM_VOLATILE | DT_MEM_NON_CACHEABLE | DT_MEM_OOO )>; is only describing the kind of memory. it COULD be used to create an MPU regions but currently there is no code doing that.

I tried to be as much clear as possible here: https://github.com/zephyrproject-rtos/zephyr/blob/5664796d1f5b525dc27bb6dfc862c35e7c5b7b13/doc/services/mem_mgmt/index.rst?plain=1#L57-L64

and here:
https://github.com/zephyrproject-rtos/zephyr/blob/5664796d1f5b525dc27bb6dfc862c35e7c5b7b13/include/zephyr/dt-bindings/memory-attr/memory-attr-mpu.h#L12-L20

@dcpleung
Copy link
Member

LGTM on Xtensa.

Copy link
Contributor

@mbolivar-ampere mbolivar-ampere left a comment

Choose a reason for hiding this comment

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

Thanks for the nice doc and for a smooth migration path. I have a few minor questions and nitpicks

povergoing
povergoing previously approved these changes Sep 6, 2023
@carlescufi
Copy link
Member

Want to make sure we get input/mapping for multiple architectures for this (arm, arc, xtensa, risc-v, x86).

Given that @teburd and @evgeniy-paltsev are both in the conversation I will dismiss the -1 after we get a +1 from both.

povergoing
povergoing previously approved these changes Sep 12, 2023
Copy link
Member

@povergoing povergoing left a comment

Choose a reason for hiding this comment

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

refresh +1

@carlescufi
Copy link
Member

@carlocaione please rebase

Copy link
Contributor

@mbolivar-ampere mbolivar-ampere left a comment

Choose a reason for hiding this comment

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

LGTM, a couple of nonblocking suggestions. Thanks for taking the time to document the transition guide.

@carlocaione carlocaione added the DNM This PR should not be merged (Do Not Merge) label Sep 14, 2023
@carlocaione
Copy link
Contributor Author

Adding DNM because this needs to be squashed before merging.

This is the final step in making the `zephyr,memory-attr` property
actually useful.

The problem with the current implementation is that `zephyr,memory-attr`
is an enum type, this is making very difficult to use that to actually
describe the memory capabilities. The solution proposed in this PR is to
use the `zephyr,memory-attr` property as an OR-ed bitmask of memory
attributes.

With the change proposed in this PR it is possible in the DeviceTree to
mark the memory regions with a bitmask of attributes by using the
`zephyr,memory-attr` property. This property and the related memory
region can then be retrieved at run-time by leveraging a provided helper
library or the usual DT helpers.

The set of general attributes that can be specified in the property are
defined and explained in
`include/zephyr/dt-bindings/memory-attr/memory-attr.h` (the list can be
extended when needed).

For example, to mark a memory region in the DeviceTree as volatile,
non-cacheable, out-of-order:

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-attr = <( DT_MEM_VOLATILE |
			       DT_MEM_NON_CACHEABLE |
			       DT_MEM_OOO )>;
   };

The `zephyr,memory-attr` property can also be used to set
architecture-specific custom attributes that can be interpreted at run
time. This is leveraged, among other things, to create MPU regions out
of DeviceTree defined memory regions on ARM, for example:

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-region = "NOCACHE_REGION";
       zephyr,memory-attr = <( DT_ARM_MPU(ATTR_MPU_RAM_NOCACHE) )>;
   };

See `include/zephyr/dt-bindings/memory-attr/memory-attr-mpu.h` to see
how an architecture can define its own special memory attributes (in
this case ARM MPU).

The property can also be used to set custom software-specific
attributes. For example we can think of marking a memory region as
available to be used for memory allocation (not yet implemented):

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-attr = <( DT_MEM_NON_CACHEABLE |
			       DT_MEM_SW_ALLOCATABLE )>;
   };

Or maybe we can leverage the property to specify some alignment
requirements for the region:

   mem: memory@10000000 {
       compatible = "mmio-sram";
       reg = <0x10000000 0x1000>;
       zephyr,memory-attr = <( DT_MEM_CACHEABLE |
			       DT_MEM_SW_ALIGN(32) )>;
   };

The conventional and recommended way to deal and manage with memory
regions marked with attributes is by using the provided `mem-attr`
helper library by enabling `CONFIG_MEM_ATTR` (or by using the usual DT
helpers).

When this option is enabled the list of memory regions and their
attributes are compiled in a user-accessible array and a set of
functions is made available that can be used to query, probe and act on
regions and attributes, see `include/zephyr/mem_mgmt/mem_attr.h`

Note that the `zephyr,memory-attr` property is only a descriptive
property of the capabilities of the associated memory  region, but it
does not result in any actual setting for the memory to be set. The
user, code or subsystem willing to use this information to do some work
(for example creating an MPU region out of the property) must use either
the provided `mem-attr` library or the usual DeviceTree helpers to
perform the required work / setting.

Signed-off-by: Carlo Caione <[email protected]>
@carlocaione carlocaione removed the DNM This PR should not be merged (Do Not Merge) label Sep 15, 2023
@carlescufi carlescufi merged commit e4a125b into zephyrproject-rtos:main Sep 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Architecture Review Discussion in the Architecture WG required treewide 🧹

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.