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

[Palmer Dabbelt]

On Wed, 19 Jun 2019 06:42:21 PDT (-0700), address@hidden wrote:
> Hi Alistair,
>
> On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <address@hidden> wrote:
> > On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <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
> >
> > Do you mind sharing where you got this value from? I can't find
> > details on this device in the SiFive manuals.
>
> I don't think this is a device that actually exists on SiFive's
> chipset. It's hypothetical.

The device actually does exist in the hardware, but that's just an
implementation quirk.  Essentially what's going on here is that the RTL
contains this device, which has a register and then a behavioral verilog block
that causes simulations to terminate.  This is how we exit from tests in RTL
simulation, and we've just gone ahead and implemented the same device in QEMU
in order to make it easy to have compatibility with those bare-metal tests.
Due to how our design flow is set up we end up with exactly the same block in
the ASIC.  The register is still there, but the behavioral code to exit
simulations doesn't do anything so it's essentially just a useless device.
Since it's useless we don't bother writing it up in the ASIC documentation, but
it should be in the RTL documentation.

I'm not opposed to extending the interface in the suggested fashion, but I
wanted to check with the hardware team first to see if they're doing anything
with the other numbers.  I'm out of the office (and somewhat backed up on code
review) until July, so it might take a bit to dig through this.