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(-)
'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
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 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org
+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.
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
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
* 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 |_______/
© 2016 - 2024 Red Hat, Inc.