qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Bin Meng
Subject: Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
Date: Wed, 26 Jun 2019 09:45:46 +0800

Hi Palmer,

On Sun, Jun 23, 2019 at 10:40 PM Palmer Dabbelt <address@hidden> wrote:
>
> On Thu, 20 Jun 2019 22:40:24 PDT (-0700), address@hidden wrote:
> > Hi Palmer,
> >
> > On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <address@hidden> wrote:
> >>
> >> 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.
> >
> > Thanks for the clarification. The main reason of adding this
> > functionality is to provide software a way of rebooting the whole
> > system. Please provide update after you consult SiFive hardware guys
> > :)
>
> Ya, it makes sense.  My only worry here is that we have some way of doing this
> already, in which case I'd just want to make sure it matches.

If the SiFive chipset already has the functionality to do the reset
via this test module, I agree let's align the number to what hardware
actually accepts.

Regards,
Bin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]