[PATCH] hmp: allow filtering `info tlb` entries by address on i386

Josh Junon posted 1 patch 3 months ago
hmp-commands-info.hx  |  5 +++++
target/i386/monitor.c | 45 +++++++++++++++++++++++++++----------------
2 files changed, 33 insertions(+), 17 deletions(-)
[PATCH] hmp: allow filtering `info tlb` entries by address on i386
Posted by Josh Junon 3 months ago
This change adds an optional virtual address parameter
to the `info tlb` monitor command on i386 targets,
only printing a specific entry if found.

Signed-off-by: Josh Junon <junon@oro.sh>
---
 hmp-commands-info.hx  |  5 +++++
 target/i386/monitor.c | 45 +++++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index c59cd6637b..e42427b738 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -199,8 +199,13 @@ ERST
     defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
     {
         .name       = "tlb",
+#if defined(TARGET_I386)
+        .args_type  = "virt:l?",
+        .params     = "[virt]",
+#else
         .args_type  = "",
         .params     = "",
+#endif
         .help       = "show virtual to physical memory mappings",
         .cmd        = hmp_info_tlb,
     },
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 2d766b2637..f54d400c97 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -50,10 +50,14 @@ static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
 }
 
 static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
-                      hwaddr pte, hwaddr mask)
+                      hwaddr pte, hwaddr mask, hwaddr *filter)
 {
     addr = addr_canonical(env, addr);
 
+    if (filter && (*filter & mask) != (addr & mask)) {
+        return;
+    }
+
     monitor_printf(mon, HWADDR_FMT_plx ": " HWADDR_FMT_plx
                    " %c%c%c%c%c%c%c%c%c\n",
                    addr,
@@ -69,7 +73,7 @@ static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
                    pte & PG_RW_MASK ? 'W' : '-');
 }
 
-static void tlb_info_32(Monitor *mon, CPUArchState *env)
+static void tlb_info_32(Monitor *mon, CPUArchState *env, hwaddr *filter)
 {
     unsigned int l1, l2;
     uint32_t pgd, pde, pte;
@@ -81,7 +85,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
         if (pde & PG_PRESENT_MASK) {
             if ((pde & PG_PSE_MASK) && (env->cr[4] & CR4_PSE_MASK)) {
                 /* 4M pages */
-                print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1));
+                print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1), filter);
             } else {
                 for(l2 = 0; l2 < 1024; l2++) {
                     cpu_physical_memory_read((pde & ~0xfff) + l2 * 4, &pte, 4);
@@ -89,7 +93,8 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
                     if (pte & PG_PRESENT_MASK) {
                         print_pte(mon, env, (l1 << 22) + (l2 << 12),
                                   pte & ~PG_PSE_MASK,
-                                  ~0xfff);
+                                  ~0xfff,
+                                  filter);
                     }
                 }
             }
@@ -97,7 +102,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
     }
 }
 
-static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
+static void tlb_info_pae32(Monitor *mon, CPUArchState *env, hwaddr *filter)
 {
     unsigned int l1, l2, l3;
     uint64_t pdpe, pde, pte;
@@ -116,7 +121,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
                     if (pde & PG_PSE_MASK) {
                         /* 2M pages with PAE, CR4.PSE is ignored */
                         print_pte(mon, env, (l1 << 30) + (l2 << 21), pde,
-                                  ~((hwaddr)(1 << 20) - 1));
+                                  ~((hwaddr)(1 << 20) - 1),
+                                  filter);
                     } else {
                         pt_addr = pde & 0x3fffffffff000ULL;
                         for (l3 = 0; l3 < 512; l3++) {
@@ -126,7 +132,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
                                 print_pte(mon, env, (l1 << 30) + (l2 << 21)
                                           + (l3 << 12),
                                           pte & ~PG_PSE_MASK,
-                                          ~(hwaddr)0xfff);
+                                          ~(hwaddr)0xfff,
+                                          filter);
                             }
                         }
                     }
@@ -138,7 +145,7 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
 
 #ifdef TARGET_X86_64
 static void tlb_info_la48(Monitor *mon, CPUArchState *env,
-        uint64_t l0, uint64_t pml4_addr)
+        uint64_t l0, uint64_t pml4_addr, hwaddr *filter)
 {
     uint64_t l1, l2, l3, l4;
     uint64_t pml4e, pdpe, pde, pte;
@@ -162,7 +169,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
             if (pdpe & PG_PSE_MASK) {
                 /* 1G pages, CR4.PSE is ignored */
                 print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30),
-                        pdpe, 0x3ffffc0000000ULL);
+                        pdpe, 0x3ffffc0000000ULL, filter);
                 continue;
             }
 
@@ -177,7 +184,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
                 if (pde & PG_PSE_MASK) {
                     /* 2M pages, CR4.PSE is ignored */
                     print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30) +
-                            (l3 << 21), pde, 0x3ffffffe00000ULL);
+                            (l3 << 21), pde, 0x3ffffffe00000ULL, filter);
                     continue;
                 }
 
@@ -190,7 +197,8 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
                     if (pte & PG_PRESENT_MASK) {
                         print_pte(mon, env, (l0 << 48) + (l1 << 39) +
                                 (l2 << 30) + (l3 << 21) + (l4 << 12),
-                                pte & ~PG_PSE_MASK, 0x3fffffffff000ULL);
+                                pte & ~PG_PSE_MASK, 0x3fffffffff000ULL,
+                                filter);
                     }
                 }
             }
@@ -198,7 +206,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
     }
 }
 
-static void tlb_info_la57(Monitor *mon, CPUArchState *env)
+static void tlb_info_la57(Monitor *mon, CPUArchState *env, hwaddr *filter)
 {
     uint64_t l0;
     uint64_t pml5e;
@@ -209,7 +217,7 @@ static void tlb_info_la57(Monitor *mon, CPUArchState *env)
         cpu_physical_memory_read(pml5_addr + l0 * 8, &pml5e, 8);
         pml5e = le64_to_cpu(pml5e);
         if (pml5e & PG_PRESENT_MASK) {
-            tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL);
+            tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL, filter);
         }
     }
 }
@@ -219,6 +227,9 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
 {
     CPUArchState *env;
 
+    hwaddr filter_addr = qdict_get_try_int(qdict, "virt", 0);
+    hwaddr *filter = qdict_haskey(qdict, "virt") ? &filter_addr : NULL;
+
     env = mon_get_cpu_env(mon);
     if (!env) {
         monitor_printf(mon, "No CPU available\n");
@@ -233,17 +244,17 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
 #ifdef TARGET_X86_64
         if (env->hflags & HF_LMA_MASK) {
             if (env->cr[4] & CR4_LA57_MASK) {
-                tlb_info_la57(mon, env);
+                tlb_info_la57(mon, env, filter);
             } else {
-                tlb_info_la48(mon, env, 0, env->cr[3] & 0x3fffffffff000ULL);
+                tlb_info_la48(mon, env, 0, env->cr[3] & 0x3fffffffff000ULL, filter);
             }
         } else
 #endif
         {
-            tlb_info_pae32(mon, env);
+            tlb_info_pae32(mon, env, filter);
         }
     } else {
-        tlb_info_32(mon, env);
+        tlb_info_32(mon, env, filter);
     }
 }
 
-- 
2.34.1
Re: [PATCH] hmp: allow filtering `info tlb` entries by address on i386
Posted by Dr. David Alan Gilbert 1 month, 3 weeks ago
* Josh Junon (junon@oro.sh) wrote:
> This change adds an optional virtual address parameter
> to the `info tlb` monitor command on i386 targets,
> only printing a specific entry if found.

Hi Josh,

> Signed-off-by: Josh Junon <junon@oro.sh>
> ---
>  hmp-commands-info.hx  |  5 +++++
>  target/i386/monitor.c | 45 +++++++++++++++++++++++++++----------------
>  2 files changed, 33 insertions(+), 17 deletions(-)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index c59cd6637b..e42427b738 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -199,8 +199,13 @@ ERST
>      defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
>      {
>          .name       = "tlb",
> +#if defined(TARGET_I386)
> +        .args_type  = "virt:l?",
> +        .params     = "[virt]",
> +#else
>          .args_type  = "",
>          .params     = "",
> +#endif

I don't think we've got any other case where we only define a parameter
on some architecture; and it feels best to avoid adding it now.

>          .help       = "show virtual to physical memory mappings",

The meaning of parameters should be documented in the .help;
you could add a comment there saying 'only some architectures'

>          .cmd        = hmp_info_tlb,
>      },
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 2d766b2637..f54d400c97 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -50,10 +50,14 @@ static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
>  }
>  
>  static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> -                      hwaddr pte, hwaddr mask)
> +                      hwaddr pte, hwaddr mask, hwaddr *filter)
>  {
>      addr = addr_canonical(env, addr);
>  
> +    if (filter && (*filter & mask) != (addr & mask)) {
> +        return;
> +    }
> +
>      monitor_printf(mon, HWADDR_FMT_plx ": " HWADDR_FMT_plx
>                     " %c%c%c%c%c%c%c%c%c\n",
>                     addr,
> @@ -69,7 +73,7 @@ static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
>                     pte & PG_RW_MASK ? 'W' : '-');
>  }
>  
> -static void tlb_info_32(Monitor *mon, CPUArchState *env)
> +static void tlb_info_32(Monitor *mon, CPUArchState *env, hwaddr *filter)
>  {
>      unsigned int l1, l2;
>      uint32_t pgd, pde, pte;
> @@ -81,7 +85,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
>          if (pde & PG_PRESENT_MASK) {
>              if ((pde & PG_PSE_MASK) && (env->cr[4] & CR4_PSE_MASK)) {
>                  /* 4M pages */
> -                print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1));
> +                print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1), filter);
>              } else {
>                  for(l2 = 0; l2 < 1024; l2++) {
>                      cpu_physical_memory_read((pde & ~0xfff) + l2 * 4, &pte, 4);
> @@ -89,7 +93,8 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
>                      if (pte & PG_PRESENT_MASK) {
>                          print_pte(mon, env, (l1 << 22) + (l2 << 12),
>                                    pte & ~PG_PSE_MASK,
> -                                  ~0xfff);
> +                                  ~0xfff,
> +                                  filter);
>                      }
>                  }
>              }
> @@ -97,7 +102,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
>      }
>  }
>  
> -static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
> +static void tlb_info_pae32(Monitor *mon, CPUArchState *env, hwaddr *filter)
>  {
>      unsigned int l1, l2, l3;
>      uint64_t pdpe, pde, pte;
> @@ -116,7 +121,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
>                      if (pde & PG_PSE_MASK) {
>                          /* 2M pages with PAE, CR4.PSE is ignored */
>                          print_pte(mon, env, (l1 << 30) + (l2 << 21), pde,
> -                                  ~((hwaddr)(1 << 20) - 1));
> +                                  ~((hwaddr)(1 << 20) - 1),
> +                                  filter);
>                      } else {
>                          pt_addr = pde & 0x3fffffffff000ULL;
>                          for (l3 = 0; l3 < 512; l3++) {
> @@ -126,7 +132,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
>                                  print_pte(mon, env, (l1 << 30) + (l2 << 21)
>                                            + (l3 << 12),
>                                            pte & ~PG_PSE_MASK,
> -                                          ~(hwaddr)0xfff);
> +                                          ~(hwaddr)0xfff,
> +                                          filter);
>                              }
>                          }
>                      }
> @@ -138,7 +145,7 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
>  
>  #ifdef TARGET_X86_64
>  static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> -        uint64_t l0, uint64_t pml4_addr)
> +        uint64_t l0, uint64_t pml4_addr, hwaddr *filter)
>  {
>      uint64_t l1, l2, l3, l4;
>      uint64_t pml4e, pdpe, pde, pte;
> @@ -162,7 +169,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
>              if (pdpe & PG_PSE_MASK) {
>                  /* 1G pages, CR4.PSE is ignored */
>                  print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30),
> -                        pdpe, 0x3ffffc0000000ULL);
> +                        pdpe, 0x3ffffc0000000ULL, filter);
>                  continue;
>              }
>  
> @@ -177,7 +184,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
>                  if (pde & PG_PSE_MASK) {
>                      /* 2M pages, CR4.PSE is ignored */
>                      print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30) +
> -                            (l3 << 21), pde, 0x3ffffffe00000ULL);
> +                            (l3 << 21), pde, 0x3ffffffe00000ULL, filter);
>                      continue;
>                  }
>  
> @@ -190,7 +197,8 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
>                      if (pte & PG_PRESENT_MASK) {
>                          print_pte(mon, env, (l0 << 48) + (l1 << 39) +
>                                  (l2 << 30) + (l3 << 21) + (l4 << 12),
> -                                pte & ~PG_PSE_MASK, 0x3fffffffff000ULL);
> +                                pte & ~PG_PSE_MASK, 0x3fffffffff000ULL,
> +                                filter);
>                      }
>                  }
>              }
> @@ -198,7 +206,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
>      }
>  }
>  
> -static void tlb_info_la57(Monitor *mon, CPUArchState *env)
> +static void tlb_info_la57(Monitor *mon, CPUArchState *env, hwaddr *filter)
>  {
>      uint64_t l0;
>      uint64_t pml5e;
> @@ -209,7 +217,7 @@ static void tlb_info_la57(Monitor *mon, CPUArchState *env)
>          cpu_physical_memory_read(pml5_addr + l0 * 8, &pml5e, 8);
>          pml5e = le64_to_cpu(pml5e);
>          if (pml5e & PG_PRESENT_MASK) {
> -            tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL);
> +            tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL, filter);
>          }
>      }
>  }
> @@ -219,6 +227,9 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>  {
>      CPUArchState *env;
>  
> +    hwaddr filter_addr = qdict_get_try_int(qdict, "virt", 0);
> +    hwaddr *filter = qdict_haskey(qdict, "virt") ? &filter_addr : NULL;
> +

I'm a bit confused about the format of the parameter you're trying to use;
can you add an example in the commit message please.

>      env = mon_get_cpu_env(mon);
>      if (!env) {
>          monitor_printf(mon, "No CPU available\n");
> @@ -233,17 +244,17 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
>  #ifdef TARGET_X86_64
>          if (env->hflags & HF_LMA_MASK) {
>              if (env->cr[4] & CR4_LA57_MASK) {
> -                tlb_info_la57(mon, env);
> +                tlb_info_la57(mon, env, filter);
>              } else {
> -                tlb_info_la48(mon, env, 0, env->cr[3] & 0x3fffffffff000ULL);
> +                tlb_info_la48(mon, env, 0, env->cr[3] & 0x3fffffffff000ULL, filter);

I think you're over line length there.

>              }
>          } else
>  #endif
>          {
> -            tlb_info_pae32(mon, env);
> +            tlb_info_pae32(mon, env, filter);
>          }
>      } else {
> -        tlb_info_32(mon, env);
> +        tlb_info_32(mon, env, filter);
>      }
>  }
>  
> -- 
> 2.34.1
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/