From: "Dr. David Alan Gilbert" <dave@treblig.org>
To: Josh Junon <junon@oro.sh>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
armbru@redhat.com
Subject: Re: [PATCH] hmp: allow filtering `info tlb` entries by address on i386
Date: Tue, 1 Oct 2024 00:10:04 +0000 [thread overview]
Message-ID: <Zvs93PoHm0imx-r4@gallifrey> (raw)
In-Reply-To: <20240824083423.9332-1-junon@oro.sh>
* Josh Junon (junon@oro.sh) wrote:
> This change adds an optional virtual address parameter
> to the `info tlb` monitor command on i386 targets,
> only printing a specific entry if found.
Hi Josh,
> Signed-off-by: Josh Junon <junon@oro.sh>
> ---
> hmp-commands-info.hx | 5 +++++
> target/i386/monitor.c | 45 +++++++++++++++++++++++++++----------------
> 2 files changed, 33 insertions(+), 17 deletions(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index c59cd6637b..e42427b738 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -199,8 +199,13 @@ ERST
> defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
> {
> .name = "tlb",
> +#if defined(TARGET_I386)
> + .args_type = "virt:l?",
> + .params = "[virt]",
> +#else
> .args_type = "",
> .params = "",
> +#endif
I don't think we've got any other case where we only define a parameter
on some architecture; and it feels best to avoid adding it now.
> .help = "show virtual to physical memory mappings",
The meaning of parameters should be documented in the .help;
you could add a comment there saying 'only some architectures'
> .cmd = hmp_info_tlb,
> },
> diff --git a/target/i386/monitor.c b/target/i386/monitor.c
> index 2d766b2637..f54d400c97 100644
> --- a/target/i386/monitor.c
> +++ b/target/i386/monitor.c
> @@ -50,10 +50,14 @@ static hwaddr addr_canonical(CPUArchState *env, hwaddr addr)
> }
>
> static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> - hwaddr pte, hwaddr mask)
> + hwaddr pte, hwaddr mask, hwaddr *filter)
> {
> addr = addr_canonical(env, addr);
>
> + if (filter && (*filter & mask) != (addr & mask)) {
> + return;
> + }
> +
> monitor_printf(mon, HWADDR_FMT_plx ": " HWADDR_FMT_plx
> " %c%c%c%c%c%c%c%c%c\n",
> addr,
> @@ -69,7 +73,7 @@ static void print_pte(Monitor *mon, CPUArchState *env, hwaddr addr,
> pte & PG_RW_MASK ? 'W' : '-');
> }
>
> -static void tlb_info_32(Monitor *mon, CPUArchState *env)
> +static void tlb_info_32(Monitor *mon, CPUArchState *env, hwaddr *filter)
> {
> unsigned int l1, l2;
> uint32_t pgd, pde, pte;
> @@ -81,7 +85,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
> if (pde & PG_PRESENT_MASK) {
> if ((pde & PG_PSE_MASK) && (env->cr[4] & CR4_PSE_MASK)) {
> /* 4M pages */
> - print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1));
> + print_pte(mon, env, (l1 << 22), pde, ~((1 << 21) - 1), filter);
> } else {
> for(l2 = 0; l2 < 1024; l2++) {
> cpu_physical_memory_read((pde & ~0xfff) + l2 * 4, &pte, 4);
> @@ -89,7 +93,8 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
> if (pte & PG_PRESENT_MASK) {
> print_pte(mon, env, (l1 << 22) + (l2 << 12),
> pte & ~PG_PSE_MASK,
> - ~0xfff);
> + ~0xfff,
> + filter);
> }
> }
> }
> @@ -97,7 +102,7 @@ static void tlb_info_32(Monitor *mon, CPUArchState *env)
> }
> }
>
> -static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
> +static void tlb_info_pae32(Monitor *mon, CPUArchState *env, hwaddr *filter)
> {
> unsigned int l1, l2, l3;
> uint64_t pdpe, pde, pte;
> @@ -116,7 +121,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
> if (pde & PG_PSE_MASK) {
> /* 2M pages with PAE, CR4.PSE is ignored */
> print_pte(mon, env, (l1 << 30) + (l2 << 21), pde,
> - ~((hwaddr)(1 << 20) - 1));
> + ~((hwaddr)(1 << 20) - 1),
> + filter);
> } else {
> pt_addr = pde & 0x3fffffffff000ULL;
> for (l3 = 0; l3 < 512; l3++) {
> @@ -126,7 +132,8 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
> print_pte(mon, env, (l1 << 30) + (l2 << 21)
> + (l3 << 12),
> pte & ~PG_PSE_MASK,
> - ~(hwaddr)0xfff);
> + ~(hwaddr)0xfff,
> + filter);
> }
> }
> }
> @@ -138,7 +145,7 @@ static void tlb_info_pae32(Monitor *mon, CPUArchState *env)
>
> #ifdef TARGET_X86_64
> static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> - uint64_t l0, uint64_t pml4_addr)
> + uint64_t l0, uint64_t pml4_addr, hwaddr *filter)
> {
> uint64_t l1, l2, l3, l4;
> uint64_t pml4e, pdpe, pde, pte;
> @@ -162,7 +169,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> if (pdpe & PG_PSE_MASK) {
> /* 1G pages, CR4.PSE is ignored */
> print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30),
> - pdpe, 0x3ffffc0000000ULL);
> + pdpe, 0x3ffffc0000000ULL, filter);
> continue;
> }
>
> @@ -177,7 +184,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> if (pde & PG_PSE_MASK) {
> /* 2M pages, CR4.PSE is ignored */
> print_pte(mon, env, (l0 << 48) + (l1 << 39) + (l2 << 30) +
> - (l3 << 21), pde, 0x3ffffffe00000ULL);
> + (l3 << 21), pde, 0x3ffffffe00000ULL, filter);
> continue;
> }
>
> @@ -190,7 +197,8 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> if (pte & PG_PRESENT_MASK) {
> print_pte(mon, env, (l0 << 48) + (l1 << 39) +
> (l2 << 30) + (l3 << 21) + (l4 << 12),
> - pte & ~PG_PSE_MASK, 0x3fffffffff000ULL);
> + pte & ~PG_PSE_MASK, 0x3fffffffff000ULL,
> + filter);
> }
> }
> }
> @@ -198,7 +206,7 @@ static void tlb_info_la48(Monitor *mon, CPUArchState *env,
> }
> }
>
> -static void tlb_info_la57(Monitor *mon, CPUArchState *env)
> +static void tlb_info_la57(Monitor *mon, CPUArchState *env, hwaddr *filter)
> {
> uint64_t l0;
> uint64_t pml5e;
> @@ -209,7 +217,7 @@ static void tlb_info_la57(Monitor *mon, CPUArchState *env)
> cpu_physical_memory_read(pml5_addr + l0 * 8, &pml5e, 8);
> pml5e = le64_to_cpu(pml5e);
> if (pml5e & PG_PRESENT_MASK) {
> - tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL);
> + tlb_info_la48(mon, env, l0, pml5e & 0x3fffffffff000ULL, filter);
> }
> }
> }
> @@ -219,6 +227,9 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> {
> CPUArchState *env;
>
> + hwaddr filter_addr = qdict_get_try_int(qdict, "virt", 0);
> + hwaddr *filter = qdict_haskey(qdict, "virt") ? &filter_addr : NULL;
> +
I'm a bit confused about the format of the parameter you're trying to use;
can you add an example in the commit message please.
> env = mon_get_cpu_env(mon);
> if (!env) {
> monitor_printf(mon, "No CPU available\n");
> @@ -233,17 +244,17 @@ void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> #ifdef TARGET_X86_64
> if (env->hflags & HF_LMA_MASK) {
> if (env->cr[4] & CR4_LA57_MASK) {
> - tlb_info_la57(mon, env);
> + tlb_info_la57(mon, env, filter);
> } else {
> - tlb_info_la48(mon, env, 0, env->cr[3] & 0x3fffffffff000ULL);
> + tlb_info_la48(mon, env, 0, env->cr[3] & 0x3fffffffff000ULL, filter);
I think you're over line length there.
> }
> } else
> #endif
> {
> - tlb_info_pae32(mon, env);
> + tlb_info_pae32(mon, env, filter);
> }
> } else {
> - tlb_info_32(mon, env);
> + tlb_info_32(mon, env, filter);
> }
> }
>
> --
> 2.34.1
>
--
-----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-10-01 0:11 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-08-24 8:34 [PATCH] hmp: allow filtering `info tlb` entries by address on i386 Josh Junon
2024-10-01 0:10 ` 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=Zvs93PoHm0imx-r4@gallifrey \
--to=dave@treblig.org \
--cc=armbru@redhat.com \
--cc=junon@oro.sh \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/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 NNTP newsgroup(s).