Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality

[Palmer Dabbelt]

On Mon, 22 Jul 2019 22:30:15 PDT (-0700), address@hidden wrote:
> Hi Palmer,
>
> On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <address@hidden> wrote:
> > On Fri, 14 Jun 2019 08:15:51 PDT (-0700), address@hidden wrote:
> > > This adds a reset opcode for sifive_test device to trigger a system
> > > reset for testing purpose.
> > >
> > > Signed-off-by: Bin Meng <address@hidden>
> > > ---
> > >
> > >  hw/riscv/sifive_test.c         | 4 ++++
> > >  include/hw/riscv/sifive_test.h | 3 ++-
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/riscv/sifive_test.c b/hw/riscv/sifive_test.c
> > > index 24a04d7..cd86831 100644
> > > --- a/hw/riscv/sifive_test.c
> > > +++ b/hw/riscv/sifive_test.c
> > > @@ -21,6 +21,7 @@
> > >  #include "qemu/osdep.h"
> > >  #include "hw/sysbus.h"
> > >  #include "qemu/module.h"
> > > +#include "sysemu/sysemu.h"
> > >  #include "target/riscv/cpu.h"
> > >  #include "hw/riscv/sifive_test.h"
> > >
> > > @@ -40,6 +41,9 @@ static void sifive_test_write(void *opaque, hwaddr addr,
> > >              exit(code);
> > >          case FINISHER_PASS:
> > >              exit(0);
> > > +        case FINISHER_RESET:
> > > +            qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > > +            return;
> > >          default:
> > >              break;
> > >          }
> > > diff --git a/include/hw/riscv/sifive_test.h b/include/hw/riscv/sifive_test.h
> > > index 71d4c9f..c186a31 100644
> > > --- a/include/hw/riscv/sifive_test.h
> > > +++ b/include/hw/riscv/sifive_test.h
> > > @@ -34,7 +34,8 @@ typedef struct SiFiveTestState {
> > >
> > >  enum {
> > >      FINISHER_FAIL = 0x3333,
> > > -    FINISHER_PASS = 0x5555
> > > +    FINISHER_PASS = 0x5555,
> > > +    FINISHER_RESET = 0x7777
> > >  };
> > >
> > >  DeviceState *sifive_test_create(hwaddr addr);
> >
> > Reviewed-by: Palmer Dabbelt <address@hidden>
>
> Thanks a lot!
>
> > Sorry this took a while, but it's in the hardware now.  I'll merge this, but
> > I'm considering it a new feature so it'll be held off a bit.
>
> "but it's in the hardware now", do you mean the code I added (0x7777)
> is now supported by a newer version SiFive test device with compatible
> string "sifive,test1", and can actually do the system wide reset?

No, the hardware is still a "sifive,test0" as plumbing through the reset is
trickier than I wanted to take on.  I just reserved the 0x7777 code and
implemented it by triggering an unsupported function error, so we don't
accidentally use it for something else later.