From: "Dr. David Alan Gilbert" <dave@treblig.org>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
qemu-devel@nongnu.org, qemu-riscv@nongnu.org,
"Richard Henderson" <richard.henderson@linaro.org>,
"Manos Pitsidianakis" <manos.pitsidianakis@linaro.org>,
qemu-ppc@nongnu.org, "Thomas Huth" <thuth@redhat.com>,
devel@lists.libvirt.org, "Paolo Bonzini" <pbonzini@redhat.com>,
"Alex Bennée" <alex.bennee@linaro.org>,
"Daniel P. Berrangé" <berrange@redhat.com>
Subject: Re: [PATCH-for-9.0 0/2] target/monitor: Deprecate 'info tlb/mem' in favor of 'info mmu'
Date: Wed, 20 Mar 2024 21:59:41 +0000 [thread overview]
Message-ID: <ZftcTb2_O8zHPd3Z@gallifrey> (raw)
In-Reply-To: <CAFEAcA_4J32gpUgxXxND8ZVhrEEhFK=gPG2SF8xsVJ+jpkV0WA@mail.gmail.com>
* 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 |_______/
prev parent reply other threads:[~2024-03-21 5:20 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZftcTb2_O8zHPd3Z@gallifrey \
--to=dave@treblig.org \
--cc=alex.bennee@linaro.org \
--cc=berrange@redhat.com \
--cc=devel@lists.libvirt.org \
--cc=manos.pitsidianakis@linaro.org \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 read-only IMAP folder(s) and NNTP newsgroup(s).