* [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu'
@ 2024-03-20 16:40 Philippe Mathieu-Daudé
2024-03-20 16:40 ` [PATCH-for-9.0 1/2] target/monitor: Introduce 'info mmu' command Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-03-20 16:40 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Richard Henderson, Peter Maydell,
Dr. David Alan Gilbert, Manos Pitsidianakis, qemu-ppc,
Thomas Huth, devel, Paolo Bonzini, Philippe Mathieu-Daudé
'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
docs/about/deprecated.rst | 10 ++++++++
include/monitor/hmp-target.h | 3 +++
monitor/hmp-cmds-target.c | 49 ++++++++++++++++++++++++++++++++++++
target/i386/monitor.c | 4 +--
target/m68k/monitor.c | 2 +-
target/nios2/monitor.c | 2 +-
target/ppc/ppc-qmp-cmds.c | 2 +-
target/riscv/monitor.c | 2 +-
target/sh4/monitor.c | 2 +-
target/sparc/monitor.c | 2 +-
target/xtensa/monitor.c | 2 +-
hmp-commands-info.hx | 22 +++++++++++++---
12 files changed, 89 insertions(+), 13 deletions(-)
--
2.41.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH-for-9.0 1/2] target/monitor: Introduce 'info mmu' command 2024-03-20 16:40 [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Philippe Mathieu-Daudé @ 2024-03-20 16:40 ` Philippe Mathieu-Daudé 2024-03-20 16:40 ` [PATCH-for-9.0 2/2] target/monitor: Deprecate 'info tlb' and 'info mem' commands Philippe Mathieu-Daudé 2024-03-20 16:53 ` [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Peter Maydell 2 siblings, 0 replies; 7+ messages in thread From: Philippe Mathieu-Daudé @ 2024-03-20 16:40 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, Richard Henderson, Peter Maydell, Dr. David Alan Gilbert, Manos Pitsidianakis, qemu-ppc, Thomas Huth, devel, Paolo Bonzini, Philippe Mathieu-Daudé Introduce the 'info mmu' command. For now it only forward to the 'info tlb' and 'info mem' commands, which will be deprecated. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/monitor/hmp-target.h | 1 + monitor/hmp-cmds-target.c | 37 ++++++++++++++++++++++++++++++++++++ hmp-commands-info.hx | 14 ++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h index d78e979f05..2af84b1915 100644 --- a/include/monitor/hmp-target.h +++ b/include/monitor/hmp-target.h @@ -44,6 +44,7 @@ int target_get_monitor_def(CPUState *cs, const char *name, uint64_t *pval); CPUArchState *mon_get_cpu_env(Monitor *mon); CPUState *mon_get_cpu(Monitor *mon); +void hmp_info_mmu(Monitor *mon, const QDict *qdict); void hmp_info_mem(Monitor *mon, const QDict *qdict); void hmp_info_tlb(Monitor *mon, const QDict *qdict); void hmp_mce(Monitor *mon, const QDict *qdict); diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c index 9338ae8440..71bce4870a 100644 --- a/monitor/hmp-cmds-target.c +++ b/monitor/hmp-cmds-target.c @@ -31,6 +31,7 @@ #include "qapi/error.h" #include "qapi/qmp/qdict.h" #include "sysemu/hw_accel.h" +#include "sysemu/tcg.h" /* Set the current CPU defined by the user. Callers must hold BQL. */ int monitor_set_cpu(Monitor *mon, int cpu_index) @@ -379,3 +380,39 @@ void hmp_gpa2hpa(Monitor *mon, const QDict *qdict) memory_region_unref(mr); } #endif + +__attribute__((weak)) +void hmp_info_mem(Monitor *mon, const QDict *qdict) +{ + monitor_puts(mon, + "No per-CPU mapping information available on this target\n"); +} + +__attribute__((weak)) +void hmp_info_tlb(Monitor *mon, const QDict *qdict) +{ + monitor_puts(mon, + "No per-CPU TLB information available on this target\n"); +} + +void hmp_info_mmu(Monitor *mon, const QDict *qdict) +{ + bool tlb = qdict_get_try_bool(qdict, "tlb", false); + bool mem = qdict_get_try_bool(qdict, "mem", false); + + if (!tcg_enabled()) { + monitor_puts(mon, "This command is specific to TCG accelerator\n"); + return; + } + + if (!tlb && !mem) { + tlb = mem = true; + } + + if (mem) { + hmp_info_mem(mon, qdict); + } + if (tlb) { + hmp_info_tlb(mon, qdict); + } +} diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index ad1b1306e3..e31f2467fb 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -208,6 +208,20 @@ SRST Show PCI information. ERST + { + .name = "mmu", + .args_type = "tlb:-t,mem:-m", + .params = "[-t][-m]", + .help = "show virtual to physical memory " + "(-t: TLB; -m: active mapping)", + .cmd = hmp_info_mmu, + }, + +SRST + ``info mmu`` + Show virtual to physical memory mappings. +ERST + #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \ defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) { -- 2.41.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH-for-9.0 2/2] target/monitor: Deprecate 'info tlb' and 'info mem' commands 2024-03-20 16:40 [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Philippe Mathieu-Daudé 2024-03-20 16:40 ` [PATCH-for-9.0 1/2] target/monitor: Introduce 'info mmu' command Philippe Mathieu-Daudé @ 2024-03-20 16:40 ` Philippe Mathieu-Daudé 2024-03-20 16:53 ` [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Peter Maydell 2 siblings, 0 replies; 7+ messages in thread From: Philippe Mathieu-Daudé @ 2024-03-20 16:40 UTC (permalink / raw) To: qemu-devel Cc: qemu-riscv, Richard Henderson, Peter Maydell, Dr. David Alan Gilbert, Manos Pitsidianakis, qemu-ppc, Thomas Huth, devel, Paolo Bonzini, Philippe Mathieu-Daudé, Laurent Vivier, Chris Wulff, Marek Vasut, Nicholas Piggin, Daniel Henrique Barboza, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei, Yoshinori Sato, Mark Cave-Ayland, Artyom Tarasenko, Max Filippov 'info tlb' has been replaced by 'info mmu -t', and 'info mem' by 'info mmu -m'. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/about/deprecated.rst | 10 ++++++++++ include/monitor/hmp-target.h | 2 ++ monitor/hmp-cmds-target.c | 20 ++++++++++++++++---- target/i386/monitor.c | 4 ++-- target/m68k/monitor.c | 2 +- target/nios2/monitor.c | 2 +- target/ppc/ppc-qmp-cmds.c | 2 +- target/riscv/monitor.c | 2 +- target/sh4/monitor.c | 2 +- target/sparc/monitor.c | 2 +- target/xtensa/monitor.c | 2 +- hmp-commands-info.hx | 8 ++++---- 12 files changed, 41 insertions(+), 17 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b548519b5..4f5f4becbe 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -158,6 +158,16 @@ points was removed in 7.0. However QMP still exposed the vcpu parameter. This argument has now been deprecated and the remaining remaining trace points that used it are selected just by name. +Human Monitor Protocol (HMP) commands +------------------------------------- + +``info tlb`` and ``info mem`` (since 9.0) +''''''''''''''''''''''''''''''''''''''''' + +The ``info tlb`` and ``info mem`` commands have been replaced by +the ``info mmu`` command, which has the same behaviour but a less +misleading name. + Host Architectures ------------------ diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h index 2af84b1915..057f7c6841 100644 --- a/include/monitor/hmp-target.h +++ b/include/monitor/hmp-target.h @@ -46,7 +46,9 @@ CPUState *mon_get_cpu(Monitor *mon); void hmp_info_mmu(Monitor *mon, const QDict *qdict); void hmp_info_mem(Monitor *mon, const QDict *qdict); +void hmp_info_mem_deprecated(Monitor *mon, const QDict *qdict); void hmp_info_tlb(Monitor *mon, const QDict *qdict); +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict); void hmp_mce(Monitor *mon, const QDict *qdict); void hmp_info_local_apic(Monitor *mon, const QDict *qdict); void hmp_info_sev(Monitor *mon, const QDict *qdict); diff --git a/monitor/hmp-cmds-target.c b/monitor/hmp-cmds-target.c index 71bce4870a..086b58b8d6 100644 --- a/monitor/hmp-cmds-target.c +++ b/monitor/hmp-cmds-target.c @@ -382,19 +382,31 @@ void hmp_gpa2hpa(Monitor *mon, const QDict *qdict) #endif __attribute__((weak)) -void hmp_info_mem(Monitor *mon, const QDict *qdict) +void hmp_info_mem_deprecated(Monitor *mon, const QDict *qdict) { monitor_puts(mon, "No per-CPU mapping information available on this target\n"); } __attribute__((weak)) -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { monitor_puts(mon, "No per-CPU TLB information available on this target\n"); } +void hmp_info_mem(Monitor *mon, const QDict *qdict) +{ + monitor_puts(mon, "This command is deprecated, please use 'info mmu -m'\n"); + hmp_info_mem_deprecated(mon, qdict); +} + +void hmp_info_tlb(Monitor *mon, const QDict *qdict) +{ + monitor_puts(mon, "This command is deprecated, please use 'info mmu -t'\n"); + hmp_info_tlb_deprecated(mon, qdict); +} + void hmp_info_mmu(Monitor *mon, const QDict *qdict) { bool tlb = qdict_get_try_bool(qdict, "tlb", false); @@ -410,9 +422,9 @@ void hmp_info_mmu(Monitor *mon, const QDict *qdict) } if (mem) { - hmp_info_mem(mon, qdict); + hmp_info_mem_deprecated(mon, qdict); } if (tlb) { - hmp_info_tlb(mon, qdict); + hmp_info_tlb_deprecated(mon, qdict); } } diff --git a/target/i386/monitor.c b/target/i386/monitor.c index 3a281dab02..5da77b6b22 100644 --- a/target/i386/monitor.c +++ b/target/i386/monitor.c @@ -217,7 +217,7 @@ static void tlb_info_la57(Monitor *mon, CPUArchState *env) } #endif /* TARGET_X86_64 */ -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env; @@ -545,7 +545,7 @@ static void mem_info_la57(Monitor *mon, CPUArchState *env) } #endif /* TARGET_X86_64 */ -void hmp_info_mem(Monitor *mon, const QDict *qdict) +void hmp_info_mem_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env; diff --git a/target/m68k/monitor.c b/target/m68k/monitor.c index 2bdf6acae0..ea303805c4 100644 --- a/target/m68k/monitor.c +++ b/target/m68k/monitor.c @@ -10,7 +10,7 @@ #include "monitor/hmp-target.h" #include "monitor/monitor.h" -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env1 = mon_get_cpu_env(mon); diff --git a/target/nios2/monitor.c b/target/nios2/monitor.c index 0152dec3fa..2e4efee1aa 100644 --- a/target/nios2/monitor.c +++ b/target/nios2/monitor.c @@ -27,7 +27,7 @@ #include "monitor/hmp-target.h" #include "monitor/hmp.h" -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env1 = mon_get_cpu_env(mon); diff --git a/target/ppc/ppc-qmp-cmds.c b/target/ppc/ppc-qmp-cmds.c index a25d86a8d1..891fdc44ef 100644 --- a/target/ppc/ppc-qmp-cmds.c +++ b/target/ppc/ppc-qmp-cmds.c @@ -80,7 +80,7 @@ static target_long monitor_get_tbl(Monitor *mon, const struct MonitorDef *md, return cpu_ppc_load_tbl(env); } -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env1 = mon_get_cpu_env(mon); diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c index f5b1ffe6c3..00f2a22f11 100644 --- a/target/riscv/monitor.c +++ b/target/riscv/monitor.c @@ -208,7 +208,7 @@ static void mem_info_svxx(Monitor *mon, CPUArchState *env) last_paddr + last_size - pbase, last_attr); } -void hmp_info_mem(Monitor *mon, const QDict *qdict) +void hmp_info_mem_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env; diff --git a/target/sh4/monitor.c b/target/sh4/monitor.c index 2da6a5426e..ad6f21defe 100644 --- a/target/sh4/monitor.c +++ b/target/sh4/monitor.c @@ -39,7 +39,7 @@ static void print_tlb(Monitor *mon, int idx, tlb_t *tlb) tlb->d, tlb->wt); } -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env = mon_get_cpu_env(mon); int i; diff --git a/target/sparc/monitor.c b/target/sparc/monitor.c index 73f15aa272..95e6217568 100644 --- a/target/sparc/monitor.c +++ b/target/sparc/monitor.c @@ -28,7 +28,7 @@ #include "monitor/hmp.h" -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env1 = mon_get_cpu_env(mon); diff --git a/target/xtensa/monitor.c b/target/xtensa/monitor.c index fbf60d5553..92bfad8129 100644 --- a/target/xtensa/monitor.c +++ b/target/xtensa/monitor.c @@ -27,7 +27,7 @@ #include "monitor/hmp-target.h" #include "monitor/hmp.h" -void hmp_info_tlb(Monitor *mon, const QDict *qdict) +void hmp_info_tlb_deprecated(Monitor *mon, const QDict *qdict) { CPUArchState *env1 = mon_get_cpu_env(mon); diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index e31f2467fb..0066b49ceb 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -228,14 +228,14 @@ ERST .name = "tlb", .args_type = "", .params = "", - .help = "show virtual to physical memory mappings", + .help = "deprecated synonym for 'info mmu -t'", .cmd = hmp_info_tlb, }, #endif SRST ``info tlb`` - Show virtual to physical memory mappings. + This is a deprecated synonym for the mmu command. ERST #if defined(TARGET_I386) || defined(TARGET_RISCV) @@ -243,14 +243,14 @@ ERST .name = "mem", .args_type = "", .params = "", - .help = "show the active virtual memory mappings", + .help = "deprecated synonym for 'info mmu -m'", .cmd = hmp_info_mem, }, #endif SRST ``info mem`` - Show the active virtual memory mappings. + This is a deprecated synonym for the mmu command. ERST { -- 2.41.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' 2024-03-20 16:40 [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Philippe Mathieu-Daudé 2024-03-20 16:40 ` [PATCH-for-9.0 1/2] target/monitor: Introduce 'info mmu' command Philippe Mathieu-Daudé 2024-03-20 16:40 ` [PATCH-for-9.0 2/2] target/monitor: Deprecate 'info tlb' and 'info mem' commands Philippe Mathieu-Daudé @ 2024-03-20 16:53 ` Peter Maydell 2024-03-20 17:06 ` Philippe Mathieu-Daudé 2 siblings, 1 reply; 7+ messages in thread From: Peter Maydell @ 2024-03-20 16:53 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, qemu-riscv, Richard Henderson, Dr. David Alan Gilbert, Manos Pitsidianakis, qemu-ppc, Thomas Huth, devel, Paolo Bonzini On Wed, 20 Mar 2024 at 16:40, Philippe Mathieu-Daudé <philmd@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 * which targets implement which and why * what the plan is for the future 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. thanks -- PMM ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' 2024-03-20 16:53 ` [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Peter Maydell @ 2024-03-20 17:06 ` Philippe Mathieu-Daudé 2024-03-20 17:17 ` Peter Maydell 0 siblings, 1 reply; 7+ messages in thread From: Philippe Mathieu-Daudé @ 2024-03-20 17:06 UTC (permalink / raw) To: Peter Maydell Cc: qemu-devel, qemu-riscv, Richard Henderson, Dr. David Alan Gilbert, Manos Pitsidianakis, qemu-ppc, Thomas Huth, devel, Paolo Bonzini, Alex Bennée, Daniel P. Berrangé +Alex/Daniel On 20/3/24 17:53, Peter Maydell wrote: > On Wed, 20 Mar 2024 at 16:40, Philippe Mathieu-Daudé <philmd@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. > * 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. Luckily for me these are the only 2 implemented by more then one target: $ git grep TARGET_ -- hmp-commands* hmp-commands-info.hx:116:#if defined(TARGET_I386) hmp-commands-info.hx:225:#if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \ hmp-commands-info.hx:226: defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) hmp-commands-info.hx:241:#if defined(TARGET_I386) || defined(TARGET_RISCV) hmp-commands-info.hx:729:#if defined(TARGET_S390X) hmp-commands-info.hx:744:#if defined(TARGET_S390X) hmp-commands-info.hx:828:#if defined(TARGET_I386) hmp-commands-info.hx:882:#if defined(TARGET_I386) hmp-commands.hx:1126:#if defined(TARGET_S390X) hmp-commands.hx:1141:#if defined(TARGET_S390X) hmp-commands.hx:1489:#if defined(TARGET_I386) All the other ones are only implemented by a single target, so not a problem for now. 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"); + } +} > 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. Regards, Phil. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' 2024-03-20 17:06 ` Philippe Mathieu-Daudé @ 2024-03-20 17:17 ` Peter Maydell 2024-03-20 21:59 ` Dr. David Alan Gilbert 0 siblings, 1 reply; 7+ messages in thread From: Peter Maydell @ 2024-03-20 17:17 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, qemu-riscv, Richard Henderson, Dr. David Alan Gilbert, Manos Pitsidianakis, qemu-ppc, Thomas Huth, devel, Paolo Bonzini, Alex Bennée, Daniel P. Berrangé On Wed, 20 Mar 2024 at 17:06, Philippe Mathieu-Daudé <philmd@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@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' 2024-03-20 17:17 ` Peter Maydell @ 2024-03-20 21:59 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 7+ messages in thread From: Dr. David Alan Gilbert @ 2024-03-20 21:59 UTC (permalink / raw) To: Peter Maydell Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-riscv, Richard Henderson, Manos Pitsidianakis, qemu-ppc, Thomas Huth, devel, Paolo Bonzini, Alex Bennée, Daniel P. Berrangé * Peter Maydell (peter.maydell@linaro.org) wrote: > On Wed, 20 Mar 2024 at 17:06, Philippe Mathieu-Daudé <philmd@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@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. Right, so no rush to get the deprecation in; change it when you agree what you'd like a replacement to look like. Dave > -- PMM -- -----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 |_______/ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-03-20 22:00 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-03-20 16:40 [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Philippe Mathieu-Daudé 2024-03-20 16:40 ` [PATCH-for-9.0 1/2] target/monitor: Introduce 'info mmu' command Philippe Mathieu-Daudé 2024-03-20 16:40 ` [PATCH-for-9.0 2/2] target/monitor: Deprecate 'info tlb' and 'info mem' commands Philippe Mathieu-Daudé 2024-03-20 16:53 ` [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu' Peter Maydell 2024-03-20 17:06 ` Philippe Mathieu-Daudé 2024-03-20 17:17 ` Peter Maydell 2024-03-20 21:59 ` Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).