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> 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>
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