xlat: Remove mmap_attr_t enum type
authorAntonio Nino Diaz <antonio.ninodiaz@arm.com>
Thu, 21 Jun 2018 13:39:16 +0000 (14:39 +0100)
committerAntonio Nino Diaz <antonio.ninodiaz@arm.com>
Fri, 22 Jun 2018 07:36:21 +0000 (08:36 +0100)
The values defined in this type are used in logical operations, which
goes against MISRA Rule 10.1: "Operands shall not be of an inappropriate
essential type".

Now, `unsigned int` is used instead. This also allows us to move the
dynamic mapping bit from 30 to 31. It was an undefined behaviour in the
past because an enum is signed by default, and bit 31 corresponds to the
sign bit. It is undefined behaviour to modify the sign bit. Now, bit 31
is free to use as it was originally meant to be.

mmap_attr_t is now defined as an `unsigned int` for backwards
compatibility.

Change-Id: I6b31218c14b9c7fdabebe432de7fae6e90a97f34
Signed-off-by: Antonio Nino Diaz <antonio.ninodiaz@arm.com>
docs/xlat-tables-lib-v2-design.rst
include/lib/xlat_tables/xlat_tables.h
include/lib/xlat_tables/xlat_tables_v2.h
lib/utils/mem_region.c
lib/xlat_tables/xlat_tables_common.c
lib/xlat_tables_v2/xlat_tables_internal.c
lib/xlat_tables_v2/xlat_tables_private.h

index d256b6b5780ec5919f71c8624d9ef3799451524a..d207f301e698b08e94fd05284f020e0a26a2ba46 100644 (file)
@@ -79,9 +79,9 @@ The region attributes specify the type of memory (for example device or cached
 normal memory) as well as the memory access permissions (read-only or
 read-write, executable or not, secure or non-secure, and so on). In the case of
 the EL1&0 translation regime, the attributes also specify whether the region is
-a User region (EL0) or Privileged region (EL1). See the ``mmap_attr_t``
-enumeration type in `xlat\_tables\_v2.h`_. Note that for the EL1&0 translation
-regime the Execute Never attribute is set simultaneously for both EL1 and EL0.
+a User region (EL0) or Privileged region (EL1). See the ``MT_xxx`` definitions
+in `xlat\_tables\_v2.h`_. Note that for the EL1&0 translation regime the Execute
+Never attribute is set simultaneously for both EL1 and EL0.
 
 The granularity controls the translation table level to go down to when mapping
 the region. For example, assuming the MMU has been configured to use a 4KB
index 91f2f055b8dc95ba77c233279042afbeec374d54..c017e193d11679b87cae7218fac8b2e1c9db6b85 100644 (file)
@@ -25,7 +25,7 @@
 #define MAP_REGION(pa, va, sz, attr) {(pa), (va), (sz), (attr)}
 
 /*
- * Shifts and masks to access fields of an mmap_attr_t
+ * Shifts and masks to access fields of an mmap attribute
  */
 #define MT_TYPE_MASK   U(0x7)
 #define MT_TYPE(_attr) ((_attr) & MT_TYPE_MASK)
 /*
  * Memory mapping attributes
  */
-typedef enum  {
-       /*
-        * Memory types supported.
-        * These are organised so that, going down the list, the memory types
-        * are getting weaker; conversely going up the list the memory types are
-        * getting stronger.
-        */
-       MT_DEVICE,
-       MT_NON_CACHEABLE,
-       MT_MEMORY,
-       /* Values up to 7 are reserved to add new memory types in the future */
-
-       MT_RO           = U(0) << MT_PERM_SHIFT,
-       MT_RW           = U(1) << MT_PERM_SHIFT,
-
-       MT_SECURE       = U(0) << MT_SEC_SHIFT,
-       MT_NS           = U(1) << MT_SEC_SHIFT,
-
-       /*
-        * Access permissions for instruction execution are only relevant for
-        * normal read-only memory, i.e. MT_MEMORY | MT_RO. They are ignored
-        * (and potentially overridden) otherwise:
-        *  - Device memory is always marked as execute-never.
-        *  - Read-write normal memory is always marked as execute-never.
-        */
-       MT_EXECUTE              = U(0) << MT_EXECUTE_SHIFT,
-       MT_EXECUTE_NEVER        = U(1) << MT_EXECUTE_SHIFT,
-} mmap_attr_t;
-
-#define MT_CODE                (MT_MEMORY | MT_RO | MT_EXECUTE)
-#define MT_RO_DATA     (MT_MEMORY | MT_RO | MT_EXECUTE_NEVER)
+
+/*
+ * Memory types supported.
+ * These are organised so that, going down the list, the memory types are
+ * getting weaker; conversely going up the list the memory types are getting
+ * stronger.
+ */
+#define MT_DEVICE              U(0)
+#define MT_NON_CACHEABLE       U(1)
+#define MT_MEMORY              U(2)
+/* Values up to 7 are reserved to add new memory types in the future */
+
+#define MT_RO                  (U(0) << MT_PERM_SHIFT)
+#define MT_RW                  (U(1) << MT_PERM_SHIFT)
+
+#define MT_SECURE              (U(0) << MT_SEC_SHIFT)
+#define MT_NS                  (U(1) << MT_SEC_SHIFT)
+
+/*
+ * Access permissions for instruction execution are only relevant for normal
+ * read-only memory, i.e. MT_MEMORY | MT_RO. They are ignored (and potentially
+ * overridden) otherwise:
+ *  - Device memory is always marked as execute-never.
+ *  - Read-write normal memory is always marked as execute-never.
+ */
+#define MT_EXECUTE             (U(0) << MT_EXECUTE_SHIFT)
+#define MT_EXECUTE_NEVER       (U(1) << MT_EXECUTE_SHIFT)
+
+/* Compound attributes for most common usages */
+#define MT_CODE                        (MT_MEMORY | MT_RO | MT_EXECUTE)
+#define MT_RO_DATA             (MT_MEMORY | MT_RO | MT_EXECUTE_NEVER)
+
+#if !ERROR_DEPRECATED
+typedef unsigned int mmap_attr_t;
+#endif
 
 /*
  * Structure for specifying a single region of memory.
@@ -78,13 +82,13 @@ typedef struct mmap_region {
        unsigned long long      base_pa;
        uintptr_t               base_va;
        size_t                  size;
-       mmap_attr_t             attr;
+       unsigned int            attr;
 } mmap_region_t;
 
 /* Generic translation table APIs */
 void init_xlat_tables(void);
 void mmap_add_region(unsigned long long base_pa, uintptr_t base_va,
-                               size_t size, mmap_attr_t attr);
+                    size_t size, unsigned int attr);
 void mmap_add(const mmap_region_t *mm);
 
 #endif /*__ASSEMBLY__*/
index b2379454dd63fc6f046069878943f627bace8cdd..98f00d7156735f04d707ccd903eceaf085f0f6b4 100644 (file)
@@ -47,7 +47,7 @@
        _MAP_REGION_FULL_SPEC(_pa, _va, _sz, _attr, _gr)
 
 /*
- * Shifts and masks to access fields of an mmap_attr_t
+ * Shifts and masks to access fields of an mmap attribute
  */
 #define MT_TYPE_MASK           U(0x7)
 #define MT_TYPE(_attr)         ((_attr) & MT_TYPE_MASK)
 #define MT_SEC_SHIFT           U(4)
 /* Access permissions for instruction execution (EXECUTE/EXECUTE_NEVER) */
 #define MT_EXECUTE_SHIFT       U(5)
-/*
- * In the EL1&0 translation regime, mark the region as User (EL0) or
- * Privileged (EL1). In the EL3 translation regime this has no effect.
- */
+/* In the EL1&0 translation regime, User (EL0) or Privileged (EL1). */
 #define MT_USER_SHIFT          U(6)
 /* All other bits are reserved */
 
 /*
  * Memory mapping attributes
  */
-typedef enum  {
-       /*
-        * Memory types supported.
-        * These are organised so that, going down the list, the memory types
-        * are getting weaker; conversely going up the list the memory types are
-        * getting stronger.
-        */
-       MT_DEVICE,
-       MT_NON_CACHEABLE,
-       MT_MEMORY,
-       /* Values up to 7 are reserved to add new memory types in the future */
-
-       MT_RO           = U(0) << MT_PERM_SHIFT,
-       MT_RW           = U(1) << MT_PERM_SHIFT,
-
-       MT_SECURE       = U(0) << MT_SEC_SHIFT,
-       MT_NS           = U(1) << MT_SEC_SHIFT,
-
-       /*
-        * Access permissions for instruction execution are only relevant for
-        * normal read-only memory, i.e. MT_MEMORY | MT_RO. They are ignored
-        * (and potentially overridden) otherwise:
-        *  - Device memory is always marked as execute-never.
-        *  - Read-write normal memory is always marked as execute-never.
-        */
-       MT_EXECUTE              = U(0) << MT_EXECUTE_SHIFT,
-       MT_EXECUTE_NEVER        = U(1) << MT_EXECUTE_SHIFT,
-
-       /*
-        * When mapping a region at EL0 or EL1, this attribute will be used to
-        * determine if a User mapping (EL0) will be created or a Privileged
-        * mapping (EL1).
-        */
-       MT_USER                         = U(1) << MT_USER_SHIFT,
-       MT_PRIVILEGED                   = U(0) << MT_USER_SHIFT,
-} mmap_attr_t;
+
+/*
+ * Memory types supported.
+ * These are organised so that, going down the list, the memory types are
+ * getting weaker; conversely going up the list the memory types are getting
+ * stronger.
+ */
+#define MT_DEVICE              U(0)
+#define MT_NON_CACHEABLE       U(1)
+#define MT_MEMORY              U(2)
+/* Values up to 7 are reserved to add new memory types in the future */
+
+#define MT_RO                  (U(0) << MT_PERM_SHIFT)
+#define MT_RW                  (U(1) << MT_PERM_SHIFT)
+
+#define MT_SECURE              (U(0) << MT_SEC_SHIFT)
+#define MT_NS                  (U(1) << MT_SEC_SHIFT)
+
+/*
+ * Access permissions for instruction execution are only relevant for normal
+ * read-only memory, i.e. MT_MEMORY | MT_RO. They are ignored (and potentially
+ * overridden) otherwise:
+ *  - Device memory is always marked as execute-never.
+ *  - Read-write normal memory is always marked as execute-never.
+ */
+#define MT_EXECUTE             (U(0) << MT_EXECUTE_SHIFT)
+#define MT_EXECUTE_NEVER       (U(1) << MT_EXECUTE_SHIFT)
+
+/*
+ * When mapping a region at EL0 or EL1, this attribute will be used to determine
+ * if a User mapping (EL0) will be created or a Privileged mapping (EL1).
+ */
+#define MT_USER                        (U(1) << MT_USER_SHIFT)
+#define MT_PRIVILEGED          (U(0) << MT_USER_SHIFT)
 
 /* Compound attributes for most common usages */
-#define MT_CODE                (MT_MEMORY | MT_RO | MT_EXECUTE)
-#define MT_RO_DATA     (MT_MEMORY | MT_RO | MT_EXECUTE_NEVER)
-#define MT_RW_DATA     (MT_MEMORY | MT_RW | MT_EXECUTE_NEVER)
+#define MT_CODE                        (MT_MEMORY | MT_RO | MT_EXECUTE)
+#define MT_RO_DATA             (MT_MEMORY | MT_RO | MT_EXECUTE_NEVER)
+#define MT_RW_DATA             (MT_MEMORY | MT_RW | MT_EXECUTE_NEVER)
+
+#if !ERROR_DEPRECATED
+typedef unsigned int mmap_attr_t;
+#endif
 
 /*
  * Structure for specifying a single region of memory.
@@ -116,7 +115,7 @@ typedef struct mmap_region {
        unsigned long long      base_pa;
        uintptr_t               base_va;
        size_t                  size;
-       mmap_attr_t             attr;
+       unsigned int            attr;
        /* Desired granularity. See the MAP_REGION2() macro for more details. */
        size_t                  granularity;
 } mmap_region_t;
@@ -213,7 +212,7 @@ void init_xlat_tables_ctx(xlat_ctx_t *ctx);
  * removed afterwards.
  */
 void mmap_add_region(unsigned long long base_pa, uintptr_t base_va,
-                               size_t size, mmap_attr_t attr);
+                    size_t size, unsigned int attr);
 void mmap_add_region_ctx(xlat_ctx_t *ctx, const mmap_region_t *mm);
 
 /*
@@ -238,7 +237,7 @@ void mmap_add_ctx(xlat_ctx_t *ctx, const mmap_region_t *mm);
  *    EPERM: It overlaps another region in an invalid way.
  */
 int mmap_add_dynamic_region(unsigned long long base_pa, uintptr_t base_va,
-                               size_t size, mmap_attr_t attr);
+                           size_t size, unsigned int attr);
 int mmap_add_dynamic_region_ctx(xlat_ctx_t *ctx, mmap_region_t *mm);
 
 /*
index 24c2c1ded4afefccab616f7d5d550ccc9478d04e..e9541ba320edade7e552b3d88471ce5ef88861e1 100644 (file)
@@ -58,7 +58,7 @@ void clear_map_dyn_mem_regions(mem_region_t *regions,
        uintptr_t begin;
        int r;
        size_t size;
-       const mmap_attr_t attr = MT_MEMORY|MT_RW|MT_NS;
+       const unsigned int attr = MT_MEMORY | MT_RW | MT_NS;
 
        assert(regions != NULL);
        assert(nregions > 0 && chunk > 0);
index 21bf48973975ac0b7b1c6dcc53fc5c237f77b362..b42cd6814245d1cd5fcbb00bd58dca5f820d2529 100644 (file)
@@ -66,7 +66,7 @@ void print_mmap(void)
 }
 
 void mmap_add_region(unsigned long long base_pa, uintptr_t base_va,
-                       size_t size, mmap_attr_t attr)
+                    size_t size, unsigned int attr)
 {
        mmap_region_t *mm = mmap;
        mmap_region_t *mm_last = mm + ARRAY_SIZE(mmap) - 1;
@@ -178,8 +178,8 @@ void mmap_add(const mmap_region_t *mm)
        }
 }
 
-static uint64_t mmap_desc(mmap_attr_t attr, unsigned long long addr_pa,
-                                                       unsigned int level)
+static uint64_t mmap_desc(unsigned int attr, unsigned long long addr_pa,
+                         unsigned int level)
 {
        uint64_t desc;
        int mem_type;
@@ -264,7 +264,7 @@ static uint64_t mmap_desc(mmap_attr_t attr, unsigned long long addr_pa,
  * value pointed by attr should be ignored by the caller.
  */
 static int mmap_region_attr(mmap_region_t *mm, uintptr_t base_va,
-                                       size_t size, mmap_attr_t *attr)
+                           size_t size, unsigned int *attr)
 {
        /* Don't assume that the area is contained in the first region */
        int ret = -1;
@@ -348,7 +348,7 @@ static mmap_region_t *init_xlation_table_inner(mmap_region_t *mm,
                         * there are partially overlapping regions. On success,
                         * it will return the innermost region's attributes.
                         */
-                       mmap_attr_t attr;
+                       unsigned int attr;
                        int r = mmap_region_attr(mm, base_va, level_size, &attr);
 
                        if (!r) {
index 3b586b2a0449d01df8b30a6da3bd382e438e124c..5beb51e90a2eb3896ba4b976b937d910c1e83ec4 100644 (file)
@@ -823,10 +823,8 @@ void mmap_add_region_ctx(xlat_ctx_t *ctx, const mmap_region_t *mm)
                ctx->max_va = end_va;
 }
 
-void mmap_add_region(unsigned long long base_pa,
-                               uintptr_t base_va,
-                               size_t size,
-                               mmap_attr_t attr)
+void mmap_add_region(unsigned long long base_pa, uintptr_t base_va, size_t size,
+                    unsigned int attr)
 {
        mmap_region_t mm = MAP_REGION(base_pa, base_va, size, attr);
        mmap_add_region_ctx(&tf_xlat_ctx, &mm);
@@ -947,8 +945,8 @@ int mmap_add_dynamic_region_ctx(xlat_ctx_t *ctx, mmap_region_t *mm)
        return 0;
 }
 
-int mmap_add_dynamic_region(unsigned long long base_pa,
-                           uintptr_t base_va, size_t size, mmap_attr_t attr)
+int mmap_add_dynamic_region(unsigned long long base_pa, uintptr_t base_va,
+                           size_t size, unsigned int attr)
 {
        mmap_region_t mm = MAP_REGION(base_pa, base_va, size, attr);
        return mmap_add_dynamic_region_ctx(&tf_xlat_ctx, &mm);
index 07963b187cd73b75682be10944587503fc7673d3..157dd039622a0960a0bbef2cc2e42488d4fbf394 100644 (file)
 
 #if PLAT_XLAT_TABLES_DYNAMIC
 /*
- * Shifts and masks to access fields of an mmap_attr_t
+ * Private shifts and masks to access fields of an mmap attribute
  */
 /* Dynamic or static */
-#define MT_DYN_SHIFT           30 /* 31 would cause undefined behaviours */
+#define MT_DYN_SHIFT           U(31)
 
 /*
  * Memory mapping private attributes
  *
- * Private attributes not exposed in the mmap_attr_t enum.
+ * Private attributes not exposed in the public header.
  */
-typedef enum  {
-       /*
-        * Regions mapped before the MMU can't be unmapped dynamically (they are
-        * static) and regions mapped with MMU enabled can be unmapped. This
-        * behaviour can't be overridden.
-        *
-        * Static regions can overlap each other, dynamic regions can't.
-        */
-       MT_STATIC       = 0 << MT_DYN_SHIFT,
-       MT_DYNAMIC      = 1 << MT_DYN_SHIFT
-} mmap_priv_attr_t;
+
+/*
+ * Regions mapped before the MMU can't be unmapped dynamically (they are
+ * static) and regions mapped with MMU enabled can be unmapped. This
+ * behaviour can't be overridden.
+ *
+ * Static regions can overlap each other, dynamic regions can't.
+ */
+#define MT_STATIC      (U(0) << MT_DYN_SHIFT)
+#define MT_DYNAMIC     (U(1) << MT_DYN_SHIFT)
 
 #endif /* PLAT_XLAT_TABLES_DYNAMIC */