[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 at 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
>