Re: [PATCH v2] mips/tlb_helper: Add support for 'info tlb' cmd

[Philippe Mathieu-Daudé]

Hi,

On 7/23/21 11:22 AM, Arkadiy wrote:
> From: NDNF <arkaisp2021@gmail.com>
> 
> This adds hmp 'info tlb' command support for the mips platform.
> 1k pages are not supported.
> 
> Signed-off-by: Ivanov Arkady <arkaisp2021@gmail.com>
> ---
>  hmp-commands-info.hx     |  3 ++-
>  target/mips/cpu.h        |  3 +++
>  target/mips/meson.build  |  1 +
>  target/mips/monitor.c    | 26 ++++++++++++++++++++++
>  target/mips/tlb_helper.c | 48 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 80 insertions(+), 1 deletion(-)
>  create mode 100644 target/mips/monitor.c
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index 117ba25f91..d6aab9839c 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -222,7 +222,8 @@ SRST
>  ERST
>  
>  #if defined(TARGET_I386) || defined(TARGET_SH4) || defined(TARGET_SPARC) || \
> -    defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K)
> +    defined(TARGET_PPC) || defined(TARGET_XTENSA) || defined(TARGET_M68K) || 
> \
> +    defined(TARGET_MIPS) || defined(TARGET_MIPS64)
>      {
>          .name       = "tlb",
>          .args_type  = "",
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index b9e227a30e..5aa6f2b760 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -1337,6 +1337,9 @@ void itc_reconfigure(struct MIPSITUState *tag);
>  /* helper.c */
>  target_ulong exception_resume_pc(CPUMIPSState *env);
>  
> +/*tlb_helper.c*/
> +void dump_mmu(CPUMIPSState *env);
> +
>  static inline void cpu_get_tb_cpu_state(CPUMIPSState *env, target_ulong *pc,
>                                          target_ulong *cs_base, uint32_t 
> *flags)
>  {
> diff --git a/target/mips/meson.build b/target/mips/meson.build
> index 9741545440..c0b19048ee 100644
> --- a/target/mips/meson.build
> +++ b/target/mips/meson.build
> @@ -31,6 +31,7 @@ mips_softmmu_ss.add(files(
>    'cp0_timer.c',
>    'machine.c',
>    'mips-semi.c',
> +  'monitor.c',
>  ))
>  mips_softmmu_ss.add(when: 'CONFIG_TCG', if_true: files(
>    'cp0_helper.c',
> diff --git a/target/mips/monitor.c b/target/mips/monitor.c
> new file mode 100644
> index 0000000000..9c9dfd2caa
> --- /dev/null
> +++ b/target/mips/monitor.c
> @@ -0,0 +1,26 @@
> +/*
> + * monitor.c
> + *
> + * Copyright (c) 2010-2021 Institute for System Programming
> + *                         of the Russian Academy of Sciences.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "monitor/monitor.h"
> +#include "monitor/hmp-target.h"
> +#include "monitor/hmp.h"
> +
> +void hmp_info_tlb(Monitor *mon, const QDict *qdict)
> +{
> +    CPUArchState *env = mon_get_cpu_env(mon);

I don't understand which core you are dumping.

Please dump all cores instead. Or take the core index as
argument. See hmp_mce() which does qdict_get_int(qdict, "cpu_index");
Maybe hmp_info_registers() is a good example, as it uses CPU_FOREACH().

> +    if (!env) {
> +        monitor_printf(mon, "No CPU available\n");
> +        return;
> +    }
> +    dump_mmu(env);
> +}
> diff --git a/target/mips/tlb_helper.c b/target/mips/tlb_helper.c
> index 082c17928d..1c0c2831d6 100644
> --- a/target/mips/tlb_helper.c
> +++ b/target/mips/tlb_helper.c
> @@ -24,6 +24,7 @@
>  #include "exec/cpu_ldst.h"
>  #include "exec/log.h"
>  #include "hw/mips/cpudevs.h"
> +#include "qemu/qemu-print.h"
>  
>  enum {
>      TLBRET_XI = -6,
> @@ -37,6 +38,53 @@ enum {
>  
>  #if !defined(CONFIG_USER_ONLY)
>  
> +static void r4k_mmu_dump(CPUMIPSState *env)
> +{
> +    int i;

Maybe display TLB count?

> +    for (i = 0; i < env->tlb->tlb_in_use; i++) {
> +        r4k_tlb_t *tlb = &env->tlb->mmu.r4k.tlb[i];
> +
> +        bool mi = !!((env->CP0_Config5 >> CP0C5_MI) & 1);
> +
> +        qemu_printf("TLB[%i]:\nG = %i EHINV = %i\nPageMask = %08x", i, 
> tlb->G,

Please prefix hexadecimal with 0x.

> +                    tlb->EHINV, tlb->PageMask);
> +        if (!tlb->EHINV) {
> +            if (mi) {
> +                qemu_printf(" MMID = %i", tlb->MMID);
> +            } else if (!tlb->G) {
> +                qemu_printf(" ASID = %i", tlb->ASID);
> +            }
> +            qemu_printf("\nVPN = "TARGET_FMT_lx" PFN[0] = %08lx RI0 = %i"
> +                        " XI0 = %i C0 = %i D0 = %i V0 = %i\n"
> +                        "VPN = "TARGET_FMT_lx" PFN[1] = %08lx RI1 = %i XI1 = 
> %i"
> +                        " C1 = %i D1 = %i V1 = %i\n", tlb->VPN, tlb->PFN[0],
> +                        tlb->RI0, tlb->XI0, tlb->C0, tlb->D0, tlb->V0, 
> tlb->VPN,
> +                        tlb->PFN[1], tlb->RI1, tlb->XI1,
> +                        tlb->C1, tlb->D1, tlb->V1);
> +        }
> +    }
> +}
> +
> +void dump_mmu(CPUMIPSState *env)
> +{
> +    switch (env->cpu_model->mmu_type) {
> +    case MMU_TYPE_NONE:
> +        qemu_printf("no MMU emulation\n");

Maybe "No TLB (no MMU)"

> +        break;
> +    case MMU_TYPE_R4000:
> +        r4k_mmu_dump(env);
> +        break;
> +    case MMU_TYPE_FMT:
> +        qemu_printf("fixed mapping MMU emulation\n");

and "No TLB (fixed mapping MMU)"?

> +        break;
> +    case MMU_TYPE_R3000:
> +    case MMU_TYPE_R6000:
> +    case MMU_TYPE_R8000:
> +    default:
> +        qemu_printf("MMU type not supported\n");

Simply g_assert_not_reached().

> +    }
> +}
> +
>  /* no MMU emulation */
>  int no_mmu_map_address(CPUMIPSState *env, hwaddr *physical, int *prot,
>                         target_ulong address, int rw, int access_type)
>