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

[Leo Liang <ycliang@andestech.com>]

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