Add commands that are like the other "md" commands but that allow you
to read memory that's in the IO space.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
kernel/debug/kdb/kdb_main.c | 38 +++++++++++++++++++++++----
kernel/debug/kdb/kdb_private.h | 1 +
kernel/debug/kdb/kdb_support.c | 48 ++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 5 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index be72657741a5..a90d1e1482c2 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1526,7 +1526,7 @@ static int kdb_mdr(int argc, const char **argv)
*/
static void kdb_md_line(const char *fmtstr, unsigned long addr,
bool symbolic, bool nosect, int bytesperword,
- int num, int repeat, bool phys)
+ int num, int repeat, bool phys, bool do_iomap)
{
/* print just one line of data */
kdb_symtab_t symtab;
@@ -1543,7 +1543,10 @@ static void kdb_md_line(const char *fmtstr, unsigned long addr,
kdb_printf(kdb_machreg_fmt0 " ", addr);
for (i = 0; i < num && repeat--; i++) {
- if (phys) {
+ if (do_iomap) {
+ if (kdb_getioword(&word, addr, bytesperword))
+ break;
+ } else if (phys) {
if (kdb_getphysword(&word, addr, bytesperword))
break;
} else if (kdb_getword(&word, addr, bytesperword))
@@ -1646,6 +1649,7 @@ static int kdb_md(int argc, const char **argv)
bool symbolic = false;
bool valid = false;
bool phys = false;
+ bool do_iomap = false;
kdbgetintenv("MDCOUNT", &mdcount);
kdbgetintenv("RADIX", &radix);
@@ -1655,6 +1659,8 @@ static int kdb_md(int argc, const char **argv)
valid = true;
else if (kdb_md_parse_arg0("mdp", argv[0], &repeat, &bytesperword))
phys = valid = true;
+ else if (kdb_md_parse_arg0("mdi", argv[0], &repeat, &bytesperword))
+ do_iomap = valid = true;
else if (strcmp(argv[0], "mds") == 0)
valid = true;
@@ -1765,7 +1771,11 @@ static int kdb_md(int argc, const char **argv)
if (KDB_FLAG(CMD_INTERRUPT))
return 0;
for (a = addr, z = 0; z < repeat; a += bytesperword, ++z) {
- if (phys) {
+ if (do_iomap) {
+ if (kdb_getioword(&word, a, bytesperword)
+ || word)
+ break;
+ } else if (phys) {
if (kdb_getphysword(&word, a, bytesperword)
|| word)
break;
@@ -1774,7 +1784,7 @@ static int kdb_md(int argc, const char **argv)
}
n = min(num, repeat);
kdb_md_line(fmtstr, addr, symbolic, nosect, bytesperword,
- num, repeat, phys);
+ num, repeat, phys, do_iomap);
addr += bytesperword * n;
repeat -= n;
z = (z + num - 1) / num;
@@ -2604,7 +2614,7 @@ static int kdb_per_cpu(int argc, const char **argv)
kdb_printf("%5d ", cpu);
kdb_md_line(fmtstr, addr,
bytesperword == KDB_WORD_SIZE,
- 1, bytesperword, 1, 1, 0);
+ true, bytesperword, 1, 1, false, false);
}
#undef KDB_PCU
return 0;
@@ -2717,6 +2727,24 @@ static kdbtab_t maintab[] = {
.help = "Display RAM given a PA using word size (W); show N words",
.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
},
+ { .name = "mdi",
+ .func = kdb_md,
+ .usage = "<paddr> <bytes>",
+ .help = "Display IO Memory",
+ .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
+ },
+ { .name = "mdiW",
+ .func = kdb_md,
+ .usage = "<paddr> <bytes>",
+ .help = "Display IO Memory using word size (W)",
+ .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
+ },
+ { .name = "mdiWcN",
+ .func = kdb_md,
+ .usage = "<paddr> <bytes>",
+ .help = "Display IO Memory using word size (W); show N words",
+ .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
+ },
{ .name = "mdr",
.func = kdb_mdr,
.usage = "<vaddr> <bytes>",
diff --git a/kernel/debug/kdb/kdb_private.h b/kernel/debug/kdb/kdb_private.h
index 1f685d9f16f9..caece6240140 100644
--- a/kernel/debug/kdb/kdb_private.h
+++ b/kernel/debug/kdb/kdb_private.h
@@ -97,6 +97,7 @@ extern int kdb_putarea_size(unsigned long, void *, size_t);
#define kdb_getarea(x, addr) kdb_getarea_size(&(x), addr, sizeof((x)))
#define kdb_putarea(addr, x) kdb_putarea_size(addr, &(x), sizeof((x)))
+extern int kdb_getioword(unsigned long *word, unsigned long addr, size_t size);
extern int kdb_getphysword(unsigned long *word,
unsigned long addr, size_t size);
extern int kdb_getword(unsigned long *, unsigned long, size_t);
diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 0a39497140bf..5a4e3a0e96a5 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -19,6 +19,7 @@
#include <linux/ptrace.h>
#include <linux/highmem.h>
#include <linux/hardirq.h>
+#include <linux/io.h>
#include <linux/delay.h>
#include <linux/uaccess.h>
#include <linux/kdb.h>
@@ -331,6 +332,53 @@ static int kdb_getphys(void *res, unsigned long addr, size_t size)
return 0;
}
+/*
+ * kdb_getioword
+ * Inputs:
+ * word Pointer to the word to receive the result.
+ * addr Address of the area to copy.
+ * size Size of the area.
+ * Returns:
+ * 0 for success, < 0 for error.
+ */
+int kdb_getioword(unsigned long *word, unsigned long addr, size_t size)
+{
+ void __iomem *mapped = ioremap(addr, size);
+ int diag = 0;
+
+ *word = 0; /* Default value if addr or size is invalid */
+
+ if (!mapped)
+ return KDB_BADADDR;
+
+ switch (size) {
+ case 1:
+ *word = readb(mapped);
+ break;
+ case 2:
+ *word = readw(mapped);
+ break;
+ case 4:
+ *word = readl(mapped);
+ break;
+ case 8:
+#ifdef CONFIG_64BIT
+ if (size <= sizeof(*word)) {
+ *word = readq(mapped);
+ break;
+ }
+#endif
+ fallthrough;
+ default:
+ kdb_func_printf("bad width %zu\n", size);
+ diag = KDB_BADWIDTH;
+ }
+
+ iounmap(mapped);
+
+ return diag;
+}
+
/*
* kdb_getphysword
* Inputs:
--
2.45.2.627.g7a2c4fd464-googOn Mon, Jun 17, 2024 at 05:34:47PM -0700, Douglas Anderson wrote:
> Add commands that are like the other "md" commands but that allow you
> to read memory that's in the IO space.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Sorry to be the bearer of bad news but...
> ---
> <snip>
> +/*
> + * kdb_getioword
> + * Inputs:
> + * word Pointer to the word to receive the result.
> + * addr Address of the area to copy.
> + * size Size of the area.
> + * Returns:
> + * 0 for success, < 0 for error.
> + */
> +int kdb_getioword(unsigned long *word, unsigned long addr, size_t size)
> +{
> + void __iomem *mapped = ioremap(addr, size);
ioremap() is a might_sleep() function. It's unsafe to call it from the
debug trap handler.
I'm afraid I don't know a safe alternative either. Machinary such as
kmap_atomic() needs a page and iomem won't have one.
Daniel.
Hi,
On Tue, Jun 18, 2024 at 8:59 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Mon, Jun 17, 2024 at 05:34:47PM -0700, Douglas Anderson wrote:
> > Add commands that are like the other "md" commands but that allow you
> > to read memory that's in the IO space.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
>
> Sorry to be the bearer of bad news but...
>
>
> > ---
> > <snip>
> > +/*
> > + * kdb_getioword
> > + * Inputs:
> > + * word Pointer to the word to receive the result.
> > + * addr Address of the area to copy.
> > + * size Size of the area.
> > + * Returns:
> > + * 0 for success, < 0 for error.
> > + */
> > +int kdb_getioword(unsigned long *word, unsigned long addr, size_t size)
> > +{
> > + void __iomem *mapped = ioremap(addr, size);
>
> ioremap() is a might_sleep() function. It's unsafe to call it from the
> debug trap handler.
Hmmmm. Do you have a pointer to documentation or code showing that
it's a might_sleep() function. I was worried about this initially but
I couldn't find any documentation or code indicating it to be so. I
also got no warnings when I ran my prototype code and then the code
worked fine, so I assumed that it must somehow work. Sigh...
Looking more closely, maybe this is:
ioremap() -> ioremap_prot() -> generic_ioremap_prot() ->
__get_vm_area_caller() -> __get_vm_area_node() -> __get_vm_area_node()
...and that has a kernel allocation with GFP_KERNEL?
I guess it also then calls alloc_vmap_area() which has might_sleep()...
I'll have to track down why no warnings triggered...
> I'm afraid I don't know a safe alternative either. Machinary such as
> kmap_atomic() needs a page and iomem won't have one.
Ugh. It would be nice to come up with something since it's not
uncommon to need to look at the state of hardware registers when a
crash happens. In the past I've managed to get into gdb, track down a
global variable where someone has already mapped the memory, and then
use gdb to look at the memory. It's always a big pain, though...
...even if I could just look up the virtual address where someone else
had already mapped it that might be enough. At least I wouldn't need
to go track down the globals myself...
...anyway, I guess I'll ponder on it and poke if I have time...
-Doug
On Tue, Jun 18, 2024 at 12:33:05PM -0700, Doug Anderson wrote:
> Hi,
>
> On Tue, Jun 18, 2024 at 8:59 AM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Mon, Jun 17, 2024 at 05:34:47PM -0700, Douglas Anderson wrote:
> > > Add commands that are like the other "md" commands but that allow you
> > > to read memory that's in the IO space.
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> >
> > Sorry to be the bearer of bad news but...
> >
> >
> > > ---
> > > <snip>
> > > +/*
> > > + * kdb_getioword
> > > + * Inputs:
> > > + * word Pointer to the word to receive the result.
> > > + * addr Address of the area to copy.
> > > + * size Size of the area.
> > > + * Returns:
> > > + * 0 for success, < 0 for error.
> > > + */
> > > +int kdb_getioword(unsigned long *word, unsigned long addr, size_t size)
> > > +{
> > > + void __iomem *mapped = ioremap(addr, size);
> >
> > ioremap() is a might_sleep() function. It's unsafe to call it from the
> > debug trap handler.
>
> Hmmmm. Do you have a pointer to documentation or code showing that
> it's a might_sleep() function. I was worried about this initially but
> I couldn't find any documentation or code indicating it to be so. I
> also got no warnings when I ran my prototype code and then the code
> worked fine, so I assumed that it must somehow work. Sigh...
>
> Looking more closely, maybe this is:
>
> ioremap() -> ioremap_prot() -> generic_ioremap_prot() ->
> __get_vm_area_caller() -> __get_vm_area_node() -> __get_vm_area_node()
>
> ...and that has a kernel allocation with GFP_KERNEL?
To be honest there were a lot of problems, I just simplified.
__get_vm_area_node() already commences with a BUG_ON(in_interrupt())
before it ends up doing a GFP_KERNEL memory allocation.
I think there are multiple calls to might_sleep() (for example from
__get_vm_area_node() -> alloc_vmap_area() ).
However even if we had preallocated some vmap addresses for peek/poke
there are still problems in ioremap_page_range() too. For example:
generic_ioremap_prot()
-> ioremap_page_range()
-> find_vm_area()
-> find_vmap_area()
-> spin_lock()
We could go further down the rabbit hole and pre-lookup the VM area too
but then we hit.
generic_ioremap_prot()
-> ioremap_page_range()
-> vmap_page_range()
-> vmap_range_noflush()
-> might_sleep()
It is remotely possible that the only lock vmap_page_range() takes is
init_mm->page_table_lock but I doubt we can be sure of that.
> I guess it also then calls alloc_vmap_area() which has might_sleep()...
>
> I'll have to track down why no warnings triggered...
>
> > I'm afraid I don't know a safe alternative either. Machinary such as
> > kmap_atomic() needs a page and iomem won't have one.
>
> Ugh. It would be nice to come up with something since it's not
> uncommon to need to look at the state of hardware registers when a
> crash happens. In the past I've managed to get into gdb, track down a
> global variable where someone has already mapped the memory, and then
> use gdb to look at the memory. It's always a big pain, though...
>
> ...even if I could just look up the virtual address where someone else
> had already mapped it that might be enough. At least I wouldn't need
> to go track down the globals myself...
>
> ...anyway, I guess I'll ponder on it and poke if I have time...
I've often thought about implementing longjmp-on-spin-wait for kgdb for
these kind of reasons. For example I have long wanted to be able to let
the user see /proc/interrupts before the usespace comes up but the spin
locks get in the way.
This approach wouldn't make calling ioremap() safe (since we could end
up bailing out halfway through a non-atomic operation) but it could at
least give control back to kdb and let the user know they have ruined
their system.
I know... there is a *reason* I've never quite got round to writing
this!
Daniel.
Hi, On Fri, Jun 21, 2024 at 8:43 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > For example I have long wanted to be able to let > the user see /proc/interrupts before the usespace comes up but the spin > locks get in the way. BTW the "gdb" scripts are supposed to handle this with "lx-interruptlist", though when I try it right now it doesn't seem to work. ...actually, there's also a script "lx-iomem" which sounds great but only has the physical addresses. :( -Doug
© 2016 - 2025 Red Hat, Inc.