Re: [Qemu-devel] [PATCH] riscv: hmp: Add a command to show virtual memory mappings

[Bin Meng]

Hi Palmer,

On Wed, Aug 14, 2019 at 9:35 AM Bin Meng <address@hidden> wrote:
>
> Hi Palmer,
>
> On Tue, Aug 13, 2019 at 11:18 PM Palmer Dabbelt <address@hidden> wrote:
> >
> > On Wed, 31 Jul 2019 05:49:15 PDT (-0700), address@hidden wrote:
> > > This adds 'info mem' command for RISC-V, to show virtual memory
> > > mappings that aids debugging.
> > >
> > > Rather than showing every valid PTE, the command compacts the
> > > output by merging all contiguous physical address mappings into
> > > one block and only shows the merged block mapping details.
> > >
> > > Signed-off-by: Bin Meng <address@hidden>
> > > ---
> > >
> > >  hmp-commands-info.hx       |   2 +-
> > >  target/riscv/Makefile.objs |   4 +
> > >  target/riscv/monitor.c     | 227 
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 232 insertions(+), 1 deletion(-)
> > >  create mode 100644 target/riscv/monitor.c
> > >
> > > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > > index c59444c..257ee7d 100644
> > > --- a/hmp-commands-info.hx
> > > +++ b/hmp-commands-info.hx
> > > @@ -249,7 +249,7 @@ STEXI
> > >  Show virtual to physical memory mappings.
> > >  ETEXI
> > >
> > > -#if defined(TARGET_I386)
> > > +#if defined(TARGET_I386) || defined(TARGET_RISCV)
> > >      {
> > >          .name       = "mem",
> > >          .args_type  = "",
> > > diff --git a/target/riscv/Makefile.objs b/target/riscv/Makefile.objs
> > > index b1c79bc..a8ceccd 100644
> > > --- a/target/riscv/Makefile.objs
> > > +++ b/target/riscv/Makefile.objs
> > > @@ -1,5 +1,9 @@
> > >  obj-y += translate.o op_helper.o cpu_helper.o cpu.o csr.o fpu_helper.o 
> > > gdbstub.o pmp.o
> > >
> > > +ifeq ($(CONFIG_SOFTMMU),y)
> > > +obj-y += monitor.o
> > > +endif
> > > +
> > >  DECODETREE = $(SRC_PATH)/scripts/decodetree.py
> > >
> > >  decode32-y = $(SRC_PATH)/target/riscv/insn32.decode
> > > diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
> > > new file mode 100644
> > > index 0000000..30560ff
> > > --- /dev/null
> > > +++ b/target/riscv/monitor.c
> > > @@ -0,0 +1,227 @@
> > > +/*
> > > + * QEMU monitor for RISC-V
> > > + *
> > > + * Copyright (c) 2019 Bin Meng <address@hidden>
> > > + *
> > > + * RISC-V specific monitor commands implementation
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify 
> > > it
> > > + * under the terms and conditions of the GNU General Public License,
> > > + * version 2 or later, as published by the Free Software Foundation.
> > > + *
> > > + * This program is distributed in the hope it will be useful, but WITHOUT
> > > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > > for
> > > + * more details.
> > > + *
> > > + * You should have received a copy of the GNU General Public License 
> > > along with
> > > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "cpu.h"
> > > +#include "cpu_bits.h"
> > > +#include "monitor/monitor.h"
> > > +#include "monitor/hmp-target.h"
> > > +
> > > +#ifdef TARGET_RISCV64
> > > +#define PTE_HEADER_FIELDS       "vaddr            paddr            "\
> > > +                                "size             attr\n"
> > > +#define PTE_HEADER_DELIMITER    "---------------- ---------------- "\
> > > +                                "---------------- -------\n"
> > > +#else
> > > +#define PTE_HEADER_FIELDS       "vaddr    paddr            size     
> > > attr\n"
> > > +#define PTE_HEADER_DELIMITER    "-------- ---------------- -------- 
> > > -------\n"
> > > +#endif
> > > +
> > > +/* Perform linear address sign extension */
> > > +static target_ulong addr_canonical(int va_bits, target_ulong addr)
> > > +{
> > > +#ifdef TARGET_RISCV64
> > > +    if (addr & (1UL << (va_bits - 1))) {
> > > +        addr |= (hwaddr)-(1L << va_bits);
> > > +    }
> > > +#endif
> > > +
> > > +    return addr;
> > > +}
> > > +
> > > +static void print_pte_header(Monitor *mon)
> > > +{
> > > +    monitor_printf(mon, PTE_HEADER_FIELDS);
> > > +    monitor_printf(mon, PTE_HEADER_DELIMITER);
> > > +}
> > > +
> > > +static void print_pte(Monitor *mon, int va_bits, target_ulong vaddr,
> > > +                      hwaddr paddr, target_ulong size, int attr)
> > > +{
> > > +    /* santity check on vaddr */
> > > +    if (vaddr >= (1UL << va_bits)) {
> > > +        return;
> > > +    }
> > > +
> > > +    if (!size) {
> > > +        return;
> > > +    }
> > > +
> > > +    monitor_printf(mon, TARGET_FMT_lx " " TARGET_FMT_plx " " 
> > > TARGET_FMT_lx
> > > +                   " %c%c%c%c%c%c%c\n",
> > > +                   addr_canonical(va_bits, vaddr),
> > > +                   paddr, size,
> > > +                   attr & PTE_R ? 'r' : '-',
> > > +                   attr & PTE_W ? 'w' : '-',
> > > +                   attr & PTE_X ? 'x' : '-',
> > > +                   attr & PTE_U ? 'u' : '-',
> > > +                   attr & PTE_G ? 'g' : '-',
> > > +                   attr & PTE_A ? 'a' : '-',
> > > +                   attr & PTE_D ? 'd' : '-');
> > > +}
> > > +
> > > +static void walk_pte(Monitor *mon, hwaddr base, target_ulong start,
> > > +                     int level, int ptidxbits, int ptesize, int va_bits,
> > > +                     hwaddr *vbase, hwaddr *pbase, hwaddr *last_paddr,
> > > +                     target_ulong *last_size, int *last_attr)
> > > +{
> > > +    hwaddr pte_addr;
> > > +    hwaddr paddr;
> > > +    target_ulong pgsize;
> > > +    target_ulong pte;
> > > +    int ptshift;
> > > +    int attr;
> > > +    int idx;
> > > +
> > > +    if (level < 0) {
> > > +        return;
> > > +    }
> > > +
> > > +    ptshift = level * ptidxbits;
> > > +    pgsize = 1UL << (PGSHIFT + ptshift);
> > > +
> > > +    for (idx = 0; idx < (1UL << ptidxbits); idx++) {
> > > +        pte_addr = base + idx * ptesize;
> > > +        cpu_physical_memory_read(pte_addr, &pte, ptesize);
> > > +
> > > +        paddr = (pte >> PTE_PPN_SHIFT) << PGSHIFT;
> > > +        attr = pte & 0xff;
> > > +
> > > +        /* PTE has to be valid */
> > > +        if (attr & PTE_V) {
> > > +            if (attr & (PTE_R | PTE_W | PTE_X)) {
> > > +                /*
> > > +                 * A leaf PTE has been found
> > > +                 *
> > > +                 * If current PTE's permission bits differ from the last 
> > > one,
> > > +                 * or current PTE's ppn does not make a contiguous 
> > > physical
> > > +                 * address block together with the last one, print out 
> > > the last
> > > +                 * contiguous mapped block details.
> > > +                 */
> > > +                if ((*last_attr != attr) ||
> > > +                    (*last_paddr + *last_size != paddr)) {
> > > +                    print_pte(mon, va_bits, *vbase, *pbase,
> > > +                              *last_paddr + *last_size - *pbase, 
> > > *last_attr);
> > > +
> > > +                    *vbase = start;
> > > +                    *pbase = paddr;
> > > +                    *last_attr = attr;
> > > +                }
> > > +
> > > +                *last_paddr = paddr;
> > > +                *last_size = pgsize;
> > > +            } else {
> > > +                /* pointer to the next level of the page table */
> > > +                walk_pte(mon, paddr, start, level - 1, ptidxbits, 
> > > ptesize,
> > > +                         va_bits, vbase, pbase, last_paddr,
> > > +                         last_size, last_attr);
> > > +            }
> > > +        }
> > > +
> > > +        start += pgsize;
> > > +    }
> > > +
> > > +}
> > > +
> > > +static void mem_info_svxx(Monitor *mon, CPUArchState *env)
> > > +{
> > > +    hwaddr base;
> > > +    int levels, ptidxbits, ptesize, vm, va_bits;
> > > +    hwaddr vbase, pbase, last_paddr;
> > > +    target_ulong last_size;
> > > +    int last_attr;
> > > +
> > > +    base = get_field(env->satp, SATP_PPN) << PGSHIFT;
> > > +
> > > +    vm = get_field(env->satp, SATP_MODE);
> > > +    switch (vm) {
> > > +    case VM_1_10_SV32:
> > > +        levels = 2;
> > > +        ptidxbits = 10;
> > > +        ptesize = 4;
> > > +        break;
> > > +    case VM_1_10_SV39:
> > > +        levels = 3;
> > > +        ptidxbits = 9;
> > > +        ptesize = 8;
> > > +        break;
> > > +    case VM_1_10_SV48:
> > > +        levels = 4;
> > > +        ptidxbits = 9;
> > > +        ptesize = 8;
> > > +        break;
> > > +    case VM_1_10_SV57:
> > > +        levels = 5;
> > > +        ptidxbits = 9;
> > > +        ptesize = 8;
> > > +        break;
> > > +    default:
> > > +        g_assert_not_reached();
> > > +        break;
> > > +    }
> > > +
> > > +    /* calculate virtual address bits */
> > > +    va_bits = PGSHIFT + levels * ptidxbits;
> > > +
> > > +    /* print header */
> > > +    print_pte_header(mon);
> > > +
> > > +    vbase = -1;
> > > +    pbase = -1;
> > > +    last_paddr = -1;
> > > +    last_size = 0;
> > > +    last_attr = 0;
> > > +
> > > +    /* walk page tables, starting from address 0 */
> > > +    walk_pte(mon, base, 0, levels - 1, ptidxbits, ptesize, va_bits,
> > > +             &vbase, &pbase, &last_paddr, &last_size, &last_attr);
> > > +
> > > +    /* don't forget the last one */
> > > +    print_pte(mon, va_bits, vbase, pbase,
> > > +              last_paddr + last_size - pbase, last_attr);
> > > +}
> > > +
> > > +void hmp_info_mem(Monitor *mon, const QDict *qdict)
> > > +{
> > > +    CPUArchState *env;
> > > +
> > > +    env = mon_get_cpu_env();
> > > +    if (!env) {
> > > +        monitor_printf(mon, "No CPU available\n");
> > > +        return;
> > > +    }
> > > +
> > > +    if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
> > > +        monitor_printf(mon, "S-mode MMU unavailable\n");
> > > +        return;
> > > +    }
> > > +
> > > +    if (env->priv_ver < PRIV_VERSION_1_10_0) {
> > > +        monitor_printf(mon, "Privileged mode < 1.10 unsupported\n");
> > > +        return;
> > > +    }
> > > +
> > > +    if (!(env->satp & SATP_MODE)) {
> > > +        monitor_printf(mon, "No translation or protection\n");
> > > +        return;
> > > +    }
> > > +
> > > +    mem_info_svxx(mon, env);
> > > +}
> >
> > Reviewed-by: Palmer Dabbelt <address@hidden>
> >
> > I'm putting this in the RISC-V tree with David's Acked-by, LMK if you want 
> > me
> > to do something else.
>
> Thanks, but please apply the v3 patch:
> http://patchwork.ozlabs.org/patch/1145744/

Sorry, please apply the v4 patch below which is essentially v2, as I
figured out that v3 is not necessary.
http://patchwork.ozlabs.org/patch/1147104/

Regards,
Bin