[src/trunk]: src/sys/arch Improve ddb disassembly for mips (and riscv, cribbe...

[dholland <dholland%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/caad638de893
branches:  trunk
changeset: 379255:caad638de893
user:      dholland <dholland%NetBSD.org@localhost>
date:      Sun May 23 23:22:55 2021 +0000

description:
Improve ddb disassembly for mips (and riscv, cribbed from mips).

- use db_read_bytes to get instructions
- move the address check logic previously attached only to fetching
  instructions for disassembly to db_read_bytes (and db_write_bytes)

Motivated by related x86 changes this afternoon.

Note that the address check logic is not as sophisticated as what the
x86 code does, but it's what we had before. (Except that riscv will
now also try to fetch usermode instructions instead of just failing.)

diffstat:

 sys/arch/mips/mips/db_disasm.c    |  29 ++++++++++-------------------
 sys/arch/mips/mips/db_interface.c |  33 +++++++++++++++++++++++++++++++--
 sys/arch/riscv/riscv/db_disasm.c  |  18 ++++++------------
 sys/arch/riscv/riscv/db_machdep.c |  32 ++++++++++++++++++++++++++++++--
 4 files changed, 77 insertions(+), 35 deletions(-)

diffs (253 lines):

diff -r fcbc4902ede3 -r caad638de893 sys/arch/mips/mips/db_disasm.c
--- a/sys/arch/mips/mips/db_disasm.c    Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/mips/mips/db_disasm.c    Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_disasm.c,v 1.42 2021/04/12 11:35:22 simonb Exp $    */
+/*     $NetBSD: db_disasm.c,v 1.43 2021/05/23 23:22:55 dholland Exp $  */
 
 /*-
  * Copyright (c) 1991, 1993
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.42 2021/04/12 11:35:22 simonb Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_disasm.c,v 1.43 2021/05/23 23:22:55 dholland Exp $");
 
 #include <sys/param.h>
 #include <sys/cpu.h>
@@ -47,6 +47,7 @@
 
 #include <machine/db_machdep.h>
 
+#include <ddb/db_access.h>
 #include <ddb/db_user.h>
 #include <ddb/db_interface.h>
 #include <ddb/db_output.h>
@@ -218,11 +219,6 @@ static void print_addr(db_addr_t);
  * "next instruction" does NOT mean the next instruction to
  * be executed but the 'linear' next instruction.
  */
-#ifdef _LP64
-#define        DISASM_KERN_START       MIPS_XKSEG_START
-#else
-#define        DISASM_KERN_START       MIPS_KSEG0_START
-#endif
 db_addr_t
 db_disasm(db_addr_t loc, bool altfmt)
 {
@@ -232,19 +228,14 @@ db_disasm(db_addr_t loc, bool altfmt)
         * Take some care with addresses to not UTLB here as it
         * loses the current debugging context.  KSEG2 and XKSEG
         * are not checked.
+        * Update: db_read_bytes is supposed to do that, and now
+        * does, so we can use that.
+        *
+        * XXX db_read_bytes_can't return failure but instead zeros
+        * the output. That's ok here, but if ever improved the
+        * proper thing here on error is to return the original loc.
         */
-       if (loc < (db_addr_t)DISASM_KERN_START) {
-#ifdef _KERNEL
-               if (ufetch_32((void *)loc, &instr) != 0) {
-                       db_printf("invalid address.\n");
-                       return loc;
-               }
-#else
-               return loc;
-#endif
-       } else {
-               instr =  *(uint32_t *)loc;
-       }
+       db_read_bytes(loc, sizeof(instr), (void *)&instr);
 
        return (db_disasm_insn(instr, loc, altfmt));
 }
diff -r fcbc4902ede3 -r caad638de893 sys/arch/mips/mips/db_interface.c
--- a/sys/arch/mips/mips/db_interface.c Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/mips/mips/db_interface.c Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_interface.c,v 1.93 2021/04/12 02:23:41 mrg Exp $    */
+/*     $NetBSD: db_interface.c,v 1.94 2021/05/23 23:22:55 dholland Exp $       */
 
 /*
  * Mach Operating System
@@ -27,7 +27,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: db_interface.c,v 1.93 2021/04/12 02:23:41 mrg Exp $");
+__KERNEL_RCSID(0, "$NetBSD: db_interface.c,v 1.94 2021/05/23 23:22:55 dholland Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_multiprocessor.h"
@@ -180,6 +180,23 @@ void
 db_read_bytes(vaddr_t addr, size_t size, char *data)
 {
        const char *src = (char *)addr;
+       int err;
+
+       /*
+        * If asked to fetch from userspace, do it safely.
+        * Note that MIPS_KSEG0_START is the proper address for
+        * both 32-bit and 64-bit kernels.
+        */
+       if (addr < (vaddr_t)MIPS_KSEG0_START) {
+               err = copyin(src, data, size);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", src);
+#endif
+                       memset(data, 0, size);
+               }
+               return;
+       }
 
        if (size <= 8 && (size & (size-1)) == 0 && (addr & (size-1)) == 0
            && ((uintptr_t)data & (size-1)) == 0) {
@@ -205,6 +222,18 @@ db_write_bytes(vaddr_t addr, size_t size
 {
        char *p = (char *)addr;
        size_t n = size;
+       int err;
+
+       /* If asked to fetch from userspace, do it safely */
+       if (addr < (vaddr_t)MIPS_KSEG0_START) {
+               err = copyout(data, p, size);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", p);
+#endif
+               }
+               return;
+       }
 
        if (size <= 8 && (size & (size-1)) == 0 && (addr & (size-1)) == 0
            && ((uintptr_t)data & (size-1)) == 0) {
diff -r fcbc4902ede3 -r caad638de893 sys/arch/riscv/riscv/db_disasm.c
--- a/sys/arch/riscv/riscv/db_disasm.c  Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/riscv/riscv/db_disasm.c  Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_disasm.c,v 1.7 2021/05/01 06:48:51 skrll Exp $      */
+/*     $NetBSD: db_disasm.c,v 1.8 2021/05/23 23:22:55 dholland Exp $   */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__RCSID("$NetBSD: db_disasm.c,v 1.7 2021/05/01 06:48:51 skrll Exp $");
+__RCSID("$NetBSD: db_disasm.c,v 1.8 2021/05/23 23:22:55 dholland Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -39,6 +39,7 @@
 #include <riscv/db_machdep.h>
 #include <riscv/insn.h>
 
+#include <ddb/db_access.h>
 #include <ddb/db_user.h>
 #include <ddb/db_interface.h>
 #include <ddb/db_output.h>
@@ -1463,23 +1464,16 @@ db_disasm(db_addr_t loc, bool altfmt)
        unsigned n, i;
        uint32_t insn32;
 
-#ifdef _KERNEL
-       if ((intptr_t) loc >= 0) {
-               db_printf("%s: %#"PRIxVADDR" is not a kernel address\n",
-                   __func__, loc);
-               return loc;
-       }
-#endif
-
        /*
         * Fetch the instruction. The first halfword tells us how many
         * more there are, and they're always in little-endian order.
         */
-       insn[0] = ((const uint16_t *)loc)[0];
+       db_read_bytes(loc, sizeof(insn[0]), (void *)&insn[0]);
        n = INSN_HALFWORDS(insn[0]);
        KASSERT(n > 0 && n <= 5);
        for (i = 1; i < n; i++) {
-               insn[i] = ((const uint16_t *)loc)[i];
+               db_read_bytes(loc + i * sizeof(insn[i]), sizeof(insn[i]),
+                             (void *)&insn[i]);
        }
 
        switch (n) {
diff -r fcbc4902ede3 -r caad638de893 sys/arch/riscv/riscv/db_machdep.c
--- a/sys/arch/riscv/riscv/db_machdep.c Sun May 23 21:12:28 2021 +0000
+++ b/sys/arch/riscv/riscv/db_machdep.c Sun May 23 23:22:55 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: db_machdep.c,v 1.7 2021/04/14 06:32:20 dholland Exp $  */
+/*     $NetBSD: db_machdep.c,v 1.8 2021/05/23 23:22:55 dholland Exp $  */
 
 /*-
  * Copyright (c) 2014 The NetBSD Foundation, Inc.
@@ -31,7 +31,7 @@
 
 #include <sys/cdefs.h>
 
-__RCSID("$NetBSD: db_machdep.c,v 1.7 2021/04/14 06:32:20 dholland Exp $");
+__RCSID("$NetBSD: db_machdep.c,v 1.8 2021/05/23 23:22:55 dholland Exp $");
 
 #include <sys/param.h>
 
@@ -42,6 +42,7 @@
 #include <ddb/db_interface.h>
 #include <ddb/db_extern.h>
 #include <ddb/db_variables.h>
+#include <ddb/db_output.h>
 
 int db_active = 0;
 
@@ -225,6 +226,19 @@ void
 db_read_bytes(db_addr_t addr, size_t len, char *data)
 {
        const char *src = (char *)addr;
+       int err;
+
+       /* If asked to fetch from userspace, do it safely */
+       if ((intptr_t)addr >= 0) {
+               err = copyin(src, data, len);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", src);
+#endif
+                       memset(data, 0, len);
+               }
+               return;
+       }
 
        while (len--) {
                *data++ = *src++;
@@ -237,6 +251,19 @@ db_read_bytes(db_addr_t addr, size_t len
 void
 db_write_bytes(vaddr_t addr, size_t len, const char *data)
 {
+       int err;
+
+       /* If asked to fetch from userspace, do it safely */
+       if ((intptr_t)addr >= 0) {
+               err = copyout(data, (char *)addr, len);
+               if (err) {
+#ifdef DDB
+                       db_printf("address %p is invalid\n", (char *)addr);
+#endif
+               }
+               return;
+       }
+
        if (len == 8) {
                *(uint64_t *)addr = *(const uint64_t *) data;
        } else if (len == 4) {
@@ -244,6 +271,7 @@ db_write_bytes(vaddr_t addr, size_t len,
        } else if (len == 2) {
                *(uint16_t *)addr = *(const uint16_t *) data;
        } else {
+               KASSERT(len == 1);
                *(uint8_t *)addr = *(const uint8_t *) data;
        }
        __asm("fence rw,rw; fence.i");