On 1/30/25 07:20, Ilias Apalodimas wrote:
Upcoming patches are mapping memory with RO, RW^X etc permsissions.
Fix the meminfo command to display them properly

Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
---
  arch/arm/cpu/armv8/cache_v8.c    | 18 +++++++++++++++++-
  arch/arm/include/asm/armv8/mmu.h |  2 ++
  cmd/meminfo.c                    |  5 +++++
  3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index 5d6953ffedd1..a4ca56c8ed42 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -421,7 +421,7 @@ static int count_ranges(void)
        return count;
  }

-#define ALL_ATTRS (3 << 8 | PMD_ATTRINDX_MASK)
+#define ALL_ATTRS (3 << 8 | PMD_ATTRMASK)
  #define PTE_IS_TABLE(pte, level) (pte_type(&(pte)) == PTE_TYPE_TABLE && (level) 
< 3)

  enum walker_state {
@@ -568,6 +568,17 @@ static void pretty_print_table_attrs(u64 pte)
  static void pretty_print_block_attrs(u64 pte)
  {
        u64 attrs = pte & PMD_ATTRINDX_MASK;
+       u64 perm_attrs = pte & PMD_ATTRMASK;

The variable perm_attrs is not needed. You could use pte directly.

+
+       if ((perm_attrs & PTE_BLOCK_PXN) && (perm_attrs & PTE_BLOCK_UXN) &&
+           perm_attrs & PTE_BLOCK_RO)
+               printf("%-5s", " | RO");
+       else if ((perm_attrs & PTE_BLOCK_PXN) && (perm_attrs & PTE_BLOCK_UXN))
+               printf("%-5s", " | RW");
+       else if (perm_attrs & PTE_BLOCK_RO)
+               printf("%-5s", " | RX");
+       else
+               printf("%-5s", " | RWX");

The string has 6 characters. "%-5s" does not match!
As the string length is known using a length identifier is superfluous.

It would be helpful to add descriptions to the flags in
arch/arm/include/asm/armv8/mmu.h.

https://documentation-service.arm.com/static/5efa1d23dbdee951c1ccdec5
describes:

"The Unprivileged Execute Never (UXN) and Privileged Execute Never (PXN)
attributes can be set separately. This is used to prevent, for example,
application code running with kernel privilege, or attempts to execute
kernel code while in an unprivileged state."

Shouldn't we show the values of UXN and PXN separately?

E.g:

printf("%s%s%s\",
        pte & PTE_BLOCK_RO  ? " | RO " : "",
        pte & PTE_BLOCK_PXN ? " | PXN" : "",
        pte & PTE_BLOCK_UXN ? " | UXN" : "");

Best regards

Heinrich


        switch (attrs) {
        case PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE):
@@ -1112,3 +1123,8 @@ void __weak enable_caches(void)
        icache_enable();
        dcache_enable();
  }
+
+void arch_dump_mem_attrs(void)
+{
+       dump_pagetable(gd->arch.tlb_addr, get_tcr(NULL, NULL));
+}
diff --git a/arch/arm/include/asm/armv8/mmu.h b/arch/arm/include/asm/armv8/mmu.h
index 0ab681c893d3..6af8cd111a44 100644
--- a/arch/arm/include/asm/armv8/mmu.h
+++ b/arch/arm/include/asm/armv8/mmu.h
@@ -66,6 +66,7 @@
  #define PTE_BLOCK_NG          (1 << 11)
  #define PTE_BLOCK_PXN         (UL(1) << 53)
  #define PTE_BLOCK_UXN         (UL(1) << 54)
+#define PTE_BLOCK_RO            (UL(1) << 7)

  /*
   * AttrIndx[2:0]
@@ -75,6 +76,7 @@
  #define PMD_ATTRMASK          (PTE_BLOCK_PXN          | \
                                 PTE_BLOCK_UXN          | \
                                 PMD_ATTRINDX_MASK      | \
+                                PTE_BLOCK_RO           | \
                                 PTE_TYPE_VALID)

  /*
diff --git a/cmd/meminfo.c b/cmd/meminfo.c
index 5e83d61c2dd3..3915e2bbb268 100644
--- a/cmd/meminfo.c
+++ b/cmd/meminfo.c
@@ -15,6 +15,10 @@

  DECLARE_GLOBAL_DATA_PTR;

+void __weak arch_dump_mem_attrs(void)
+{
+}
+
  static void print_region(const char *name, ulong base, ulong size, ulong 
*uptop)
  {
        ulong end = base + size;
@@ -54,6 +58,7 @@ static int do_meminfo(struct cmd_tbl *cmdtp, int flag, int 
argc,

        puts("DRAM:  ");
        print_size(gd->ram_size, "\n");
+       arch_dump_mem_attrs();

        if (!IS_ENABLED(CONFIG_CMD_MEMINFO_MAP))
                return 0;

Reply via email to