[PATCH 1/1] cmd: exception: unaligned data access on RISC-V

[PATCH 1/1] cmd: exception: unaligned data access on RISC-V

[Heinrich Schuchardt]

The command 'exception' can be used to test the handling of exceptions.

Currently the exception command only allows to create an illegal
instruction exception on RISC-V.

Provide a sub-command 'exception unaligned' to cause a misaligned load
address exception.

Adjust the online help for 'exception undefined'.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 cmd/riscv/exception.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/cmd/riscv/exception.c b/cmd/riscv/exception.c
index 3c8dbbec0e..53159531d9 100644
--- a/cmd/riscv/exception.c
+++ b/cmd/riscv/exception.c
@@ -8,6 +8,17 @@
 #include <common.h>
 #include <command.h>

+static int do_unaligned(struct cmd_tbl *cmdtp, int flag, int argc,
+			char *const argv[])
+{
+	asm volatile (
+		"auipc a1, 0\n"
+		"ori   a1, a1, 3\n"
+		"lw    a2, (0)(a1)\n"
+	);
+	return CMD_RET_FAILURE;
+}
+
 static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
 			char *const argv[])
 {
@@ -16,6 +27,8 @@ static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
 }

 static struct cmd_tbl cmd_sub[] = {
+	U_BOOT_CMD_MKENT(unaligned, CONFIG_SYS_MAXARGS, 1, do_unaligned,
+			 "", ""),
 	U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
 			 "", ""),
 };
@@ -23,7 +36,8 @@ static struct cmd_tbl cmd_sub[] = {
 static char exception_help_text[] =
 	"<ex>\n"
 	"  The following exceptions are available:\n"
-	"  undefined  - undefined instruction\n"
+	"  undefined - illegal instruction\n"
+	"  unaligned - load address misaligned\n"
 	;

 #include <exception.h>
--
2.27.0

[PATCH 1/1] cmd: exception: unaligned data access on RISC-V

[Rick Chen]

Hi Heinrich

> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de]
> Sent: Tuesday, August 04, 2020 7:10 PM
> To: Rick Jian-Zhi Chen(???)
> Cc: u-boot at lists.denx.de; Heinrich Schuchardt
> Subject: [PATCH 1/1] cmd: exception: unaligned data access on RISC-V
>
> The command 'exception' can be used to test the handling of exceptions.
>
> Currently the exception command only allows to create an illegal instruction exception on RISC-V.
>
> Provide a sub-command 'exception unaligned' to cause a misaligned load address exception.
>
> Adjust the online help for 'exception undefined'.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  cmd/riscv/exception.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/riscv/exception.c b/cmd/riscv/exception.c index 3c8dbbec0e..53159531d9 100644
> --- a/cmd/riscv/exception.c
> +++ b/cmd/riscv/exception.c
> @@ -8,6 +8,17 @@
>  #include <common.h>
>  #include <command.h>
>
> +static int do_unaligned(struct cmd_tbl *cmdtp, int flag, int argc,
> +                       char *const argv[])
> +{
> +       asm volatile (
> +               "auipc a1, 0\n"
> +               "ori   a1, a1, 3\n"
> +               "lw    a2, (0)(a1)\n"
> +       );
> +       return CMD_RET_FAILURE;

I suggest that we can modify it as below to print the unaligned
address and data for more debug information.

int ret = 0;
int addr = 0;

asm volatile (
"auipc a1, 0\n"
"ori   %0, a1, 3\n"
"lw    %1, (0)(a1)\n"
: "=r" (addr), "=r" (ret)
:
: "memory"
);
printf("unaligned addr 0x%x , ret 0x%x\n",addr,ret);

return CMD_RET_SUCCESS;
====================================
So if run in S-Mode, it will work as below:

RISC-V # exception unaligned
unaligned addr 0x3ff92fd7 , ret 0x35e59300
RISC-V #


(gdb) x/4x 0x3ff92fd0
0x3ff92fd0:     0x7ac362ef      0x00000597      0x0035e593      0xc5174190
(gdb)

====================================
If run in M-Mode, it will work as below:

RISC-V # exception unaligned
Unhandled exception: Load address misaligned
EPC: 000000003ff92fdc RA: 000000003ff93032 TVAL: 000000003ff92fd7
EPC: 0000000000009fdc RA: 000000000000a032 reloc adjusted

### ERROR ### Please RESET the board ###

Thanks,
Rick

> +}
> +
>  static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
>                         char *const argv[])
>  {
> @@ -16,6 +27,8 @@ static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,  }
>
>  static struct cmd_tbl cmd_sub[] = {
> +       U_BOOT_CMD_MKENT(unaligned, CONFIG_SYS_MAXARGS, 1, do_unaligned,
> +                        "", ""),
>         U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
>                          "", ""),
>  };
> @@ -23,7 +36,8 @@ static struct cmd_tbl cmd_sub[] = {  static char exception_help_text[] =
>         "<ex>\n"
>         "  The following exceptions are available:\n"
> -       "  undefined  - undefined instruction\n"
> +       "  undefined - illegal instruction\n"
> +       "  unaligned - load address misaligned\n"
>         ;
>
>  #include <exception.h>
> --
> 2.27.0

[PATCH 1/1] cmd: exception: unaligned data access on RISC-V

[Heinrich Schuchardt]

On 8/5/20 10:47 AM, Rick Chen wrote:
> Hi Heinrich
>
>> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de]
>> Sent: Tuesday, August 04, 2020 7:10 PM
>> To: Rick Jian-Zhi Chen(???)
>> Cc: u-boot at lists.denx.de; Heinrich Schuchardt
>> Subject: [PATCH 1/1] cmd: exception: unaligned data access on RISC-V
>>
>> The command 'exception' can be used to test the handling of exceptions.
>>
>> Currently the exception command only allows to create an illegal instruction exception on RISC-V.
>>
>> Provide a sub-command 'exception unaligned' to cause a misaligned load address exception.
>>
>> Adjust the online help for 'exception undefined'.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  cmd/riscv/exception.c | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmd/riscv/exception.c b/cmd/riscv/exception.c index 3c8dbbec0e..53159531d9 100644
>> --- a/cmd/riscv/exception.c
>> +++ b/cmd/riscv/exception.c
>> @@ -8,6 +8,17 @@
>>  #include <common.h>
>>  #include <command.h>
>>
>> +static int do_unaligned(struct cmd_tbl *cmdtp, int flag, int argc,
>> +                       char *const argv[])
>> +{
>> +       asm volatile (
>> +               "auipc a1, 0\n"
>> +               "ori   a1, a1, 3\n"
>> +               "lw    a2, (0)(a1)\n"
>> +       );
>> +       return CMD_RET_FAILURE;
>
> I suggest that we can modify it as below to print the unaligned
> address and data for more debug information.
>
> int ret = 0;
> int addr = 0;
>
> asm volatile (
> "auipc a1, 0\n"
> "ori   %0, a1, 3\n"
> "lw    %1, (0)(a1)\n"
> : "=r" (addr), "=r" (ret)
> :
> : "memory"
> );
> printf("unaligned addr 0x%x , ret 0x%x\n",addr,ret);

Thanks for reviewing.

The printf statement will never be reached if the system does not
support unaligned access. Why should anybody care about the actual
addresses?

A reasonable message here might be:

printf("The system supports unaligned access.\n");

>
> return CMD_RET_SUCCESS;
> ====================================
> So if run in S-Mode, it will work as below:
>
> RISC-V # exception unaligned

Do we ever run the U-Boot shell in S-mode? Shouldn't we always drop to
M-mode before reaching the shell?

If I am in the U-Boot shell, why does S-mode not print out the registers
like M-Mode does? What is missing?

Best regards

Heinrich

> unaligned addr 0x3ff92fd7 , ret 0x35e59300
> RISC-V #
>
>
> (gdb) x/4x 0x3ff92fd0
> 0x3ff92fd0:     0x7ac362ef      0x00000597      0x0035e593      0xc5174190
> (gdb)
>
> ====================================
> If run in M-Mode, it will work as below:
>
> RISC-V # exception unaligned
> Unhandled exception: Load address misaligned
> EPC: 000000003ff92fdc RA: 000000003ff93032 TVAL: 000000003ff92fd7
> EPC: 0000000000009fdc RA: 000000000000a032 reloc adjusted
>
> ### ERROR ### Please RESET the board ###
>
> Thanks,
> Rick
>
>> +}
>> +
>>  static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
>>                         char *const argv[])
>>  {
>> @@ -16,6 +27,8 @@ static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,  }
>>
>>  static struct cmd_tbl cmd_sub[] = {
>> +       U_BOOT_CMD_MKENT(unaligned, CONFIG_SYS_MAXARGS, 1, do_unaligned,
>> +                        "", ""),
>>         U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
>>                          "", ""),
>>  };
>> @@ -23,7 +36,8 @@ static struct cmd_tbl cmd_sub[] = {  static char exception_help_text[] =
>>         "<ex>\n"
>>         "  The following exceptions are available:\n"
>> -       "  undefined  - undefined instruction\n"
>> +       "  undefined - illegal instruction\n"
>> +       "  unaligned - load address misaligned\n"
>>         ;
>>
>>  #include <exception.h>
>> --
>> 2.27.0

[PATCH 1/1] cmd: exception: unaligned data access on RISC-V

[Rick Chen]

Hi Heinrich

> >> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de]
> >> Sent: Tuesday, August 04, 2020 7:10 PM
> >> To: Rick Jian-Zhi Chen(???)
> >> Cc: u-boot at lists.denx.de; Heinrich Schuchardt
> >> Subject: [PATCH 1/1] cmd: exception: unaligned data access on RISC-V
> >>
> >> The command 'exception' can be used to test the handling of exceptions.
> >>
> >> Currently the exception command only allows to create an illegal instruction exception on RISC-V.
> >>
> >> Provide a sub-command 'exception unaligned' to cause a misaligned load address exception.
> >>
> >> Adjust the online help for 'exception undefined'.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  cmd/riscv/exception.c | 16 +++++++++++++++-
> >>  1 file changed, 15 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/cmd/riscv/exception.c b/cmd/riscv/exception.c index 3c8dbbec0e..53159531d9 100644
> >> --- a/cmd/riscv/exception.c
> >> +++ b/cmd/riscv/exception.c
> >> @@ -8,6 +8,17 @@
> >>  #include <common.h>
> >>  #include <command.h>
> >>
> >> +static int do_unaligned(struct cmd_tbl *cmdtp, int flag, int argc,
> >> +                       char *const argv[])
> >> +{
> >> +       asm volatile (
> >> +               "auipc a1, 0\n"
> >> +               "ori   a1, a1, 3\n"
> >> +               "lw    a2, (0)(a1)\n"
> >> +       );
> >> +       return CMD_RET_FAILURE;
> >
> > I suggest that we can modify it as below to print the unaligned
> > address and data for more debug information.
> >
> > int ret = 0;
> > int addr = 0;
> >
> > asm volatile (
> > "auipc a1, 0\n"
> > "ori   %0, a1, 3\n"
> > "lw    %1, (0)(a1)\n"
> > : "=r" (addr), "=r" (ret)
> > :
> > : "memory"
> > );
> > printf("unaligned addr 0x%x , ret 0x%x\n",addr,ret);
>
> Thanks for reviewing.
>
> The printf statement will never be reached if the system does not
> support unaligned access. Why should anybody care about the actual
> addresses?
>
> A reasonable message here might be:
>
> printf("The system supports unaligned access.\n");

Yes, this print message without printing the address and data is good for me.
My intention is that print something better than print nothing.
For some junior users they maybe expect that the shell will echo
something when input some commands.

>
> >
> > return CMD_RET_SUCCESS;
> > ====================================
> > So if run in S-Mode, it will work as below:
> >
> > RISC-V # exception unaligned
>
> Do we ever run the U-Boot shell in S-mode? Shouldn't we always drop to
> M-mode before reaching the shell?
>
> If I am in the U-Boot shell, why does S-mode not print out the registers
> like M-Mode does? What is missing?

In S-mode, Opensbi will take care this unaligned exception.
Even in M-mode if the HW support this unaligned access, it will not
trigger this exception. So it shall return CMD_RET_SUCCESS here.

Thanks,
Rick

>
> Best regards
>
> Heinrich
>
> > unaligned addr 0x3ff92fd7 , ret 0x35e59300
> > RISC-V #
> >
> >
> > (gdb) x/4x 0x3ff92fd0
> > 0x3ff92fd0:     0x7ac362ef      0x00000597      0x0035e593      0xc5174190
> > (gdb)
> >
> > ====================================
> > If run in M-Mode, it will work as below:
> >
> > RISC-V # exception unaligned
> > Unhandled exception: Load address misaligned
> > EPC: 000000003ff92fdc RA: 000000003ff93032 TVAL: 000000003ff92fd7
> > EPC: 0000000000009fdc RA: 000000000000a032 reloc adjusted
> >
> > ### ERROR ### Please RESET the board ###
> >
> > Thanks,
> > Rick
> >
> >> +}
> >> +
> >>  static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
> >>                         char *const argv[])
> >>  {
> >> @@ -16,6 +27,8 @@ static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,  }
> >>
> >>  static struct cmd_tbl cmd_sub[] = {
> >> +       U_BOOT_CMD_MKENT(unaligned, CONFIG_SYS_MAXARGS, 1, do_unaligned,
> >> +                        "", ""),
> >>         U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
> >>                          "", ""),
> >>  };
> >> @@ -23,7 +36,8 @@ static struct cmd_tbl cmd_sub[] = {  static char exception_help_text[] =
> >>         "<ex>\n"
> >>         "  The following exceptions are available:\n"
> >> -       "  undefined  - undefined instruction\n"
> >> +       "  undefined - illegal instruction\n"
> >> +       "  unaligned - load address misaligned\n"
> >>         ;
> >>
> >>  #include <exception.h>
> >> --
> >> 2.27.0
>

[PATCH 1/1] cmd: exception: unaligned data access on RISC-V

[Leo Liang]

Hi Heinrich,

On Thu, Aug 06, 2020 at 10:15:28AM +0800, Rick Chen wrote:
> Hi Heinrich
> 
> > >> From: Heinrich Schuchardt [mailto:xypron.glpk at gmx.de]
> > >> Sent: Tuesday, August 04, 2020 7:10 PM
> > >> To: Rick Jian-Zhi Chen(???)
> > >> Cc: u-boot at lists.denx.de; Heinrich Schuchardt
> > >> Subject: [PATCH 1/1] cmd: exception: unaligned data access on RISC-V
> > >>
> > >> The command 'exception' can be used to test the handling of exceptions.
> > >>
> > >> Currently the exception command only allows to create an illegal instruction exception on RISC-V.
> > >>
> > >> Provide a sub-command 'exception unaligned' to cause a misaligned load address exception.
> > >>
> > >> Adjust the online help for 'exception undefined'.
> > >>
> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > >> ---
> > >>  cmd/riscv/exception.c | 16 +++++++++++++++-
> > >>  1 file changed, 15 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/cmd/riscv/exception.c b/cmd/riscv/exception.c index 3c8dbbec0e..53159531d9 100644
> > >> --- a/cmd/riscv/exception.c
> > >> +++ b/cmd/riscv/exception.c
> > >> @@ -8,6 +8,17 @@
> > >>  #include <common.h>
> > >>  #include <command.h>
> > >>
> > >> +static int do_unaligned(struct cmd_tbl *cmdtp, int flag, int argc,
> > >> +                       char *const argv[])
> > >> +{
> > >> +       asm volatile (
> > >> +               "auipc a1, 0\n"
> > >> +               "ori   a1, a1, 3\n"
> > >> +               "lw    a2, (0)(a1)\n"
> > >> +       );
> > >> +       return CMD_RET_FAILURE;
> > >
> > > I suggest that we can modify it as below to print the unaligned
> > > address and data for more debug information.
> > >
> > > int ret = 0;
> > > int addr = 0;
> > >
> > > asm volatile (
> > > "auipc a1, 0\n"
> > > "ori   %0, a1, 3\n"
> > > "lw    %1, (0)(a1)\n"
> > > : "=r" (addr), "=r" (ret)
> > > :
> > > : "memory"
> > > );
> > > printf("unaligned addr 0x%x , ret 0x%x\n",addr,ret);
> >
> > Thanks for reviewing.
> >
> > The printf statement will never be reached if the system does not
> > support unaligned access. Why should anybody care about the actual
> > addresses?
> >
> > A reasonable message here might be:
> >
> > printf("The system supports unaligned access.\n");
> 
> Yes, this print message without printing the address and data is good for me.
> My intention is that print something better than print nothing.
> For some junior users they maybe expect that the shell will echo
> something when input some commands.
> 
> >
> > >
> > > return CMD_RET_SUCCESS;
> > > ====================================
> > > So if run in S-Mode, it will work as below:
> > >
> > > RISC-V # exception unaligned
> >
> > Do we ever run the U-Boot shell in S-mode? Shouldn't we always drop to
> > M-mode before reaching the shell?
> >

In addition to Rick's reponse, I'd like to add that to my knowledge, 
U-Boot-SPL will bring up U-Boot-proper in S-mode, so you will get a shell in S-mode.

> > If I am in the U-Boot shell, why does S-mode not print out the registers
> > like M-Mode does? What is missing?
> 
> In S-mode, Opensbi will take care this unaligned exception.
> Even in M-mode if the HW support this unaligned access, it will not
> trigger this exception. So it shall return CMD_RET_SUCCESS here.

And as explained, this S-mode exception will be taken care of by M-mode runtime (OpenSBI),
so the info of the registers will not be printed out.
Concluded that I think nothing is missing here.

Best regards,
Leo

> 
> Thanks,
> Rick
> 
> >
> > Best regards
> >
> > Heinrich
> >
> > > unaligned addr 0x3ff92fd7 , ret 0x35e59300
> > > RISC-V #
> > >
> > >
> > > (gdb) x/4x 0x3ff92fd0
> > > 0x3ff92fd0:     0x7ac362ef      0x00000597      0x0035e593      0xc5174190
> > > (gdb)
> > >
> > > ====================================
> > > If run in M-Mode, it will work as below:
> > >
> > > RISC-V # exception unaligned
> > > Unhandled exception: Load address misaligned
> > > EPC: 000000003ff92fdc RA: 000000003ff93032 TVAL: 000000003ff92fd7
> > > EPC: 0000000000009fdc RA: 000000000000a032 reloc adjusted
> > >
> > > ### ERROR ### Please RESET the board ###
> > >
> > > Thanks,
> > > Rick
> > >
> > >> +}
> > >> +
> > >>  static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,
> > >>                         char *const argv[])
> > >>  {
> > >> @@ -16,6 +27,8 @@ static int do_undefined(struct cmd_tbl *cmdtp, int flag, int argc,  }
> > >>
> > >>  static struct cmd_tbl cmd_sub[] = {
> > >> +       U_BOOT_CMD_MKENT(unaligned, CONFIG_SYS_MAXARGS, 1, do_unaligned,
> > >> +                        "", ""),
> > >>         U_BOOT_CMD_MKENT(undefined, CONFIG_SYS_MAXARGS, 1, do_undefined,
> > >>                          "", ""),
> > >>  };
> > >> @@ -23,7 +36,8 @@ static struct cmd_tbl cmd_sub[] = {  static char exception_help_text[] =
> > >>         "<ex>\n"
> > >>         "  The following exceptions are available:\n"
> > >> -       "  undefined  - undefined instruction\n"
> > >> +       "  undefined - illegal instruction\n"
> > >> +       "  unaligned - load address misaligned\n"
> > >>         ;
> > >>
> > >>  #include <exception.h>
> > >> --
> > >> 2.27.0
> >