Alano Song <[email protected]> writes: > 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.
Reproducer? I'm asking because my quick test shows just "PG disabled". Cryptic, but not your patch's job to fix. Oh, after the guest runs for a while, I do get a lot of output. But printing it takes maybe a second. > And the same situation also occurred on sparc32 and > m68k targets. > > So this change do the following: > 1) Add a address range argument to help user control > the number of output items. > 2) Add warning note in help message that such address > range argument only supported on target i386, > sparc32 and m68k. > 3) Print ignore warning when user add such address > range argument on other targets (sh4, sparc64, > ppc and xtensa). Those targets only print limited > tlb info. > > Signed-off-by: Alano Song <[email protected]> > Reviewed-by: Dr. David Alan Gilbert <[email protected]> > Reviewed-by: Marc-André Lureau <[email protected]> > --- > hmp-commands-info.hx | 17 ++++++-- > target/i386/monitor.c | 92 ++++++++++++++++++++++++++++----------- > target/m68k/cpu.h | 2 +- > target/m68k/helper.c | 55 +++++++++++++++-------- > target/m68k/monitor.c | 15 ++++++- > target/ppc/monitor.c | 6 +++ > target/sh4/monitor.c | 5 +++ > target/sparc/cpu.h | 3 ++ > target/sparc/mmu_helper.c | 40 +++++++++++++---- > target/sparc/monitor.c | 23 ++++++++++ > target/xtensa/monitor.c | 6 +++ > 11 files changed, 206 insertions(+), 58 deletions(-) > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx > index 2695ff04ff..b836a8396e 100644 > --- a/hmp-commands-info.hx > +++ b/hmp-commands-info.hx > @@ -188,17 +188,26 @@ 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, sparc32 " > + "and m68k targets. use 'info tlb [start [end]]' " > + "to show a range of entries. Note that the range " > + "argument is only supported on i386, sparc32 and " > + "m68k targets.", The .help value is rather long. The help command prints a line {.name} {.params} -- {.help} per command. Here's one that's easy to read: object_add [qom-type=]type,id=str[,prop=value][,...] -- create QOM object Kind of bad: migrate [-d] [-r] [-c uri-cpr] uri -- migrate to URI (using -d to not wait for completion) -r to resume a paused postcopy migration -c to specify a CPR URI for cpr-transfer mode Worse: info tlb [start [end]] -- show virtual to physical memory mappings. output can be extremely large for i386, sparc32 and m68k targets. use 'info tlb [start [end]]' to show a range of entries. Note that the range argument is only supported on i386, sparc32 and m68k targets. Please consider limiting .help to the bare minimum. If the resulting help output is still wider than 80 columns, consider line breaks. > .cmd = hmp_info_tlb, > .arch_bitmask = QEMU_ARCH_I386 | QEMU_ARCH_SH4 | QEMU_ARCH_SPARC \ > | QEMU_ARCH_PPC | QEMU_ARCH_XTENSA | QEMU_ARCH_M68K, The command exists only for these architectures. > }, > > SRST > - ``info tlb`` > + ``info tlb`` [*start* [*end*]] > Show virtual to physical memory mappings. > + The output can be extremely large for i386, sparc32 and m68k targets. Such lists tend to bit-rot. Is the sentence useful here? It is useful in the commit message, where it justifies the feature. What about x86_64 and sparc64? .arch_bitmask is about architectures, not targets. Are you confusing the two? > + Use *start* and *end* to print entries located in virtual address > + range [start, end] (end is optional). Note that the range argument is "(end is optional)" is redundant with the square brackets above. It is also misleading: start is also optional. Recommend to drop it. > + only supported on i386, sparc32 and m68k targets. What about x86_64 and sparc64? How hard would it be to implement the address range feature for all architectures? > ERST > > { > diff --git a/target/i386/monitor.c b/target/i386/monitor.c > index ce61132e0f..d59e67647a 100644 > --- a/target/i386/monitor.c > +++ b/target/i386/monitor.c > @@ -50,11 +50,9 @@ static hwaddr addr_canonical(CPUArchState *env, hwaddr > addr) > return addr; > } > > -static void print_pte(MonitorHMP *hmp, CPUArchState *env, hwaddr addr, > - hwaddr pte, hwaddr mask) > +static void do_print_pte(MonitorHMP *hmp, hwaddr addr, > + hwaddr pte, hwaddr mask) > { > - addr = addr_canonical(env, addr); > - > monitor_hmp_printf(hmp, HWADDR_FMT_plx ": " HWADDR_FMT_plx > " %c%c%c%c%c%c%c%c%c\n", > addr, > @@ -70,7 +68,25 @@ static void print_pte(MonitorHMP *hmp, CPUArchState *env, > hwaddr addr, > pte & PG_RW_MASK ? 'W' : '-'); > } > > -static void tlb_info_32(MonitorHMP *hmp, CPUArchState *env, AddressSpace *as) > +static void print_pte(MonitorHMP *hmp, CPUArchState *env, hwaddr addr, > + hwaddr pte, hwaddr mask, hwaddr size, > + hwaddr start, hwaddr end) > +{ > + hwaddr addr_start = addr_canonical(env, addr); > + hwaddr addr_end = addr_canonical(env, addr + size - 1); > + /* > + * Print current page [addr_start, addr_end] only if it overlaps the > + * requested virtual address range [start, end]. > + */ > + if (addr_start > end || addr_end < start) { > + return; > + } > + > + do_print_pte(hmp, addr_start, pte, mask); > +} > + > +static void tlb_info_32(MonitorHMP *hmp, CPUArchState *env, AddressSpace *as, > + hwaddr start, hwaddr end) > { > const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > unsigned int l1, l2; > @@ -82,15 +98,16 @@ static void tlb_info_32(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as) > if (pde & PG_PRESENT_MASK) { > if ((pde & PG_PSE_MASK) && (env->cr[4] & CR4_PSE_MASK)) { > /* 4M pages */ > - print_pte(hmp, env, (l1 << 22), pde, ~((1 << 21) - 1)); > + print_pte(hmp, env, (l1 << 22), pde, ~((1 << 21) - 1), > + 0x400000, 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(hmp, env, (l1 << 22) + (l2 << 12), > - pte & ~PG_PSE_MASK, > - ~0xfff); > + pte & ~PG_PSE_MASK, ~0xfff, 0x1000, > + start, end); > } > } > } > @@ -98,7 +115,8 @@ static void tlb_info_32(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as) > } > } > > -static void tlb_info_pae32(MonitorHMP *hmp, CPUArchState *env, AddressSpace > *as) > +static void tlb_info_pae32(MonitorHMP *hmp, CPUArchState *env, AddressSpace > *as, > + hwaddr start, hwaddr end) > { > const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > unsigned int l1, l2, l3; > @@ -116,17 +134,19 @@ static void tlb_info_pae32(MonitorHMP *hmp, > CPUArchState *env, AddressSpace *as) > if (pde & PG_PSE_MASK) { > /* 2M pages with PAE, CR4.PSE is ignored */ > print_pte(hmp, env, (l1 << 30) + (l2 << 21), pde, > - ~((hwaddr)(1 << 20) - 1)); > + ~((hwaddr)(1 << 20) - 1), 0x200000, > + 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(hmp, env, (l1 << 30) + (l2 << 21) > - + (l3 << 12), > + print_pte(hmp, env, > + (l1 << 30) + (l2 << 21) + (l3 << > 12), > pte & ~PG_PSE_MASK, > - ~(hwaddr)0xfff); > + ~(hwaddr)0xfff, 0x1000, > + start, end); > } > } > } > @@ -138,7 +158,8 @@ static void tlb_info_pae32(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as) > > #ifdef TARGET_X86_64 > static void tlb_info_la48(MonitorHMP *hmp, 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; > @@ -161,7 +182,7 @@ static void tlb_info_la48(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as, > if (pdpe & PG_PSE_MASK) { > /* 1G pages, CR4.PSE is ignored */ > print_pte(hmp, env, (l0 << 48) + (l1 << 39) + (l2 << 30), > - pdpe, 0x3ffffc0000000ULL); > + pdpe, 0x3ffffc0000000ULL, 0x40000000, start, end); > continue; > } > > @@ -174,8 +195,9 @@ static void tlb_info_la48(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as, > > if (pde & PG_PSE_MASK) { > /* 2M pages, CR4.PSE is ignored */ > - print_pte(hmp, env, (l0 << 48) + (l1 << 39) + (l2 << 30) > + > - (l3 << 21), pde, 0x3ffffffe00000ULL); > + print_pte(hmp, env, > + (l0 << 48) + (l1 << 39) + (l2 << 30) + (l3 << > 21), > + pde, 0x3ffffffe00000ULL, 0x200000, start, end); > continue; > } > > @@ -184,9 +206,11 @@ static void tlb_info_la48(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as, > pte = address_space_ldq_le(as, pt_addr + l4 * 8, > attrs, NULL); > if (pte & PG_PRESENT_MASK) { > - print_pte(hmp, env, (l0 << 48) + (l1 << 39) + > - (l2 << 30) + (l3 << 21) + (l4 << 12), > - pte & ~PG_PSE_MASK, 0x3fffffffff000ULL); > + print_pte(hmp, env, > + (l0 << 48) + (l1 << 39) + (l2 << 30) + > + (l3 << 21) + (l4 << 12), > + pte & ~PG_PSE_MASK, > + 0x3fffffffff000ULL, 0x1000, start, end); > } > } > } > @@ -194,7 +218,8 @@ static void tlb_info_la48(MonitorHMP *hmp, CPUArchState > *env, AddressSpace *as, > } > } > > -static void tlb_info_la57(MonitorHMP *hmp, CPUArchState *env, AddressSpace > *as) > +static void tlb_info_la57(MonitorHMP *hmp, CPUArchState *env, AddressSpace > *as, > + hwaddr start, hwaddr end) > { > const MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED; > uint64_t l0; > @@ -205,7 +230,8 @@ static void tlb_info_la57(MonitorHMP *hmp, 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(hmp, env, as, l0, pml5e & 0x3fffffffff000ULL); > + tlb_info_la48(hmp, env, as, l0, pml5e & 0x3fffffffff000ULL, > + start, end); > } > } > } > @@ -215,6 +241,18 @@ void hmp_info_tlb(MonitorHMP *hmp, 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"); > + } > + if (start > end) { > + monitor_hmp_printf(hmp, "Invalid address range: start > end.\n"); Scratch the period, please. Same for all the other error messages. > + return; > + } > > env = monitor_hmp_get_cpu_env(hmp); > if (!env) { > @@ -231,17 +269,19 @@ void hmp_info_tlb(MonitorHMP *hmp, const QDict *qdict) > #ifdef TARGET_X86_64 > if (env->hflags & HF_LMA_MASK) { > if (env->cr[4] & CR4_LA57_MASK) { > - tlb_info_la57(hmp, env, as); > + tlb_info_la57(hmp, env, as, start, end); > } else { > - tlb_info_la48(hmp, env, as, 0, env->cr[3] & > 0x3fffffffff000ULL); > + tlb_info_la48(hmp, env, as, 0, > + env->cr[3] & 0x3fffffffff000ULL, > + start, end); > } > } else > #endif > { > - tlb_info_pae32(hmp, env, as); > + tlb_info_pae32(hmp, env, as, start, end); > } > } else { > - tlb_info_32(hmp, env, as); > + tlb_info_32(hmp, env, as, start, end); > } > } > > diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h > index 7cf3791108..b129cb150f 100644 > --- a/target/m68k/cpu.h > +++ b/target/m68k/cpu.h > @@ -606,6 +606,6 @@ void m68k_cpu_transaction_failed(CPUState *cs, hwaddr > physaddr, vaddr addr, > #define TB_FLAGS_TRACE 16 > #define TB_FLAGS_TRACE_BIT (1 << TB_FLAGS_TRACE) > > -void dump_mmu(CPUM68KState *env); > +void dump_mmu(CPUM68KState *env, hwaddr start, hwaddr end); > > #endif > diff --git a/target/m68k/helper.c b/target/m68k/helper.c > index 5f91d206f5..1b854c912b 100644 > --- a/target/m68k/helper.c > +++ b/target/m68k/helper.c > @@ -461,27 +461,46 @@ void m68k_switch_sp(CPUM68KState *env) > /* MMU: 68040 only */ > > static void print_address_zone(uint32_t logical, uint32_t physical, > - uint32_t size, int attr) > + uint32_t size, int attr, > + hwaddr start, hwaddr end) > { > + uint64_t zone_start = logical; > + uint64_t zone_end = zone_start + size - 1; > + uint64_t zone_len; > + > + /* > + * Print current zone [zone_start, zone_end] only if it overlaps the > + * requested virtual address range [start, end]. > + */ > + if (zone_end < start || zone_start > end) { > + return; > + } > + > + physical += (uint32_t)(start > zone_start ? start - zone_start : 0); > + zone_start = zone_start > start ? zone_start : start; > + zone_end = zone_end < end ? zone_end : end; > + zone_len = zone_end - zone_start + 1; > + > qemu_printf("%08x - %08x -> %08x - %08x %c ", > - logical, logical + size - 1, > - physical, physical + size - 1, > + (uint32_t)zone_start, (uint32_t)zone_end, > + physical, physical + (uint32_t)(zone_len - 1), > attr & 4 ? 'W' : '-'); > - size >>= 10; > - if (size < 1024) { > - qemu_printf("(%d KiB)\n", size); > + zone_len >>= 10; > + if (zone_len < 1024) { > + qemu_printf("(%d KiB)\n", (int)zone_len); > } else { > - size >>= 10; > - if (size < 1024) { > - qemu_printf("(%d MiB)\n", size); > + zone_len >>= 10; > + if (zone_len < 1024) { > + qemu_printf("(%d MiB)\n", (int)zone_len); > } else { > - size >>= 10; > - qemu_printf("(%d GiB)\n", size); > + zone_len >>= 10; > + qemu_printf("(%d GiB)\n", (int)zone_len); > } > } > } > > -static void dump_address_map(CPUM68KState *env, uint32_t root_pointer) > +static void dump_address_map(CPUM68KState *env, uint32_t root_pointer, > + hwaddr start, hwaddr end) > { > int tic_size, tic_shift; > uint32_t tib_mask; > @@ -550,7 +569,8 @@ static void dump_address_map(CPUM68KState *env, uint32_t > root_pointer) > size = last_logical + (1 << tic_shift) - > first_logical; > print_address_zone(first_logical, > - first_physical, size, last_attr); > + first_physical, size, last_attr, > + start, end); > } > first_logical = logical; > first_physical = physical; > @@ -560,7 +580,8 @@ static void dump_address_map(CPUM68KState *env, uint32_t > root_pointer) > } > if (first_logical != logical || (attr & 4) != (last_attr & 4)) { > size = logical + (1 << tic_shift) - first_logical; > - print_address_zone(first_logical, first_physical, size, last_attr); > + print_address_zone(first_logical, first_physical, size, last_attr, > + start, end); > } > } > > @@ -610,7 +631,7 @@ static void dump_ttr(uint32_t ttr) > M68K_DESC_USERATTR_SHIFT); > } > > -void dump_mmu(CPUM68KState *env) > +void dump_mmu(CPUM68KState *env, hwaddr start, hwaddr end) > { > if ((env->mmu.tcr & M68K_TCR_ENABLED) == 0) { > qemu_printf("Translation disabled\n"); > @@ -675,10 +696,10 @@ void dump_mmu(CPUM68KState *env) > dump_ttr(env->mmu.ttr[M68K_DTTR1]); > > qemu_printf("SRP: 0x%08x\n", env->mmu.srp); > - dump_address_map(env, env->mmu.srp); > + dump_address_map(env, env->mmu.srp, start, end); > > qemu_printf("URP: 0x%08x\n", env->mmu.urp); > - dump_address_map(env, env->mmu.urp); > + dump_address_map(env, env->mmu.urp, start, end); > } > > static int check_TTR(uint32_t ttr, int *prot, target_ulong addr, > diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c > index e8b1b25a43..879a005342 100644 > --- a/target/m68k/monitor.c > +++ b/target/m68k/monitor.c > @@ -9,17 +9,30 @@ > #include "cpu.h" > #include "monitor/hmp.h" > #include "monitor/monitor.h" > +#include "qobject/qdict.h" > > #ifdef CONFIG_HMP > void hmp_info_tlb(MonitorHMP *hmp, const QDict *qdict) > { > CPUArchState *env1 = monitor_hmp_get_cpu_env(hmp); > + hwaddr start = 0, end = HWADDR_MAX; > > if (!env1) { > monitor_hmp_printf(hmp, "No CPU available\n"); > return; > } > > - dump_mmu(env1); > + if (qdict_haskey(qdict, "start")) { > + start = (hwaddr)qdict_get_int(qdict, "start"); > + } > + if (qdict_haskey(qdict, "end")) { > + end = (hwaddr)qdict_get_int(qdict, "end"); > + } > + if (start > end) { > + monitor_hmp_printf(hmp, "Invalid address range: start > end.\n"); > + return; > + } > + > + dump_mmu(env1, start, end); > } > #endif > diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c > index 5a21c01593..0409e38481 100644 > --- a/target/ppc/monitor.c > +++ b/target/ppc/monitor.c > @@ -10,6 +10,7 @@ > #include "monitor/monitor.h" > #include "monitor/hmp.h" > #include "cpu.h" > +#include "qobject/qdict.h" > > #ifdef CONFIG_HMP > void hmp_info_tlb(MonitorHMP *hmp, const QDict *qdict) > @@ -20,6 +21,11 @@ void hmp_info_tlb(MonitorHMP *hmp, const QDict *qdict) > monitor_hmp_printf(hmp, "No CPU available\n"); > return; > } > + > + if (qdict_haskey(qdict, "start") || qdict_haskey(qdict, "end")) { > + monitor_hmp_printf(hmp, "The range arguments will be ignored.\n"); > + } When the user requests something the program cannot do, it should fail the request, not change it to something it can do. Make this an error, please. Same for the other architectures that don't support the feature. > + > dump_mmu(env1); > } > #endif [...]
