Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu'

On Wed, 20 Mar 2024 at 17:06, Philippe Mathieu-Daudé <philmd(a)linaro.org&gt; wrote: > > +Alex/Daniel > > On 20/3/24 17:53, Peter Maydell wrote: > > On Wed, 20 Mar 2024 at 16:40, Philippe Mathieu-Daudé <philmd(a)linaro.org&gt; wrote: > >> > >> 'info tlb' and 'info mem' commands don't scale in heterogeneous > >> emulation. They will be reworked after the next release, hidden > >> behind the 'info mmu' command. It is not too late to deprecate > >> commands, so add the 'info mmu' command as wrapper to the other > >> ones, but already deprecate them. > >> > >> Philippe Mathieu-Daudé (2): > >> target/monitor: Introduce 'info mmu' command > >> target/monitor: Deprecate 'info tlb' and 'info mem' commands > > > > This seems to replace "info tlb" and "info mem" with "info mmu -t" > > and "info mmu -m", but it doesn't really say anything about: > > * what the difference is between these two things > > I really don't know; I'm only trying to keep the monitor interface > identical. You don't, though: you change it from "info tlb" to "info mmu -t" etc. > > * which targets implement which and why > > This one is easy to answer: > > #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) > || \ > defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) > { > .name = "tlb", > > #if defined(TARGET_I386) || defined(TARGET_RISCV) > { > .name = "mem", > > > * what the plan is for the future > > My problem is with linking a single QEMU binary, as these two symbols > (hmp_info_mem and hmp_info_tlb) clash. Yes, but they both (implicitly) operate on the current HMP CPU, so the problem with linking into a single binary is that they're not indirected through a method on the CPU object, not the syntax used in the monitor to invoke them, presumably. > I'm indeed only postponing the problem, without looking at what > this code does. I did it adding hmp_info_mmu_tlb/mem hooks in > TCGCPUOps ("hw/core/tcg-cpu-ops.h"), so the command can be > dispatched per target vcpu as target-agnostic code in > monitor/hmp-cmds.c: > > +#include "hw/core/tcg-cpu-ops.h" > + > +static void hmp_info_mmu_tlb(Monitor *mon, CPUState *cpu) > +{ > + const TCGCPUOps *tcg_ops = cpu->cc->tcg_ops; > + > + if (tcg_ops->hmp_info_mmu_tlb) { > + tcg_ops->hmp_info_mmu_tlb(mon, cpu_env(cpu)); > + } else { > + monitor_puts(mon, "No per-CPU information available on this > target\n"); > + } > +} These aren't TCG specific though, so why TCGCPUOps ? > > I am definitely not a fan of either of these commands, because > > (as we currently implement them) they effectively require each > > target architecture to implement a second copy of the page table > > walking code. But before we can deprecate them we need to be > > pretty sure that "info mmu" is what we want to replace them with. > > An alternative is to just deprecate them, without adding "info mmu" :) > > It is OK to un-deprecate stuff if we realize its usefulness. The commands are there because some users find them useful. I just dislike them because I think they're a bit niche and annoying to implement and not consistent across target architectures and not very well documented... By the way, we have no obligation to follow the deprecate-and-drop process for HMP commands; unlike QMP, we give ourselves the license to vary it when we feel like it, because the users are humans, not programs or scripts.

-- PMM