The documentation for the variouis "md" commands was inconsistent
about documenting the command arguments. It was also hard to figure
out what the differences between the "phys", "raw", and "symbolic"
versions was.
Update the help strings to make things more obvious.
As part of this, add "bogus" commands to the table for "mdW" and
"mdWcN" so we don't have to obscurely reference them in the normal
"md" help. These bogus commands don't really hurt since kdb_md()
validates argv[0] enough.
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
kernel/debug/kdb/kdb_main.c | 39 ++++++++++++++++++++++---------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index cbeb203785b4..47e037c3c002 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1516,14 +1516,9 @@ static int kdb_mdr(unsigned long addr, unsigned int count)
}
/*
- * kdb_md - This function implements the 'md', 'md1', 'md2', 'md4',
- * 'md8' 'mdr' and 'mds' commands.
+ * kdb_md - This function implements the guts of the various 'md' commands.
*
- * md|mds [<addr arg> [<line count> [<radix>]]]
- * mdWcN [<addr arg> [<line count> [<radix>]]]
- * where W = is the width (1, 2, 4 or 8) and N is the count.
- * for eg., md1c20 reads 20 bytes, 1 at a time.
- * mdr <addr arg>,<byte count>
+ * See the kdb help for syntax.
*/
static void kdb_md_line(const char *fmtstr, unsigned long addr,
int symbolic, int nosect, int bytesperword,
@@ -2677,26 +2672,38 @@ EXPORT_SYMBOL_GPL(kdb_unregister);
static kdbtab_t maintab[] = {
{ .name = "md",
.func = kdb_md,
- .usage = "<vaddr>",
- .help = "Display Memory Contents, also mdWcN, e.g. md8c1",
+ .usage = "<vaddr> [<lines> [<radix>]]",
+ .help = "Display RAM using BYTESPERWORD; show MDCOUNT lines",
.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
},
- { .name = "mdr",
+ { .name = "mdW",
.func = kdb_md,
- .usage = "<vaddr> <bytes>",
- .help = "Display Raw Memory",
+ .usage = "<vaddr> [<lines> [<radix>]]",
+ .help = "Display RAM using word size (W) of 1, 2, 4, or 8",
+ .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
+ },
+ { .name = "mdWcN",
+ .func = kdb_md,
+ .usage = "<vaddr> [<lines> [<radix>]]",
+ .help = "Display RAM using word size (W); show N words",
.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
},
{ .name = "mdp",
.func = kdb_md,
- .usage = "<paddr> <bytes>",
- .help = "Display Physical Memory",
+ .usage = "<paddr> [<lines> [<radix>]]",
+ .help = "Display RAM given a physical address",
+ .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
+ },
+ { .name = "mdr",
+ .func = kdb_md,
+ .usage = "<vaddr> <bytes>",
+ .help = "Display RAM as a stream of raw bytes",
.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
},
{ .name = "mds",
.func = kdb_md,
- .usage = "<vaddr>",
- .help = "Display Memory Symbolically",
+ .usage = "<vaddr> [<lines>]",
+ .help = "Display RAM 1 native word/line; find words in kallsyms",
.flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
},
{ .name = "mm",
--
2.45.2.627.g7a2c4fd464-googOn Mon, Jun 17, 2024 at 05:34:36PM -0700, Douglas Anderson wrote:
> The documentation for the variouis "md" commands was inconsistent
> about documenting the command arguments. It was also hard to figure
> out what the differences between the "phys", "raw", and "symbolic"
> versions was.
>
> Update the help strings to make things more obvious.
>
> As part of this, add "bogus" commands to the table for "mdW" and
> "mdWcN" so we don't have to obscurely reference them in the normal
> "md" help. These bogus commands don't really hurt since kdb_md()
> validates argv[0] enough.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> kernel/debug/kdb/kdb_main.c | 39 ++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index cbeb203785b4..47e037c3c002 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1516,14 +1516,9 @@ static int kdb_mdr(unsigned long addr, unsigned int count)
> }
>
> /*
> - * kdb_md - This function implements the 'md', 'md1', 'md2', 'md4',
> - * 'md8' 'mdr' and 'mds' commands.
> + * kdb_md - This function implements the guts of the various 'md' commands.
> *
> - * md|mds [<addr arg> [<line count> [<radix>]]]
> - * mdWcN [<addr arg> [<line count> [<radix>]]]
> - * where W = is the width (1, 2, 4 or 8) and N is the count.
> - * for eg., md1c20 reads 20 bytes, 1 at a time.
> - * mdr <addr arg>,<byte count>
> + * See the kdb help for syntax.
> */
> static void kdb_md_line(const char *fmtstr, unsigned long addr,
> int symbolic, int nosect, int bytesperword,
> @@ -2677,26 +2672,38 @@ EXPORT_SYMBOL_GPL(kdb_unregister);
> static kdbtab_t maintab[] = {
> { .name = "md",
> .func = kdb_md,
> - .usage = "<vaddr>",
> - .help = "Display Memory Contents, also mdWcN, e.g. md8c1",
> + .usage = "<vaddr> [<lines> [<radix>]]",
> + .help = "Display RAM using BYTESPERWORD; show MDCOUNT lines",
I'd prefer "memory" over "RAM" because it's what the mnemonic is
abbreviating. This applies to all of the below but I won't be adding a
"same here" for all of them.
Where we have to crush something to fit into one line we'd than have to
break the pattern and choose from thing like:
1. Show memory
2. Display RAM
3. Display mem
Personally I prefer #1 but could probably cope with #2.
> .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> },
> - { .name = "mdr",
> + { .name = "mdW",
> .func = kdb_md,
> - .usage = "<vaddr> <bytes>",
> - .help = "Display Raw Memory",
> + .usage = "<vaddr> [<lines> [<radix>]]",
> + .help = "Display RAM using word size (W) of 1, 2, 4, or 8",
We need an "e.g. md8" in here somewhere. Otherwise it is not at all
obvious that W is a wildcard.
I guess that alternatively you could also try naming the command with
hint that W is a wild card (what happens if you register a command
called md<W>?).
> + .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> + },
> + { .name = "mdWcN",
> + .func = kdb_md,
> + .usage = "<vaddr> [<lines> [<radix>]]",
> + .help = "Display RAM using word size (W); show N words",
Same here.
> .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> },
> { .name = "mdp",
> .func = kdb_md,
> - .usage = "<paddr> <bytes>",
> - .help = "Display Physical Memory",
> + .usage = "<paddr> [<lines> [<radix>]]",
> + .help = "Display RAM given a physical address",
> + .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> + },
> + { .name = "mdr",
> + .func = kdb_md,
> + .usage = "<vaddr> <bytes>",
> + .help = "Display RAM as a stream of raw bytes",
> .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> },
> { .name = "mds",
> .func = kdb_md,
> - .usage = "<vaddr>",
> - .help = "Display Memory Symbolically",
> + .usage = "<vaddr> [<lines>]",
> + .help = "Display RAM 1 native word/line; find words in kallsyms",
> .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> },
> { .name = "mm",
> --
> 2.45.2.627.g7a2c4fd464-goog
>
Hi,
On Tue, Jun 18, 2024 at 4:24 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Mon, Jun 17, 2024 at 05:34:36PM -0700, Douglas Anderson wrote:
> > The documentation for the variouis "md" commands was inconsistent
> > about documenting the command arguments. It was also hard to figure
> > out what the differences between the "phys", "raw", and "symbolic"
> > versions was.
> >
> > Update the help strings to make things more obvious.
> >
> > As part of this, add "bogus" commands to the table for "mdW" and
> > "mdWcN" so we don't have to obscurely reference them in the normal
> > "md" help. These bogus commands don't really hurt since kdb_md()
> > validates argv[0] enough.
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> > kernel/debug/kdb/kdb_main.c | 39 ++++++++++++++++++++++---------------
> > 1 file changed, 23 insertions(+), 16 deletions(-)
> >
> > diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> > index cbeb203785b4..47e037c3c002 100644
> > --- a/kernel/debug/kdb/kdb_main.c
> > +++ b/kernel/debug/kdb/kdb_main.c
> > @@ -1516,14 +1516,9 @@ static int kdb_mdr(unsigned long addr, unsigned int count)
> > }
> >
> > /*
> > - * kdb_md - This function implements the 'md', 'md1', 'md2', 'md4',
> > - * 'md8' 'mdr' and 'mds' commands.
> > + * kdb_md - This function implements the guts of the various 'md' commands.
> > *
> > - * md|mds [<addr arg> [<line count> [<radix>]]]
> > - * mdWcN [<addr arg> [<line count> [<radix>]]]
> > - * where W = is the width (1, 2, 4 or 8) and N is the count.
> > - * for eg., md1c20 reads 20 bytes, 1 at a time.
> > - * mdr <addr arg>,<byte count>
> > + * See the kdb help for syntax.
> > */
> > static void kdb_md_line(const char *fmtstr, unsigned long addr,
> > int symbolic, int nosect, int bytesperword,
> > @@ -2677,26 +2672,38 @@ EXPORT_SYMBOL_GPL(kdb_unregister);
> > static kdbtab_t maintab[] = {
> > { .name = "md",
> > .func = kdb_md,
> > - .usage = "<vaddr>",
> > - .help = "Display Memory Contents, also mdWcN, e.g. md8c1",
> > + .usage = "<vaddr> [<lines> [<radix>]]",
> > + .help = "Display RAM using BYTESPERWORD; show MDCOUNT lines",
>
> I'd prefer "memory" over "RAM" because it's what the mnemonic is
> abbreviating. This applies to all of the below but I won't be adding a
> "same here" for all of them.
>
> Where we have to crush something to fit into one line we'd than have to
> break the pattern and choose from thing like:
>
> 1. Show memory
> 2. Display RAM
> 3. Display mem
>
> Personally I prefer #1 but could probably cope with #2.
I'm not dead set on RAM, but at least for me RAM was more clarifying.
Specifically when it said "memory" I always assumed it would take in
any memory address and when I first looked at this I tried to figure
out why memory addresses in the IO space didn't work with these
commands. I figured I was holding it wrong only to find out that the
commands specifically limit you to just the RAM range of memory
addresses.
That being said, I don't feel strongly so if you really like "memory"
I'll change it back.
> > .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> > },
> > - { .name = "mdr",
> > + { .name = "mdW",
> > .func = kdb_md,
> > - .usage = "<vaddr> <bytes>",
> > - .help = "Display Raw Memory",
> > + .usage = "<vaddr> [<lines> [<radix>]]",
> > + .help = "Display RAM using word size (W) of 1, 2, 4, or 8",
>
> We need an "e.g. md8" in here somewhere. Otherwise it is not at all
> obvious that W is a wildcard.
>
> I guess that alternatively you could also try naming the command with
> hint that W is a wild card (what happens if you register a command
> called md<W>?).
>
>
> > + .flags = KDB_ENABLE_MEM_READ | KDB_REPEAT_NO_ARGS,
> > + },
> > + { .name = "mdWcN",
> > + .func = kdb_md,
> > + .usage = "<vaddr> [<lines> [<radix>]]",
> > + .help = "Display RAM using word size (W); show N words",
>
> Same here.
Sure, so:
.name = "md<W>",
.help = "Display RAM using word size 1, 2, 4, or 8; e.g. md8",
.name = "md<W>c<N>",
.help = "Display RAM using word size W; show N words; e.g. md4c6",
...or changing RAM to "memory" if you don't buy my argument above.
We're definitely ending up over the 80 character mark here, but I
assume that's OK. We were even before my change.
I'll assume that I don't need the "e.g." for all the followup (mdp,
mdi) variants introduced in later patches?
Random question: for the mdWcN variant, should I make specifying
<lines> illegal? It's pretty silly to let the user specify a word
count and then immediately override it. In that case, do I bump
"<radix>" to the 2nd argument or just don't allow "<radix>" for mdWcN?
That would need to be done in a later patch, obviously...
-Doug
© 2016 - 2025 Red Hat, Inc.