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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.