All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 1/1] cmd: add exception command
@ 2018-11-17 14:28 Heinrich Schuchardt
  2018-11-27  0:08 ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2018-11-17 14:28 UTC (permalink / raw
  To: u-boot

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@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@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);
+};
+
+#if defined(CONFIG_ARM)
+static void data_abort(void)
+{
+#if defined(CONFIG_ARM64)
+	/*
+	 * The LDR instruction requires the data source to be eight byte
+	 * aligned.
+	 */
+	asm volatile (
+		"MOV x1, sp\n"
+		"ADD x1, x1, 1\n"
+		"LDR x3, [x1]\n");
+#else
+	/*
+	 * The LDRD instruction requires the data source to be four byte aligned
+	 * even if strict alignment fault checking is disabled in the system
+	 * control register.
+	 */
+	asm volatile (
+		"MOV r5, sp\n"
+		"ADD r5, #1\n"
+		"LDRD r6, r7, [r5]\n");
+#endif
+}
+
+#if !defined(CONFIG_ARM64)
+static void breakpoint(void)
+{
+	asm volatile ("BKPT #123\n");
+}
+#endif
+#endif
+
+static void undefined_instruction(void)
+{
+#if defined(CONFIG_ARM)
+	/*
+	 * 0xe7f...f.	is undefined in ARM mode
+	 * 0xde..	is undefined in Thumb mode
+	 */
+	asm volatile (".word 0xe7f7defb\n");
+#elif defined(CONFIG_RISCV)
+	asm volatile (".word 0xffffffff\n");
+#elif defined(CONFIG_X86)
+	asm volatile (".word 0xffff\n");
+#endif
+}
+
+struct exception_str exceptions[] = {
+#if defined(CONFIG_ARM)
+#if !defined(CONFIG_ARM64)
+	{ BREAKPOINT, "breakpoint", breakpoint },
+#endif
+	{ DATA_ABORT, "unaligned", data_abort },
+#endif
+	{ UNDEFINED_INSTRUCTION, "undefined", undefined_instruction },
+	{ 0, NULL, NULL },
+};
+
+static int do_exception(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	struct exception_str *ex;
+	enum exception id = 0;
+
+	if (argc != 2)
+		return CMD_RET_USAGE;
+
+	for (ex = exceptions; ex->func ; ++ex) {
+		if (!strcmp(argv[1], ex->text)) {
+			id = ex->id;
+			break;
+		}
+	}
+	switch (id) {
+#if defined(CONFIG_ARM)
+#if !defined(CONFIG_ARM64)
+	case BREAKPOINT:
+		breakpoint();
+		break;
+#endif
+	case DATA_ABORT:
+		data_abort();
+		break;
+#endif
+	case UNDEFINED_INSTRUCTION:
+		undefined_instruction();
+		break;
+	default:
+		return CMD_RET_USAGE;
+	}
+
+	return CMD_RET_FAILURE;
+}
+
+static int exception_complete(int argc, char * const argv[], char last_char,
+			      int maxv, char *cmdv[])
+{
+	int len = 0;
+	struct exception_str *ex;
+	int i = 0;
+
+	switch (argc) {
+	case 1:
+		break;
+	case 2:
+		len = strlen(argv[1]);
+		break;
+	default:
+		return 0;
+	}
+	for (ex = exceptions; ex->func ; ++ex) {
+		if (i >= maxv - 1)
+			return i;
+		if (!strncmp(argv[1], ex->text, len))
+			cmdv[i++] = ex->text;
+	}
+	cmdv[i] = NULL;
+	return i;
+}
+
+#ifdef CONFIG_SYS_LONGHELP
+static char exception_help_text[] =
+	"<ex>\n"
+	"  The following exceptions are available:\n"
+#if defined(CONFIG_ARM)
+#if !defined(CONFIG_ARM64)
+	"  breakpoint - prefetch abort\n"
+#endif
+	"  unaligned  - data abort\n"
+#endif
+#endif
+	"  undefined  - undefined instruction\n"
+	;
+
+U_BOOT_CMD_COMPLETE(
+	exception, 2, 0, do_exception,
+	"Forces an exception to occur",
+	exception_help_text, exception_complete
+);
-- 
2.19.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-11-17 14:28 [U-Boot] [PATCH 1/1] " Heinrich Schuchardt
@ 2018-11-27  0:08 ` Simon Glass
  2018-12-20 12:23   ` Heinrich Schuchardt
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2018-11-27  0:08 UTC (permalink / raw
  To: u-boot

Hi Heinrich,

On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt <xypron.glpk@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@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@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?

> +
> +#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.

> +static void data_abort(void)
> +{
> +#if defined(CONFIG_ARM64)
> +       /*
> +        * The LDR instruction requires the data source to be eight byte
> +        * aligned.
> +        */
> +       asm volatile (
> +               "MOV x1, sp\n"
> +               "ADD x1, x1, 1\n"
> +               "LDR x3, [x1]\n");

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-11-27  0:08 ` Simon Glass
@ 2018-12-20 12:23   ` Heinrich Schuchardt
  2018-12-20 21:17     ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2018-12-20 12:23 UTC (permalink / raw
  To: u-boot

On 11/27/18 1:08 AM, Simon Glass wrote:
> Hi Heinrich,
> 
> On Sat, 17 Nov 2018 at 07:29, Heinrich Schuchardt <xypron.glpk@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@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@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_".

> 
>> +
>> +#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

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

Best regards

Heinrich

> 
>> +static void data_abort(void)
>> +{
>> +#if defined(CONFIG_ARM64)
>> +       /*
>> +        * The LDR instruction requires the data source to be eight byte
>> +        * aligned.
>> +        */
>> +       asm volatile (
>> +               "MOV x1, sp\n"
>> +               "ADD x1, x1, 1\n"
>> +               "LDR x3, [x1]\n");
> 
> Regards,
> Simon
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-12-20 12:23   ` Heinrich Schuchardt
@ 2018-12-20 21:17     ` Simon Glass
  2018-12-21  1:58       ` Heinrich Schuchardt
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2018-12-20 21:17 UTC (permalink / raw
  To: u-boot

Hi Heinrich,

On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt <xypron.glpk@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@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@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@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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-12-20 21:17     ` Simon Glass
@ 2018-12-21  1:58       ` Heinrich Schuchardt
  2018-12-21 21:15         ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2018-12-21  1:58 UTC (permalink / raw
  To: u-boot

On 12/20/18 10:17 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt <xypron.glpk@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@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@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@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.

Hello Simon,

thanks for pointing me to the right place. I will adjust my patch.

I think we have some design issues with sub-commands:

I want to have auto-completion for the sub-commands. Unfortunately the
"standard way" does not offer it out of the box.

The standard way further forces the creation of redundant code like:

static int do_zynq(cmd_tbl_t *cmdtp, int flag, int argc, char * const
argv[])
{
        cmd_tbl_t *zynq_cmd;
        int ret;

        if (!ARRAY_SIZE(zynq_commands)) {
                puts("No zynq specific command enabled\n");
                return CMD_RET_USAGE;
        }

        if (argc < 2)
                return CMD_RET_USAGE;
        zynq_cmd = find_cmd_tbl(argv[1], zynq_commands,
                                ARRAY_SIZE(zynq_commands));
        if (!zynq_cmd || argc != zynq_cmd->maxargs)
                return CMD_RET_USAGE;

        ret = zynq_cmd->cmd(zynq_cmd, flag, argc, argv);

        return cmd_process_error(zynq_cmd, ret);
}

It would be preferable to reference cmd_tbl_t in U_BOOT_CMD. Furthermore
U_BOOT_CMD_MKENT should allow references to a cmd_tbl_t for sub-sub
commands. This way autocompletion of subcommands could be provided.

This would also allow to place the help texts for subcommands into
U_BOOT_CMD_MKENT where they belong.

Why are U_BOOT_SUBCMD_START and U_BOOT_SUBCMD_END only defined for
CONFIG_CMDLINE=n? This way we cannot use them.

Why do we check CONFIG_SYS_LONGHELP inside cmd/*.c for many commands?
Hasn't this been superseded by _CMD_HELP in include/commands.h?

Best regards

Heinrich

> 
>>
>>>
>>>> +
>>>> +#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
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-12-21  1:58       ` Heinrich Schuchardt
@ 2018-12-21 21:15         ` Simon Glass
  0 siblings, 0 replies; 13+ messages in thread
From: Simon Glass @ 2018-12-21 21:15 UTC (permalink / raw
  To: u-boot

Hi Heinrich,

On Thu, 20 Dec 2018 at 18:58, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 12/20/18 10:17 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Thu, 20 Dec 2018 at 05:23, Heinrich Schuchardt <xypron.glpk@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@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@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@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.
>
> Hello Simon,
>
> thanks for pointing me to the right place. I will adjust my patch.
>
> I think we have some design issues with sub-commands:
>
> I want to have auto-completion for the sub-commands. Unfortunately the
> "standard way" does not offer it out of the box.
>
> The standard way further forces the creation of redundant code like:
>
> static int do_zynq(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> {
>         cmd_tbl_t *zynq_cmd;
>         int ret;
>
>         if (!ARRAY_SIZE(zynq_commands)) {
>                 puts("No zynq specific command enabled\n");
>                 return CMD_RET_USAGE;
>         }
>
>         if (argc < 2)
>                 return CMD_RET_USAGE;
>         zynq_cmd = find_cmd_tbl(argv[1], zynq_commands,
>                                 ARRAY_SIZE(zynq_commands));
>         if (!zynq_cmd || argc != zynq_cmd->maxargs)
>                 return CMD_RET_USAGE;
>
>         ret = zynq_cmd->cmd(zynq_cmd, flag, argc, argv);
>
>         return cmd_process_error(zynq_cmd, ret);
> }
>
> It would be preferable to reference cmd_tbl_t in U_BOOT_CMD. Furthermore
> U_BOOT_CMD_MKENT should allow references to a cmd_tbl_t for sub-sub
> commands. This way autocompletion of subcommands could be provided.
>
> This would also allow to place the help texts for subcommands into
> U_BOOT_CMD_MKENT where they belong.

Yes indeed. There was a recent series sent to introduce
auto-completion for that I think. I have not looked at it though.

>
> Why are U_BOOT_SUBCMD_START and U_BOOT_SUBCMD_END only defined for
> CONFIG_CMDLINE=n? This way we cannot use them.

Well it was a feature designed to provide a way to drop commands. I
have no objection to changing this.

>
> Why do we check CONFIG_SYS_LONGHELP inside cmd/*.c for many commands?
> Hasn't this been superseded by _CMD_HELP in include/commands.h?

Looks like it. Yes this could be cleaned up.

Regards,
Simon


>
> Best regards
>
> Heinrich
>
> >
> >>
> >>>
> >>>> +
> >>>> +#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
> >
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
@ 2018-12-26 16:20 Heinrich Schuchardt
  2018-12-29 13:39 ` Simon Glass
  2019-04-24 13:21 ` [U-Boot] [U-Boot,1/1] " Tom Rini
  0 siblings, 2 replies; 13+ messages in thread
From: Heinrich Schuchardt @ 2018-12-26 16:20 UTC (permalink / raw
  To: u-boot

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@gmx.de>
---
v2:
	Split architecture specific code into separate files.
	Provide include for common code.
	Update MAINTAINERS file.
---
 MAINTAINERS           |  3 +++
 cmd/Kconfig           |  6 +++++
 cmd/Makefile          |  2 ++
 cmd/arm/Makefile      |  7 +++++
 cmd/arm/exception.c   | 61 +++++++++++++++++++++++++++++++++++++++++++
 cmd/arm/exception64.c | 33 +++++++++++++++++++++++
 cmd/riscv/Makefile    |  3 +++
 cmd/riscv/exception.c | 29 ++++++++++++++++++++
 cmd/x86/Makefile      |  1 +
 cmd/x86/exception.c   | 29 ++++++++++++++++++++
 include/exception.h   | 58 ++++++++++++++++++++++++++++++++++++++++
 11 files changed, 232 insertions(+)
 create mode 100644 cmd/arm/Makefile
 create mode 100644 cmd/arm/exception.c
 create mode 100644 cmd/arm/exception64.c
 create mode 100644 cmd/riscv/Makefile
 create mode 100644 cmd/riscv/exception.c
 create mode 100644 cmd/x86/exception.c
 create mode 100644 include/exception.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ae825014bd..c26eece24c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -84,6 +84,7 @@ M:	Albert Aribaud <albert.u.boot@aribaud.net>
 S:	Maintained
 T:	git git://git.denx.de/u-boot-arm.git
 F:	arch/arm/
+F:	cmd/arm/
 
 ARM ALTERA SOCFPGA
 M:	Marek Vasut <marex@denx.de>
@@ -624,6 +625,7 @@ M:	Rick Chen <rick@andestech.com>
 S:	Maintained
 T:	git git://git.denx.de/u-boot-riscv.git
 F:	arch/riscv/
+F:	cmd/riscv/
 F:	tools/prelink-riscv.c
 
 ROCKUSB
@@ -725,6 +727,7 @@ M:	Bin Meng <bmeng.cn@gmail.com>
 S:	Maintained
 T:	git git://git.denx.de/u-boot-x86.git
 F:	arch/x86/
+F:	cmd/x86/
 
 XTENSA
 M:	Max Filippov <jcmvbkbc@gmail.com>
diff --git a/cmd/Kconfig b/cmd/Kconfig
index ea1a325eb3..308e5458b9 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1397,6 +1397,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 15ae4d250f..bff6be0146 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -170,6 +170,8 @@ obj-$(CONFIG_CMD_BLOB) += blob.o
 # Android Verified Boot 2.0
 obj-$(CONFIG_CMD_AVB) += avb.o
 
+obj-$(CONFIG_ARM) += arm/
+obj-$(CONFIG_RISCV) += riscv/
 obj-$(CONFIG_X86) += x86/
 
 obj-$(CONFIG_ARCH_MVEBU) += mvebu/
diff --git a/cmd/arm/Makefile b/cmd/arm/Makefile
new file mode 100644
index 0000000000..94367dcb45
--- /dev/null
+++ b/cmd/arm/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+ifdef CONFIG_ARM64
+obj-$(CONFIG_CMD_EXCEPTION) += exception64.o
+else
+obj-$(CONFIG_CMD_EXCEPTION) += exception.o
+endif
diff --git a/cmd/arm/exception.c b/cmd/arm/exception.c
new file mode 100644
index 0000000000..33bc75976f
--- /dev/null
+++ b/cmd/arm/exception.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * The 'exception' command can be used for testing exception handling.
+ *
+ * Copyright (c) 2018, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#include <common.h>
+#include <command.h>
+
+static int do_unaligned(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	/*
+	 * The LDRD instruction requires the data source to be four byte aligned
+	 * even if strict alignment fault checking is disabled in the system
+	 * control register.
+	 */
+	asm volatile (
+		"MOV r5, sp\n"
+		"ADD r5, #1\n"
+		"LDRD r6, r7, [r5]\n");
+	return CMD_RET_FAILURE;
+}
+
+static int do_breakpoint(cmd_tbl_t *cmdtp, int flag, int argc,
+			 char * const argv[])
+{
+	asm volatile ("BKPT #123\n");
+	return CMD_RET_FAILURE;
+}
+
+static int do_undefined(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	/*
+	 * 0xe7f...f.	is undefined in ARM mode
+	 * 0xde..	is undefined in Thumb mode
+	 */
+	asm volatile (".word 0xe7f7defb\n");
+	return CMD_RET_FAILURE;
+}
+
+static cmd_tbl_t cmd_sub[] = {
+	U_BOOT_CMD_MKENT(breakpoint, CONFIG_SYS_MAXARGS, 1, do_breakpoint,
+			 "", ""),
+	U_BOOT_CMD_MKENT(unaligned, CONFIG_SYS_MAXARGS, 1, do_unaligned,
+			 "", ""),
+	U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
+			 "", ""),
+};
+
+static char exception_help_text[] =
+	"<ex>\n"
+	"  The following exceptions are available:\n"
+	"  breakpoint - prefetch abort\n"
+	"  unaligned  - data abort\n"
+	"  undefined  - undefined instruction\n"
+	;
+
+#include <exception.h>
diff --git a/cmd/arm/exception64.c b/cmd/arm/exception64.c
new file mode 100644
index 0000000000..a363818532
--- /dev/null
+++ b/cmd/arm/exception64.c
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * The 'exception' command can be used for testing exception handling.
+ *
+ * Copyright (c) 2018, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#include <common.h>
+#include <command.h>
+
+static int do_undefined(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	/*
+	 * 0xe7f...f.	is undefined in ARM mode
+	 * 0xde..	is undefined in Thumb mode
+	 */
+	asm volatile (".word 0xe7f7defb\n");
+	return CMD_RET_FAILURE;
+}
+
+static cmd_tbl_t cmd_sub[] = {
+	U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
+			 "", ""),
+};
+
+static char exception_help_text[] =
+	"<ex>\n"
+	"  The following exceptions are available:\n"
+	"  undefined  - undefined instruction\n"
+	;
+
+#include <exception.h>
diff --git a/cmd/riscv/Makefile b/cmd/riscv/Makefile
new file mode 100644
index 0000000000..24df023ece
--- /dev/null
+++ b/cmd/riscv/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0+
+
+obj-$(CONFIG_CMD_EXCEPTION) += exception.o
diff --git a/cmd/riscv/exception.c b/cmd/riscv/exception.c
new file mode 100644
index 0000000000..547fb7d132
--- /dev/null
+++ b/cmd/riscv/exception.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * The 'exception' command can be used for testing exception handling.
+ *
+ * Copyright (c) 2018, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#include <common.h>
+#include <command.h>
+
+static int do_undefined(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	asm volatile (".word 0xffffffff\n");
+	return CMD_RET_FAILURE;
+}
+
+static cmd_tbl_t cmd_sub[] = {
+	U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
+			 "", ""),
+};
+
+static char exception_help_text[] =
+	"<ex>\n"
+	"  The following exceptions are available:\n"
+	"  undefined  - undefined instruction\n"
+	;
+
+#include <exception.h>
diff --git a/cmd/x86/Makefile b/cmd/x86/Makefile
index bcc6d06582..707161440d 100644
--- a/cmd/x86/Makefile
+++ b/cmd/x86/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 obj-y += mtrr.o
+obj-$(CONFIG_CMD_EXCEPTION) += exception.o
 obj-$(CONFIG_HAVE_FSP) += fsp.o
diff --git a/cmd/x86/exception.c b/cmd/x86/exception.c
new file mode 100644
index 0000000000..ade1e2ea92
--- /dev/null
+++ b/cmd/x86/exception.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * The 'exception' command can be used for testing exception handling.
+ *
+ * Copyright (c) 2018, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+#include <common.h>
+#include <command.h>
+
+static int do_undefined(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	asm volatile (".word 0xffff\n");
+	return CMD_RET_FAILURE;
+}
+
+static cmd_tbl_t cmd_sub[] = {
+	U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
+			 "", ""),
+};
+
+static char exception_help_text[] =
+	"<ex>\n"
+	"  The following exceptions are available:\n"
+	"  undefined  - undefined instruction\n"
+	;
+
+#include <exception.h>
diff --git a/include/exception.h b/include/exception.h
new file mode 100644
index 0000000000..d65c585341
--- /dev/null
+++ b/include/exception.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * The 'exception' command can be used for testing exception handling.
+ *
+ * Copyright (c) 2018, Heinrich Schuchardt <xypron.glpk@gmx.de>
+ */
+
+static int do_exception(cmd_tbl_t *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	cmd_tbl_t *cp;
+
+	if (argc != 2)
+		return CMD_RET_USAGE;
+
+	/* drop sub-command parameter */
+	argc--;
+	argv++;
+
+	cp = find_cmd_tbl(argv[0], cmd_sub, ARRAY_SIZE(cmd_sub));
+
+	if (cp)
+		return cp->cmd(cmdtp, flag, argc, argv);
+
+	return CMD_RET_USAGE;
+}
+
+static int exception_complete(int argc, char * const argv[], char last_char,
+			      int maxv, char *cmdv[])
+{
+	int len = 0;
+	int i = 0;
+	cmd_tbl_t *cmdtp;
+
+	switch (argc) {
+	case 1:
+		break;
+	case 2:
+		len = strlen(argv[1]);
+		break;
+	default:
+		return 0;
+	}
+	for (cmdtp = cmd_sub; cmdtp != cmd_sub + ARRAY_SIZE(cmd_sub); cmdtp++) {
+		if (i >= maxv - 1)
+			return i;
+		if (!strncmp(argv[1], cmdtp->name, len))
+			cmdv[i++] = cmdtp->name;
+	}
+	cmdv[i] = NULL;
+	return i;
+}
+
+U_BOOT_CMD_COMPLETE(
+	exception, 2, 0, do_exception,
+	"Forces an exception to occur",
+	exception_help_text, exception_complete
+);
-- 
2.19.2

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-12-26 16:20 [U-Boot] [PATCH 1/1] cmd: add exception command Heinrich Schuchardt
@ 2018-12-29 13:39 ` Simon Glass
  2018-12-30  8:33   ` Heinrich Schuchardt
  2019-04-24 13:21 ` [U-Boot] [U-Boot,1/1] " Tom Rini
  1 sibling, 1 reply; 13+ messages in thread
From: Simon Glass @ 2018-12-29 13:39 UTC (permalink / raw
  To: u-boot

Hi Heinrich,

On Wed, 26 Dec 2018 at 09:20, Heinrich Schuchardt <xypron.glpk@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@gmx.de>
> ---
> v2:
>         Split architecture specific code into separate files.
>         Provide include for common code.
>         Update MAINTAINERS file.
> ---
>  MAINTAINERS           |  3 +++
>  cmd/Kconfig           |  6 +++++
>  cmd/Makefile          |  2 ++
>  cmd/arm/Makefile      |  7 +++++
>  cmd/arm/exception.c   | 61 +++++++++++++++++++++++++++++++++++++++++++
>  cmd/arm/exception64.c | 33 +++++++++++++++++++++++
>  cmd/riscv/Makefile    |  3 +++
>  cmd/riscv/exception.c | 29 ++++++++++++++++++++
>  cmd/x86/Makefile      |  1 +
>  cmd/x86/exception.c   | 29 ++++++++++++++++++++
>  include/exception.h   | 58 ++++++++++++++++++++++++++++++++++++++++
>  11 files changed, 232 insertions(+)
>  create mode 100644 cmd/arm/Makefile
>  create mode 100644 cmd/arm/exception.c
>  create mode 100644 cmd/arm/exception64.c
>  create mode 100644 cmd/riscv/Makefile
>  create mode 100644 cmd/riscv/exception.c
>  create mode 100644 cmd/x86/exception.c
>  create mode 100644 include/exception.h

This needs something like Series-version: 2 (if you use patman) to set
the version number in the header.

Did you look at using a uclass and driver, like sysreset?

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-12-29 13:39 ` Simon Glass
@ 2018-12-30  8:33   ` Heinrich Schuchardt
  2019-01-05  1:56     ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2018-12-30  8:33 UTC (permalink / raw
  To: u-boot

On 12/29/18 2:39 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, 26 Dec 2018 at 09:20, Heinrich Schuchardt <xypron.glpk@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@gmx.de>
>> ---
>> v2:
>>         Split architecture specific code into separate files.
>>         Provide include for common code.
>>         Update MAINTAINERS file.
>> ---
>>  MAINTAINERS           |  3 +++
>>  cmd/Kconfig           |  6 +++++
>>  cmd/Makefile          |  2 ++
>>  cmd/arm/Makefile      |  7 +++++
>>  cmd/arm/exception.c   | 61 +++++++++++++++++++++++++++++++++++++++++++
>>  cmd/arm/exception64.c | 33 +++++++++++++++++++++++
>>  cmd/riscv/Makefile    |  3 +++
>>  cmd/riscv/exception.c | 29 ++++++++++++++++++++
>>  cmd/x86/Makefile      |  1 +
>>  cmd/x86/exception.c   | 29 ++++++++++++++++++++
>>  include/exception.h   | 58 ++++++++++++++++++++++++++++++++++++++++
>>  11 files changed, 232 insertions(+)
>>  create mode 100644 cmd/arm/Makefile
>>  create mode 100644 cmd/arm/exception.c
>>  create mode 100644 cmd/arm/exception64.c
>>  create mode 100644 cmd/riscv/Makefile
>>  create mode 100644 cmd/riscv/exception.c
>>  create mode 100644 cmd/x86/exception.c
>>  create mode 100644 include/exception.h
> 
> This needs something like Series-version: 2 (if you use patman) to set
> the version number in the header.

Sorry for the mishap.

> 
> Did you look at using a uclass and driver, like sysreset?

Yes I have considered using a u-class. But I could not see how adding a
separate u-class file would save lines, make the coding less complex, or
make the coding easier to maintain. A u-class would make sense if there
were other consumers for exceptions but the exception command. But I
cannot imagine any.

There are better places to apply u-classes, e.g. I am really missing a
u-class for file systems.

Best regards

Heinrich

> 
> Regards,
> Simon
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2018-12-30  8:33   ` Heinrich Schuchardt
@ 2019-01-05  1:56     ` Simon Glass
  2019-02-18 19:38       ` Heinrich Schuchardt
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2019-01-05  1:56 UTC (permalink / raw
  To: u-boot

Hi Heinrich,

On Sun, 30 Dec 2018 at 01:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 12/29/18 2:39 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 26 Dec 2018 at 09:20, Heinrich Schuchardt <xypron.glpk@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@gmx.de>
> >> ---
> >> v2:
> >>         Split architecture specific code into separate files.
> >>         Provide include for common code.
> >>         Update MAINTAINERS file.
> >> ---
> >>  MAINTAINERS           |  3 +++
> >>  cmd/Kconfig           |  6 +++++
> >>  cmd/Makefile          |  2 ++
> >>  cmd/arm/Makefile      |  7 +++++
> >>  cmd/arm/exception.c   | 61 +++++++++++++++++++++++++++++++++++++++++++
> >>  cmd/arm/exception64.c | 33 +++++++++++++++++++++++
> >>  cmd/riscv/Makefile    |  3 +++
> >>  cmd/riscv/exception.c | 29 ++++++++++++++++++++
> >>  cmd/x86/Makefile      |  1 +
> >>  cmd/x86/exception.c   | 29 ++++++++++++++++++++
> >>  include/exception.h   | 58 ++++++++++++++++++++++++++++++++++++++++
> >>  11 files changed, 232 insertions(+)
> >>  create mode 100644 cmd/arm/Makefile
> >>  create mode 100644 cmd/arm/exception.c
> >>  create mode 100644 cmd/arm/exception64.c
> >>  create mode 100644 cmd/riscv/Makefile
> >>  create mode 100644 cmd/riscv/exception.c
> >>  create mode 100644 cmd/x86/exception.c
> >>  create mode 100644 include/exception.h
> >
> > This needs something like Series-version: 2 (if you use patman) to set
> > the version number in the header.
>
> Sorry for the mishap.
>
> >
> > Did you look at using a uclass and driver, like sysreset?
>
> Yes I have considered using a u-class. But I could not see how adding a
> separate u-class file would save lines, make the coding less complex, or
> make the coding easier to maintain. A u-class would make sense if there
> were other consumers for exceptions but the exception command. But I
> cannot imagine any.

In some sense driver model matches consumers and producers. There are
clearly multiple producers - you have effectively implemented an API
in a few places. We even have multiple impls for each arch.

So I still favour a uclass, but since you are pretty adamant that we
should not do it, I'm not going to insist.

>
> There are better places to apply u-classes, e.g. I am really missing a
> u-class for file systems.
>
> Best regards
>
> Heinrich
>
> >
> > Regards,
> > Simon
> >

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2019-01-05  1:56     ` Simon Glass
@ 2019-02-18 19:38       ` Heinrich Schuchardt
  2019-02-19  2:11         ` Tom Rini
  0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2019-02-18 19:38 UTC (permalink / raw
  To: u-boot

On 1/5/19 2:56 AM, Simon Glass wrote:
> Hi Heinrich,
> 
> On Sun, 30 Dec 2018 at 01:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 12/29/18 2:39 PM, Simon Glass wrote:
>>> Hi Heinrich,
>>>
>>> On Wed, 26 Dec 2018 at 09:20, Heinrich Schuchardt <xypron.glpk@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@gmx.de>
>>>> ---
>>>> v2:
>>>>         Split architecture specific code into separate files.
>>>>         Provide include for common code.
>>>>         Update MAINTAINERS file.
>>>> ---
>>>>  MAINTAINERS           |  3 +++
>>>>  cmd/Kconfig           |  6 +++++
>>>>  cmd/Makefile          |  2 ++
>>>>  cmd/arm/Makefile      |  7 +++++
>>>>  cmd/arm/exception.c   | 61 +++++++++++++++++++++++++++++++++++++++++++
>>>>  cmd/arm/exception64.c | 33 +++++++++++++++++++++++
>>>>  cmd/riscv/Makefile    |  3 +++
>>>>  cmd/riscv/exception.c | 29 ++++++++++++++++++++
>>>>  cmd/x86/Makefile      |  1 +
>>>>  cmd/x86/exception.c   | 29 ++++++++++++++++++++
>>>>  include/exception.h   | 58 ++++++++++++++++++++++++++++++++++++++++
>>>>  11 files changed, 232 insertions(+)
>>>>  create mode 100644 cmd/arm/Makefile
>>>>  create mode 100644 cmd/arm/exception.c
>>>>  create mode 100644 cmd/arm/exception64.c
>>>>  create mode 100644 cmd/riscv/Makefile
>>>>  create mode 100644 cmd/riscv/exception.c
>>>>  create mode 100644 cmd/x86/exception.c
>>>>  create mode 100644 include/exception.h
>>>
>>> This needs something like Series-version: 2 (if you use patman) to set
>>> the version number in the header.
>>
>> Sorry for the mishap.
>>
>>>
>>> Did you look at using a uclass and driver, like sysreset?
>>
>> Yes I have considered using a u-class. But I could not see how adding a
>> separate u-class file would save lines, make the coding less complex, or
>> make the coding easier to maintain. A u-class would make sense if there
>> were other consumers for exceptions but the exception command. But I
>> cannot imagine any.
> 
> In some sense driver model matches consumers and producers. There are
> clearly multiple producers - you have effectively implemented an API
> in a few places. We even have multiple impls for each arch.
> 
> So I still favour a uclass, but since you are pretty adamant that we
> should not do it, I'm not going to insist.

Hello Tom,

in patchwork this patch is still in status 'NEW'.

It is unclear to me if you are going to merge it as is or if I should
rework it.

Best regards

Heinrich


> 
>>
>> There are better places to apply u-classes, e.g. I am really missing a
>> u-class for file systems.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Regards,
>>> Simon
>>>
> 
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH 1/1] cmd: add exception command
  2019-02-18 19:38       ` Heinrich Schuchardt
@ 2019-02-19  2:11         ` Tom Rini
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Rini @ 2019-02-19  2:11 UTC (permalink / raw
  To: u-boot

On Mon, Feb 18, 2019 at 08:38:52PM +0100, Heinrich Schuchardt wrote:

> On 1/5/19 2:56 AM, Simon Glass wrote:
> > Hi Heinrich,
> > 
> > On Sun, 30 Dec 2018 at 01:33, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 12/29/18 2:39 PM, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Wed, 26 Dec 2018 at 09:20, Heinrich Schuchardt <xypron.glpk@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@gmx.de>
> >>>> ---
> >>>> v2:
> >>>>         Split architecture specific code into separate files.
> >>>>         Provide include for common code.
> >>>>         Update MAINTAINERS file.
> >>>> ---
> >>>>  MAINTAINERS           |  3 +++
> >>>>  cmd/Kconfig           |  6 +++++
> >>>>  cmd/Makefile          |  2 ++
> >>>>  cmd/arm/Makefile      |  7 +++++
> >>>>  cmd/arm/exception.c   | 61 +++++++++++++++++++++++++++++++++++++++++++
> >>>>  cmd/arm/exception64.c | 33 +++++++++++++++++++++++
> >>>>  cmd/riscv/Makefile    |  3 +++
> >>>>  cmd/riscv/exception.c | 29 ++++++++++++++++++++
> >>>>  cmd/x86/Makefile      |  1 +
> >>>>  cmd/x86/exception.c   | 29 ++++++++++++++++++++
> >>>>  include/exception.h   | 58 ++++++++++++++++++++++++++++++++++++++++
> >>>>  11 files changed, 232 insertions(+)
> >>>>  create mode 100644 cmd/arm/Makefile
> >>>>  create mode 100644 cmd/arm/exception.c
> >>>>  create mode 100644 cmd/arm/exception64.c
> >>>>  create mode 100644 cmd/riscv/Makefile
> >>>>  create mode 100644 cmd/riscv/exception.c
> >>>>  create mode 100644 cmd/x86/exception.c
> >>>>  create mode 100644 include/exception.h
> >>>
> >>> This needs something like Series-version: 2 (if you use patman) to set
> >>> the version number in the header.
> >>
> >> Sorry for the mishap.
> >>
> >>>
> >>> Did you look at using a uclass and driver, like sysreset?
> >>
> >> Yes I have considered using a u-class. But I could not see how adding a
> >> separate u-class file would save lines, make the coding less complex, or
> >> make the coding easier to maintain. A u-class would make sense if there
> >> were other consumers for exceptions but the exception command. But I
> >> cannot imagine any.
> > 
> > In some sense driver model matches consumers and producers. There are
> > clearly multiple producers - you have effectively implemented an API
> > in a few places. We even have multiple impls for each arch.
> > 
> > So I still favour a uclass, but since you are pretty adamant that we
> > should not do it, I'm not going to insist.
> 
> Hello Tom,
> 
> in patchwork this patch is still in status 'NEW'.
> 
> It is unclear to me if you are going to merge it as is or if I should
> rework it.

I guess I hadn't made a decision here.  I guess if you're really sure it
doesn't need what Simon is suggesting, then yes, I'll pick this up
as-is.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190218/8fa68951/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [U-Boot,1/1] cmd: add exception command
  2018-12-26 16:20 [U-Boot] [PATCH 1/1] cmd: add exception command Heinrich Schuchardt
  2018-12-29 13:39 ` Simon Glass
@ 2019-04-24 13:21 ` Tom Rini
  1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2019-04-24 13:21 UTC (permalink / raw
  To: u-boot

On Wed, Dec 26, 2018 at 05:20:35PM +0100, Heinrich Schuchardt 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@gmx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190424/62dc1f27/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-04-24 13:21 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-12-26 16:20 [U-Boot] [PATCH 1/1] cmd: add exception command Heinrich Schuchardt
2018-12-29 13:39 ` Simon Glass
2018-12-30  8:33   ` Heinrich Schuchardt
2019-01-05  1:56     ` Simon Glass
2019-02-18 19:38       ` Heinrich Schuchardt
2019-02-19  2:11         ` Tom Rini
2019-04-24 13:21 ` [U-Boot] [U-Boot,1/1] " Tom Rini
  -- strict thread matches above, loose matches on Subject: below --
2018-11-17 14:28 [U-Boot] [PATCH 1/1] " Heinrich Schuchardt
2018-11-27  0:08 ` Simon Glass
2018-12-20 12:23   ` Heinrich Schuchardt
2018-12-20 21:17     ` Simon Glass
2018-12-21  1:58       ` Heinrich Schuchardt
2018-12-21 21:15         ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.