Re: [PATCH v2] target/i386/monitor: Refine `info tlb` command

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Alano Song (alanosong@163.com) wrote:
> At 2026-07-11 00:52:35, "Dr. David Alan Gilbert" <dave@treblig.org> wrote:
> >* Alano Song (alanosong@163.com) wrote:
> >
> >
> >See below:
> >
> >> At 2026-07-02 05:05:58, "Dr. David Alan Gilbert" <dave@treblig.org> wrote:
> >> >(Added Phil's new address in)
> >> >
> >> >* AlanoSong@163.com (AlanoSong@163.com) wrote:
> >> >> When booting an i386 target, the `info tlb` command
> >> >> may walk the entire page table hierarchy and emit
> >> >> an enormous amount of output.
> >> >> It will take dozens of minutes to print all the info,
> >> >> because each monitor print function will holds 'mon_lock'.
> >> >> This effectively hangs the qemu monitor.
> >> >> 
> >> >> It may be better to add warning note in help message,
> >> >> and add a address range by argument to help user control
> >> >> the number of output items.
> >> >> 
> >> >> Signed-off-by: Alano Song <AlanoSong@163.com>
> >> >> ---
> >> >>  hmp-commands-info.hx  | 12 ++++---
> >> >>  target/i386/monitor.c | 82 +++++++++++++++++++++++++++++--------------
> >> >>  2 files changed, 64 insertions(+), 30 deletions(-)
> >> >> 
> >> >> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> >> >> index 82134eb6c2..3189e20de3 100644
> >> >> --- a/hmp-commands-info.hx
> >> >> +++ b/hmp-commands-info.hx
> >> >> @@ -188,17 +188,21 @@ ERST
> >> >>  
> >> >>      {
> >> >>          .name       = "tlb",
> >> >> -        .args_type  = "",
> >> >> -        .params     = "",
> >> >> -        .help       = "show virtual to physical memory mappings",
> >> >> +        .args_type  = "start:l?,end:l?",
> >> >> +        .params     = "[start [end]]",
> >> >> +        .help       = "show virtual to physical memory mappings. "
> >> >
> >> >> +                      "output can be extremely large for i386 target. "
> >> >> +                      "use 'info tlb [start] [end]' to show a range of entries.",
> >> >
> >> >That can be;
> >> >> +                      "The address range can be limited with the optional parameters."
> >> >
> >> >Now, is that the *virtual* address - because it feels like that needs specifying.
> >> >
> >> >>          .cmd        = hmp_info_tlb,
> >> >>          .arch_bitmask = QEMU_ARCH_I386 | QEMU_ARCH_SH4 | QEMU_ARCH_SPARC \
> >> >>                          | QEMU_ARCH_PPC | QEMU_ARCH_XTENSA | QEMU_ARCH_M68K,
> >> >
> >> >You have added the start/end params to the help, but only implemented it on x86.
> >> >That seems odd.
> >> >
> >> >Dave
> >> 
> >> >
> >> 
> >> 
> >> Hi Dr. Gilbert,
> >> 
> >> On this patch, I’m currently enabling the `start`/`end` parameters only for
> >> x86, since other targets typically maintain a relatively small TLB.
> >> 
> >> For example, xtensa only has a handful of entries, as shown below, so it’s
> >> reasonable to simply ignore start/end parameters and dump all items:
> >
> >We shouldn't blindly ignore parameters; we should always tell the user,
> >this is mainly because a user would be very confused if they were using these
> >parameters and still getting pages of output from say sparc32.
> >
> >Perhaps if you create a little helper function that the unsupported
> >hmp_info_tlb's can call to check it.  Something like
> >
> >
> >  if (hmp_check_unsupported_info_tlb_params(qdict,something)) {
> >      return;
> >  }
> >
> >so then it's easy to add that to the ones that's not supported.
> >
> >> // target/xtensa/cpu.h
> >> ...
> >> #define MAX_TLB_WAY_SIZE 8
> >> ...
> >> struct CPUArchState {
> >> ...
> >> #ifndef CONFIG_USER_ONLY
> >>     xtensa_tlb_entry itlb[7][MAX_TLB_WAY_SIZE];
> >>     xtensa_tlb_entry dtlb[10][MAX_TLB_WAY_SIZE];
> >> ...
> >> 
> >> The same applies to sparc64, ppc, and sh4.
> >> 
> >> 
> >> Thanks for the reminder.
> >> I went through all architectures supporting the
> >> 
> >> `info tlb` command. I noticed that sparc32 and m68k may also produce a
> >> large number of TLB entries, so I plan to extend start/end filtering
> >> to those as well.
> >> What do you think?
> >> 
> >Yeh that would be great, I think the more important bit is not to confuse
> >users about ones which aren't supported.
> >
> >> >>  SRST
> >> >> -  ``info tlb``
> >> >> +  ``info tlb`` [*start* [*end*]]
> >> >>      Show virtual to physical memory mappings.
> >> >> +    The output can be extremely large for i386 target.
> >> >> +    Use *start* and *end* to print entries located in address range [start, end].
> >> >>  ERST
> >
> >I think I'd change it to something like 
> >  *start* and *end can restrict the address range printed to [start, end]
> >  (This restriction is only supported on i386)
> >
> >Dave
> >
> 
> Thanks for your patient review.
> 
> 
> I will update the v3 patch as follows:
> 1. Enable the [start, end] filter for i386/sparc32/m68k target.
>     Clarify in the helper info that this feature is
>     limited to the 3 targets above.
> 2. Add a helper function to emit a warning when the filter is
>     used on unsupported targets.
> 3. Refine the helper info about [starg, end] filter.

Great - thanks for adding the extras.

Dave

> 
> Alano
> >> Alano.
> >> 
> >> 
> >> >>      },
> >> >>  
> >> >>  SRST
> >> >> -  ``info tlb``
> >> >> +  ``info tlb`` [*start* [*end*]]
> >> >>      Show virtual to physical memory mappings.
> >> >> +    The output can be extremely large for i386 target.
> >> >> +    Use *start* and *end* to print entries located in address range [start, end].
> >> >>  ERST
> >> >>  
> >> >>      {
> >> >> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> >> >> index a536712c75..c0e0c3e405 100644
> >> >> --- a/target/i386/monitor.c
> >> >> +++ b/target/i386/monitor.c
> >> >> @@ -48,11 +48,9 @@ static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
> >> >>      return addr;
> >> >>  }
> >> >>  
> >> >> -static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> >> >> -                      hwaddr pte, hwaddr mask)
> >> >> +static void do_print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> >> >> +                         hwaddr pte, hwaddr mask)
> >> >>  {
> >> >> -    addr = addr_canonical(env, addr);
> >> >> -
> >> >>      monitor_printf(mon, HWADDR_FMT_plx ": " HWADDR_FMT_plx
> >> >>                     " %c%c%c%c%c%c%c%c%c\n",
> >> >>                     addr,
> >> >> @@ -68,7 +66,20 @@ static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> >> >>                     pte & PG_RW_MASK ? 'W' : '-');
> >> >>  }
> >> >>  
> >> >> -static void tlb_info_32(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >> +static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> >> >> +                      hwaddr pte, hwaddr mask,
> >> >> +                      hwaddr start, hwaddr end)
> >> >> +{
> >> >> +    addr = addr_canonical(env, addr);
> >> >> +    if (addr < start || addr > end) {
> >> >> +        return;
> >> >> +    }
> >> >> +
> >> >> +    do_print_pte(mon, env, addr, pte, mask);
> >> >> +}
> >> >> +
> >> >> +static void tlb_info_32(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >> +                        hwaddr start, hwaddr end)
> >> >>  {
> >> >>      const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> >> >>      unsigned int l1, l2;
> >> >> @@ -80,15 +91,15 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >>          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),
> >> >> +                          start, end);
> >> >>              } else {
> >> >>                  for(l2 = 0; l2 < 1024; l2++) {
> >> >>                      pte = address_space_ldl_le(as, (pde & ~0xfff) + l2 * 4,
> >> >>                                                 attrs, NULL);
> >> >>                      if (pte & PG_PRESENT_MASK) {
> >> >>                          print_pte(mon, env, (l1 << 22) + (l2 << 12),
> >> >> -                                  pte & ~PG_PSE_MASK,
> >> >> -                                  ~0xfff);
> >> >> +                                  pte & ~PG_PSE_MASK, ~0xfff, start, end);
> >> >>                      }
> >> >>                  }
> >> >>              }
> >> >> @@ -96,7 +107,8 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >>      }
> >> >>  }
> >> >>  
> >> >> -static void tlb_info_pae32(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >> +static void tlb_info_pae32(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >> +                           hwaddr start, hwaddr end)
> >> >>  {
> >> >>      const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> >> >>      unsigned int l1, l2, l3;
> >> >> @@ -114,17 +126,19 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >>                      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),
> >> >> +                                  start, end);
> >> >>                      } else {
> >> >>                          pt_addr = pde & 0x3fffffffff000ULL;
> >> >>                          for (l3 = 0; l3 < 512; l3++) {
> >> >>                              pte = address_space_ldq_le(as, pt_addr + l3 * 8,
> >> >>                                                         attrs, NULL);
> >> >>                              if (pte & PG_PRESENT_MASK) {
> >> >> -                                print_pte(mon, env, (l1 << 30) + (l2 << 21)
> >> >> -                                          + (l3 << 12),
> >> >> +                                print_pte(mon, env,
> >> >> +                                          (l1 << 30) + (l2 << 21) + (l3 << 12),
> >> >>                                            pte & ~PG_PSE_MASK,
> >> >> -                                          ~(hwaddr)0xfff);
> >> >> +                                          ~(hwaddr)0xfff,
> >> >> +                                          start, end);
> >> >>                              }
> >> >>                          }
> >> >>                      }
> >> >> @@ -136,7 +150,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >>  
> >> >>  #ifdef TARGET_X86_64
> >> >>  static void tlb_info_la48(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >> -        uint64_t l0, uint64_t pml4_addr)
> >> >> +                          uint64_t l0, uint64_t pml4_addr, hwaddr start,
> >> >> +                          hwaddr end)
> >> >>  {
> >> >>      const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> >> >>      uint64_t l1, l2, l3, l4;
> >> >> @@ -159,7 +174,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >>              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, start, end);
> >> >>                  continue;
> >> >>              }
> >> >>  
> >> >> @@ -172,8 +187,9 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >>  
> >> >>                  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);
> >> >> +                    print_pte(mon, env,
> >> >> +                              (l0 << 48) + (l1 << 39) + (l2 << 30) + (l3 << 21),
> >> >> +                              pde, 0x3ffffffe00000ULL, start, end);
> >> >>                      continue;
> >> >>                  }
> >> >>  
> >> >> @@ -182,9 +198,11 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >>                      pte = address_space_ldq_le(as, pt_addr + l4 * 8,
> >> >>                                                 attrs, NULL);
> >> >>                      if (pte & PG_PRESENT_MASK) {
> >> >> -                        print_pte(mon, env, (l0 << 48) + (l1 << 39) +
> >> >> -                                (l2 << 30) + (l3 << 21) + (l4 << 12),
> >> >> -                                pte & ~PG_PSE_MASK, 0x3fffffffff000ULL);
> >> >> +                        print_pte(mon, env,
> >> >> +                                  (l0 << 48) + (l1 << 39) + (l2 << 30) +
> >> >> +                                  (l3 << 21) + (l4 << 12),
> >> >> +                                  pte & ~PG_PSE_MASK,
> >> >> +                                  0x3fffffffff000ULL, start, end);
> >> >>                      }
> >> >>                  }
> >> >>              }
> >> >> @@ -192,7 +210,8 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >>      }
> >> >>  }
> >> >>  
> >> >> -static void tlb_info_la57(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >> +static void tlb_info_la57(Monitor *mon, CPUArchState *env, AddressSpace *as,
> >> >> +                           hwaddr start, hwaddr end)
> >> >>  {
> >> >>      const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> >> >>      uint64_t l0;
> >> >> @@ -203,7 +222,8 @@ static void tlb_info_la57(Monitor *mon, CPUArchState *env, AddressSpace *as)
> >> >>      for (l0 = 0; l0 < 512; l0++) {
> >> >>          pml5e = address_space_ldq_le(as, pml5_addr + l0 * 8, attrs, NULL);
> >> >>          if (pml5e & PG_PRESENT_MASK) {
> >> >> -            tlb_info_la48(mon, env, as, l0, pml5e & 0x3fffffffff000ULL);
> >> >> +            tlb_info_la48(mon, env, as, l0, pml5e & 0x3fffffffff000ULL,
> >> >> +                          start, end);
> >> >>          }
> >> >>      }
> >> >>  }
> >> >> @@ -213,6 +233,14 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> >> >>  {
> >> >>      CPUArchState *env;
> >> >>      AddressSpace *as;
> >> >> +    hwaddr start = 0, end = HWADDR_MAX;
> >> >> +
> >> >> +    if (qdict_haskey(qdict, "start")) {
> >> >> +        start = (hwaddr)qdict_get_int(qdict, "start");
> >> >> +    }
> >> >> +    if (qdict_haskey(qdict, "end")) {
> >> >> +        end = (hwaddr)qdict_get_int(qdict, "end");
> >> >> +    }
> >> >>  
> >> >>      env = mon_get_cpu_env(mon);
> >> >>      if (!env) {
> >> >> @@ -229,17 +257,19 @@ 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, as);
> >> >> +                tlb_info_la57(mon, env, as, start, end);
> >> >>              } else {
> >> >> -                tlb_info_la48(mon, env, as, 0, env->cr[3] & 0x3fffffffff000ULL);
> >> >> +                tlb_info_la48(mon, env, as, 0,
> >> >> +                              env->cr[3] & 0x3fffffffff000ULL,
> >> >> +                              start, end);
> >> >>              }
> >> >>          } else
> >> >>  #endif
> >> >>          {
> >> >> -            tlb_info_pae32(mon, env, as);
> >> >> +            tlb_info_pae32(mon, env, as, start, end);
> >> >>          }
> >> >>      } else {
> >> >> -        tlb_info_32(mon, env, as);
> >> >> +        tlb_info_32(mon, env, as, start, end);
> >> >>      }
> >> >>  }
> >> >>  
> >> >> -- 
> >> >> 2.43.0
> >> >> 
> >> >-- 
> >> > -----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   |_______/
> >-- 
> > -----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   |_______/
-- 
 -----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   |_______/