[PATCH] arm/monitor: Add support for 'info tlb' command

Changbin Du posted 1 patch 3 years, 5 months ago
Test checkpatch failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20201113095854.67668-1-changbin.du@gmail.com
Maintainers: Peter Maydell <peter.maydell@linaro.org>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>
There is a newer version of this series
hmp-commands-info.hx   |   3 +-
target/arm/helper.c    |  17 +---
target/arm/internals.h |  16 ++++
target/arm/monitor.c   | 187 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 206 insertions(+), 17 deletions(-)
[PATCH] arm/monitor: Add support for 'info tlb' command
Posted by Changbin Du 3 years, 5 months ago
This adds hmp 'info tlb' command support for the arm platform.
The limitation is that this only implements a page walker for
ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
not supported yet.

Signed-off-by: Changbin Du <changbin.du@gmail.com>
---
 hmp-commands-info.hx   |   3 +-
 target/arm/helper.c    |  17 +---
 target/arm/internals.h |  16 ++++
 target/arm/monitor.c   | 187 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 206 insertions(+), 17 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 117ba25f91..1b5b3f2616 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -222,7 +222,8 @@ SRST
 ERST
 
 #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
-    defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
+    defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) || \
+    defined(TARGET_ARM)
     {
         .name       = "tlb",
         .args_type  = "",
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 11b0803df7..c73a08943b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -9974,8 +9974,7 @@ static inline uint64_t regime_sctlr(CPUARMState *env, ARMMMUIdx mmu_idx)
 #ifndef CONFIG_USER_ONLY
 
 /* Return true if the specified stage of address translation is disabled */
-static inline bool regime_translation_disabled(CPUARMState *env,
-                                               ARMMMUIdx mmu_idx)
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
     if (arm_feature(env, ARM_FEATURE_M)) {
         switch (env->v7m.mpu_ctrl[regime_is_secure(env, mmu_idx)] &
@@ -10021,20 +10020,6 @@ static inline bool regime_translation_big_endian(CPUARMState *env,
     return (regime_sctlr(env, mmu_idx) & SCTLR_EE) != 0;
 }
 
-/* Return the TTBR associated with this translation regime */
-static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
-                                   int ttbrn)
-{
-    if (mmu_idx == ARMMMUIdx_Stage2) {
-        return env->cp15.vttbr_el2;
-    }
-    if (ttbrn == 0) {
-        return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
-    } else {
-        return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
-    }
-}
-
 #endif /* !CONFIG_USER_ONLY */
 
 /* Convert a possible stage1+2 MMU index into the appropriate
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 5460678756..12f883c636 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -949,6 +949,8 @@ static inline uint32_t regime_el(CPUARMState *env, ARMMMUIdx mmu_idx)
     }
 }
 
+bool regime_translation_disabled(CPUARMState *env, ARMMMUIdx mmu_idx);
+
 /* Return the TCR controlling this translation regime */
 static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
 {
@@ -958,6 +960,20 @@ static inline TCR *regime_tcr(CPUARMState *env, ARMMMUIdx mmu_idx)
     return &env->cp15.tcr_el[regime_el(env, mmu_idx)];
 }
 
+/* Return the TTBR associated with this translation regime */
+static inline uint64_t regime_ttbr(CPUARMState *env, ARMMMUIdx mmu_idx,
+                                   int ttbrn)
+{
+    if (mmu_idx == ARMMMUIdx_Stage2) {
+        return env->cp15.vttbr_el2;
+    }
+    if (ttbrn == 0) {
+        return env->cp15.ttbr0_el[regime_el(env, mmu_idx)];
+    } else {
+        return env->cp15.ttbr1_el[regime_el(env, mmu_idx)];
+    }
+}
+
 /* Return the FSR value for a debug exception (watchpoint, hardware
  * breakpoint or BKPT insn) targeting the specified exception level.
  */
diff --git a/target/arm/monitor.c b/target/arm/monitor.c
index 169d8a64b6..d6b64ea3b6 100644
--- a/target/arm/monitor.c
+++ b/target/arm/monitor.c
@@ -31,6 +31,9 @@
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qdict.h"
 #include "qom/qom-qobject.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp-target.h"
+#include "internals.h"
 
 static GICCapability *gic_cap_new(int version)
 {
@@ -236,3 +239,187 @@ CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType type,
 
     return expansion_info;
 }
+
+/* Perform linear address sign extension */
+static target_ulong addr_canonical(int va_bits, target_ulong addr)
+{
+#ifdef TARGET_AARCH64
+    if (addr & (1UL << (va_bits - 1))) {
+        addr |= (hwaddr)-(1L << va_bits);
+    }
+#endif
+
+    return addr;
+}
+
+#define PTE_HEADER_FIELDS       "vaddr            paddr            "\
+                                "size             attr\n"
+#define PTE_HEADER_DELIMITER    "---------------- ---------------- "\
+                                "---------------- ------------------------------\n"
+
+static void print_pte_header(Monitor *mon)
+{
+    monitor_printf(mon, PTE_HEADER_FIELDS);
+    monitor_printf(mon, PTE_HEADER_DELIMITER);
+}
+
+static void
+print_pte_lpae(Monitor *mon, uint32_t tableattrs, int va_bits, target_ulong vaddr,
+               hwaddr paddr, target_ulong size, target_ulong pte)
+{
+    uint32_t ns = extract64(pte, 5, 1) | extract32(tableattrs, 4, 1);
+    uint32_t ap = extract64(pte, 6, 2) & ~extract32(tableattrs, 2, 2);
+    uint32_t af = extract64(pte, 10, 1);
+    uint32_t ng = extract64(pte, 11, 1);
+    uint32_t gp = extract64(pte, 50, 1);
+    uint32_t con = extract64(pte, 52, 1);
+    uint32_t pxn = extract64(pte, 53, 1) | extract32(tableattrs, 0, 1);
+    uint32_t uxn = extract64(pte, 54, 1) | extract32(tableattrs, 1, 1);
+
+    monitor_printf(mon, TARGET_FMT_lx " " TARGET_FMT_plx " " TARGET_FMT_lx
+                   " %s %s %s %s %s %s %s %s %s\n",
+                   addr_canonical(va_bits, vaddr), paddr, size,
+                   ap & 0x2 ? "ro" : "RW",
+                   ap & 0x1 ? "USR" : "   ",
+                   ns ? "NS" : "  ",
+                   af ? "AF" : "  ",
+                   ng ? "nG" : "  ",
+                   gp ? "GP" : "  ",
+                   con ? "Con" : "   ",
+                   pxn ? "PXN" : "   ",
+                   uxn ? "UXN" : "   ");
+}
+
+static void
+walk_pte_lpae(Monitor *mon, bool aarch64, uint32_t tableattrs, hwaddr pt_base,
+              target_ulong vstart, int cur_level, int stride, int va_bits)
+{
+    int pg_shift = stride + 3;
+    int descaddr_high = aarch64 ? 47 : 39;
+    int max_level = 3;
+    int ptshift= pg_shift + (max_level - cur_level) * stride;
+    target_ulong pgsize = 1UL << ptshift;
+    int idx;
+
+    for (idx = 0; idx < (1UL << stride) && vstart < (1UL << va_bits);
+         idx++, vstart += pgsize) {
+        hwaddr pte_addr = pt_base + idx * 8;
+        target_ulong pte = 0;
+        hwaddr paddr;
+
+        cpu_physical_memory_read(pte_addr, &pte, 8);
+
+        if (!extract64(pte, 0, 1)) {
+            /* invalid entry */
+            continue;
+        }
+
+        if (cur_level == max_level) {
+            /* leaf entry */
+            paddr = (hwaddr)extract64(pte, pg_shift,
+                                descaddr_high - pg_shift + 1) << pg_shift;
+            print_pte_lpae(mon, tableattrs, va_bits, vstart, paddr, pgsize, pte);
+        } else {
+            if (extract64(pte, 1, 1)) {
+                /* table entry */
+                paddr = (hwaddr)extract64(pte, pg_shift,
+                                    descaddr_high - pg_shift + 1) << pg_shift;
+                tableattrs |= extract64(pte, 59, 5);
+
+                walk_pte_lpae(mon, aarch64, tableattrs, paddr, vstart,
+                              cur_level + 1, stride, va_bits);
+            } else {
+                /* block entry */
+                if ((pg_shift == 12 && (cur_level != 1 && cur_level != 2)) ||
+                    (pg_shift == 14 && (cur_level != 2)) ||
+                    (pg_shift == 16 && (cur_level != 0 && cur_level != 1))) {
+                    monitor_printf(mon, "illegal block entry at level%d\n",
+                                   cur_level);
+                    continue;
+                }
+                paddr = (hwaddr)extract64(pte, ptshift,
+                                    descaddr_high - ptshift + 1) << ptshift;
+                print_pte_lpae(mon, tableattrs, va_bits, vstart, paddr,
+                               pgsize, pte);
+            }
+        }
+    }
+}
+
+static inline int pt_start_level_vmsav8_64(int stride, int va_bits)
+{
+    int pg_shift = stride + 3;
+
+    return 3 - (va_bits - pg_shift - 1) / stride;
+}
+
+/* ARMv8-A AArch64 Long Descriptor format */
+static void tlb_info_vmsav8_64(Monitor *mon, CPUArchState *env)
+{
+    ARMMMUIdx mmu_idx = arm_stage1_mmu_idx(env);
+    uint64_t ttbr[2];
+    uint64_t tcr;
+    int tsz[2];
+    bool using16k, using64k;
+    int stride;
+
+    ttbr[0] = regime_ttbr(env, mmu_idx, 0);
+    ttbr[1] = regime_ttbr(env, mmu_idx, 1);
+
+    tcr = regime_tcr(env, mmu_idx)->raw_tcr;
+    using64k = extract32(tcr, 14, 1);
+    using16k = extract32(tcr, 15, 1);
+    tsz[0] = extract32(tcr, 0, 6);
+    tsz[1] = extract32(tcr, 16, 6);
+
+    if (using64k) {
+        stride = 13;
+    } else if (using16k) {
+        stride = 11;
+    } else {
+        stride = 9;
+    }
+
+    /* print header */
+    print_pte_header(mon);
+
+    for (unsigned int i = 0; i < 2; i++) {
+        if (ttbr[i]) {
+            hwaddr base = extract64(ttbr[i], 1, 47) << 1;
+            int va_bits = 64 - tsz[i];
+            target_ulong vstart = (target_ulong)i << (va_bits - 1);
+            int cur_level = pt_start_level_vmsav8_64(stride, va_bits);
+
+            /* walk ttbrx page tables, starting from address @vstart */
+            walk_pte_lpae(mon, true, 0, base, vstart, cur_level, stride, va_bits);
+        }
+    }
+}
+
+void hmp_info_tlb(Monitor *mon, const QDict *qdict)
+{
+    CPUArchState *env;
+
+    env = mon_get_cpu_env();
+    if (!env) {
+        monitor_printf(mon, "No CPU available\n");
+        return;
+    }
+
+    if (arm_feature(env, ARM_FEATURE_PMSA)) {
+        monitor_printf(mon, "No MMU\n");
+        return;
+    }
+
+    if (regime_translation_disabled(env, arm_stage1_mmu_idx(env))) {
+        monitor_printf(mon, "MMU disabled\n");
+        return;
+    }
+
+    if (!arm_el_is_aa64(env, 1)) {
+        monitor_printf(mon, "Only AArch64 Long Descriptor is supported\n");
+        return;
+    }
+
+    tlb_info_vmsav8_64(mon, env);
+}
-- 
2.25.1


Re: [PATCH] arm/monitor: Add support for 'info tlb' command
Posted by no-reply@patchew.org 3 years, 5 months ago
Patchew URL: https://patchew.org/QEMU/20201113095854.67668-1-changbin.du@gmail.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20201113095854.67668-1-changbin.du@gmail.com
Type: series
Subject: [PATCH] arm/monitor: Add support for 'info tlb' command

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20201113095854.67668-1-changbin.du@gmail.com -> patchew/20201113095854.67668-1-changbin.du@gmail.com
Switched to a new branch 'test'
a33e6e7 arm/monitor: Add support for 'info tlb' command

=== OUTPUT BEGIN ===
WARNING: line over 80 characters
#141: FILE: target/arm/monitor.c:267:
+print_pte_lpae(Monitor *mon, uint32_t tableattrs, int va_bits, target_ulong vaddr,

ERROR: spaces required around that '=' (ctx:VxW)
#174: FILE: target/arm/monitor.c:300:
+    int ptshift= pg_shift + (max_level - cur_level) * stride;
                ^

WARNING: line over 80 characters
#195: FILE: target/arm/monitor.c:321:
+            print_pte_lpae(mon, tableattrs, va_bits, vstart, paddr, pgsize, pte);

WARNING: line over 80 characters
#268: FILE: target/arm/monitor.c:394:
+            walk_pte_lpae(mon, true, 0, base, vstart, cur_level, stride, va_bits);

total: 1 errors, 3 warnings, 262 lines checked

Commit a33e6e7e0edd (arm/monitor: Add support for 'info tlb' command) has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201113095854.67668-1-changbin.du@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] arm/monitor: Add support for 'info tlb' command
Posted by Peter Maydell 3 years, 5 months ago
On Fri, 13 Nov 2020 at 09:59, Changbin Du <changbin.du@gmail.com> wrote:
>
> This adds hmp 'info tlb' command support for the arm platform.
> The limitation is that this only implements a page walker for
> ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
> not supported yet.

"info tlb" needs its own entirely independent implementation
of a page-table walk? I see this is how x86 has done it ,but
it seems like a recipe for the info command being perpetually
behind what we actually implement...

thanks
-- PMM

Re: [PATCH] arm/monitor: Add support for 'info tlb' command
Posted by Dr. David Alan Gilbert 3 years, 5 months ago
* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Fri, 13 Nov 2020 at 09:59, Changbin Du <changbin.du@gmail.com> wrote:
> >
> > This adds hmp 'info tlb' command support for the arm platform.
> > The limitation is that this only implements a page walker for
> > ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
> > not supported yet.
> 
> "info tlb" needs its own entirely independent implementation
> of a page-table walk? I see this is how x86 has done it ,but
> it seems like a recipe for the info command being perpetually
> behind what we actually implement...

I think the challenge is you want 'info tlb' to be quite easy
to read from a debuggers point of view and robust at pointing out
things that are odd; I'd hope at least you can use most of the macros
to extract fields etc

Dave

> thanks
> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


Re: [PATCH] arm/monitor: Add support for 'info tlb' command
Posted by Changbin Du 3 years, 5 months ago
On Fri, Nov 13, 2020 at 01:13:42PM +0000, Dr. David Alan Gilbert wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
> > On Fri, 13 Nov 2020 at 09:59, Changbin Du <changbin.du@gmail.com> wrote:
> > >
> > > This adds hmp 'info tlb' command support for the arm platform.
> > > The limitation is that this only implements a page walker for
> > > ARMv8-A AArch64 Long Descriptor format, 32bit addressing is
> > > not supported yet.
> > 
> > "info tlb" needs its own entirely independent implementation
> > of a page-table walk? I see this is how x86 has done it ,but
> > it seems like a recipe for the info command being perpetually
> > behind what we actually implement...
> 
> I think the challenge is you want 'info tlb' to be quite easy
> to read from a debuggers point of view and robust at pointing out
> things that are odd; I'd hope at least you can use most of the macros
> to extract fields etc
>
Actually I refered to function get_phys_addr_lpae() and try to use
definitions what I found. But most of the bit definitions are
hardcode.

I have fixed the code style issues in V2. Thanks for your kind review!
 
> Dave
> 
> > thanks
> > -- PMM
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

-- 
Cheers,
Changbin Du