[U-Boot] [PATCH 1/1] cmd: add exception command

[Simon Glass]

Hi Heinrich,

On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 11/27/18 1:08 AM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> The 'exception' command allows to test exception handling.
> >>
> >> This implementation supports ARM, x86, RISC-V and the following exceptions:
> >> * 'breakpoint' - prefetch abort exception (ARM 32bit only)
> >> * 'unaligned'  - data abort exception (ARM only)
> >> * 'undefined'  - undefined instruction exception
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> ---
> >> Currently RISC-V 64bit gets into an endless loop when hitting the
> >> undefined instruction.
> >>
> >> So it makes sense to have a testing capability.
> >> ---
> >>  cmd/Kconfig     |   6 ++
> >>  cmd/Makefile    |   1 +
> >>  cmd/exception.c | 161 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 168 insertions(+)
> >>  create mode 100644 cmd/exception.c
> >>
> >> diff --git a/cmd/Kconfig b/cmd/Kconfig
> >> index e2973b3c51..9d2b8199b8 100644
> >> --- a/cmd/Kconfig
> >> +++ b/cmd/Kconfig
> >> @@ -1388,6 +1388,12 @@ config CMD_DISPLAY
> >>           displayed on a simple board-specific display. Implement
> >>           display_putc() to use it.
> >>
> >> +config CMD_EXCEPTION
> >> +       bool "exception - raise exception"
> >> +       depends on ARM || RISCV || X86
> >> +       help
> >> +         Enable the 'exception' command which allows to raise an exception.
> >> +
> >>  config CMD_LED
> >>         bool "led"
> >>         default y if LED
> >> diff --git a/cmd/Makefile b/cmd/Makefile
> >> index 0534ddc679..cd67a79170 100644
> >> --- a/cmd/Makefile
> >> +++ b/cmd/Makefile
> >> @@ -46,6 +46,7 @@ endif
> >>  obj-$(CONFIG_CMD_DISPLAY) += display.o
> >>  obj-$(CONFIG_CMD_DTIMG) += dtimg.o
> >>  obj-$(CONFIG_CMD_ECHO) += echo.o
> >> +obj-$(CONFIG_CMD_EXCEPTION) += exception.o
> >>  obj-$(CONFIG_ENV_IS_IN_EEPROM) += eeprom.o
> >>  obj-$(CONFIG_CMD_EEPROM) += eeprom.o
> >>  obj-$(CONFIG_EFI_STUB) += efi.o
> >> diff --git a/cmd/exception.c b/cmd/exception.c
> >> new file mode 100644
> >> index 0000000000..fb6a15573e
> >> --- /dev/null
> >> +++ b/cmd/exception.c
> >> @@ -0,0 +1,161 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * The 'exception' command can be used for testing exception handling.
> >> + *
> >> + * Copyright (c) 2018, Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> + */
> >> +#include <common.h>
> >> +#include <command.h>
> >> +
> >> +enum exception {
> >> +       UNDEFINED_INSTRUCTION = 1,
> >> +       DATA_ABORT,
> >> +       BREAKPOINT,
> >> +};
> >> +
> >> +struct exception_str {
> >> +       enum exception id;
> >> +       char *text;
> >> +       void (*func)(void);
> >> +};
> >
> > Can you use the normal subcommand approach for this, as with other commands?
>
> Could you give an example, please. I looked at cmd/scsi. and cmd/mmc.c
> And the only difference I spot is that the names of subcommand functions
> start with "do_".

Yes, see for example:

static cmd_tbl_t cmd_mmc[] = {

It defines the subcommands in a standard way.

>
> >
> >> +
> >> +#if defined(CONFIG_ARM)
> >
> > How about creating a uclass for this? It isn't nice to have a
> > arch-specific #ifdefs in commands. Something like we did with
> > sysreset.
>
> If you want separate files per architecture, why would we need a uclass?
>
> We could simply put the architecture specific U_BOOT_CMD into the
> implementation file, e.g arch/arm/cpu/exception.c and
> arch/arm/cpu/exception_64.c

That that is one option although perhaps you might end up with
multiple ARM implementation (e.g. for ARMv47, ARMv7 and ARMv8)?

>
> With a uclass I would not know how to implement an architecture specific
> help output.

One option is something like sysreset_walk(). It allows us to have a
common function for ARM undefined instruction, for example, but finer
granularity for breakpoint.

Regards,
Simon