All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-14 15:15 ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-14 15:15 UTC (permalink / raw)
  To: qemu-riscv, qemu-devel, Alistair Francis, Palmer Dabbelt

This adds a reset opcode for sifive_test device to trigger a system
reset for testing purpose.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 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);
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-14 15:15 ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-14 15:15 UTC (permalink / raw)
  To: qemu-riscv, qemu-devel, Alistair Francis, Palmer Dabbelt

This adds a reset opcode for sifive_test device to trigger a system
reset for testing purpose.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---

 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);
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-14 15:15 ` [Qemu-riscv] " Bin Meng
@ 2019-06-17 17:12   ` Alistair Francis
  -1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2019-06-17 17:12 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> This adds a reset opcode for sifive_test device to trigger a system
> reset for testing purpose.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  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.

Alistair

>  };
>
>  DeviceState *sifive_test_create(hwaddr addr);
> --
> 2.7.4
>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-17 17:12   ` Alistair Francis
  0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2019-06-17 17:12 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, qemu-devel@nongnu.org Developers,
	Alistair Francis, Palmer Dabbelt

On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> This adds a reset opcode for sifive_test device to trigger a system
> reset for testing purpose.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  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.

Alistair

>  };
>
>  DeviceState *sifive_test_create(hwaddr addr);
> --
> 2.7.4
>
>


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-17 17:12   ` [Qemu-riscv] " Alistair Francis
@ 2019-06-19 13:42     ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-19 13:42 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers

Hi Alistair,

On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > This adds a reset opcode for sifive_test device to trigger a system
> > reset for testing purpose.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  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.

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-19 13:42     ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-19 13:42 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, qemu-devel@nongnu.org Developers,
	Alistair Francis, Palmer Dabbelt

Hi Alistair,

On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > This adds a reset opcode for sifive_test device to trigger a system
> > reset for testing purpose.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  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.

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-19 13:42     ` [Qemu-riscv] " Bin Meng
@ 2019-06-21  2:53       ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-06-21  2:53 UTC (permalink / raw)
  To: bmeng.cn; +Cc: alistair23, Alistair Francis, qemu-riscv, qemu-devel

On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi Alistair,
>
> On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>> >
>> > This adds a reset opcode for sifive_test device to trigger a system
>> > reset for testing purpose.
>> >
>> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> > ---
>> >
>> >  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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-21  2:53       ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-06-21  2:53 UTC (permalink / raw)
  To: bmeng.cn; +Cc: alistair23, qemu-riscv, qemu-devel, Alistair Francis

On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi Alistair,
>
> On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
>>
>> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>> >
>> > This adds a reset opcode for sifive_test device to trigger a system
>> > reset for testing purpose.
>> >
>> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> > ---
>> >
>> >  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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-21  2:53       ` [Qemu-riscv] " Palmer Dabbelt
@ 2019-06-21  5:40         ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-21  5:40 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers

Hi Palmer,

On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Alistair,
> >
> > On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >> >
> >> > This adds a reset opcode for sifive_test device to trigger a system
> >> > reset for testing purpose.
> >> >
> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> > ---
> >> >
> >> >  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
:)

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-21  5:40         ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-21  5:40 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

Hi Palmer,

On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Alistair,
> >
> > On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
> >>
> >> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >> >
> >> > This adds a reset opcode for sifive_test device to trigger a system
> >> > reset for testing purpose.
> >> >
> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> > ---
> >> >
> >> >  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
:)

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-21  5:40         ` [Qemu-riscv] " Bin Meng
@ 2019-06-23 14:40           ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-06-23 14:40 UTC (permalink / raw)
  To: bmeng.cn; +Cc: alistair23, Alistair Francis, qemu-riscv, qemu-devel

On Thu, 20 Jun 2019 22:40:24 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi Palmer,
>
> On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
>> > Hi Alistair,
>> >
>> > On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
>> >>
>> >> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>> >> >
>> >> > This adds a reset opcode for sifive_test device to trigger a system
>> >> > reset for testing purpose.
>> >> >
>> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> >> > ---
>> >> >
>> >> >  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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-23 14:40           ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-06-23 14:40 UTC (permalink / raw)
  To: bmeng.cn; +Cc: alistair23, qemu-riscv, qemu-devel, Alistair Francis

On Thu, 20 Jun 2019 22:40:24 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi Palmer,
>
> On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
>> > Hi Alistair,
>> >
>> > On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
>> >>
>> >> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>> >> >
>> >> > This adds a reset opcode for sifive_test device to trigger a system
>> >> > reset for testing purpose.
>> >> >
>> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> >> > ---
>> >> >
>> >> >  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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-23 14:40           ` [Qemu-riscv] " Palmer Dabbelt
@ 2019-06-26  1:45             ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-26  1:45 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers

Hi Palmer,

On Sun, Jun 23, 2019 at 10:40 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Thu, 20 Jun 2019 22:40:24 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Palmer,
> >
> > On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
> >>
> >> On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> >> > Hi Alistair,
> >> >
> >> > On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
> >> >>
> >> >> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >> >> >
> >> >> > This adds a reset opcode for sifive_test device to trigger a system
> >> >> > reset for testing purpose.
> >> >> >
> >> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> >> > ---
> >> >> >
> >> >> >  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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-06-26  1:45             ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-06-26  1:45 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

Hi Palmer,

On Sun, Jun 23, 2019 at 10:40 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Thu, 20 Jun 2019 22:40:24 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Palmer,
> >
> > On Fri, Jun 21, 2019 at 10:53 AM Palmer Dabbelt <palmer@sifive.com> wrote:
> >>
> >> On Wed, 19 Jun 2019 06:42:21 PDT (-0700), bmeng.cn@gmail.com wrote:
> >> > Hi Alistair,
> >> >
> >> > On Tue, Jun 18, 2019 at 1:15 AM Alistair Francis <alistair23@gmail.com> wrote:
> >> >>
> >> >> On Fri, Jun 14, 2019 at 8:30 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >> >> >
> >> >> > This adds a reset opcode for sifive_test device to trigger a system
> >> >> > reset for testing purpose.
> >> >> >
> >> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> >> > ---
> >> >> >
> >> >> >  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


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-14 15:15 ` [Qemu-riscv] " Bin Meng
@ 2019-07-09  9:48   ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-07-09  9:48 UTC (permalink / raw)
  To: bmeng.cn; +Cc: Alistair Francis, qemu-riscv, qemu-devel

On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> This adds a reset opcode for sifive_test device to trigger a system
> reset for testing purpose.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  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);

Poking around the hardware side, it looks like we're silently ignoring all
codes aside from 0x3333 and 0x5555.  As a result there's really no way to test
if this added reset is going to work, so this should be a "sifive,test1"
instead of a "sifive,test0".  It's backwards compatible, so the compat string
in the device trees should look something like

    compatible = "sifive,test1", "sifive,test0";

With that it should be fine, pending the change to our hardware to reserve this
as an error code within "sifive,test0" devices.

Do you mind also submitting a device tree binding to Linux that describes this
device?  That way we can all stay on the same page about what the pair of
interfaces do.

Thanks!


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-07-09  9:48   ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-07-09  9:48 UTC (permalink / raw)
  To: bmeng.cn; +Cc: qemu-riscv, qemu-devel, Alistair Francis

On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> This adds a reset opcode for sifive_test device to trigger a system
> reset for testing purpose.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  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);

Poking around the hardware side, it looks like we're silently ignoring all
codes aside from 0x3333 and 0x5555.  As a result there's really no way to test
if this added reset is going to work, so this should be a "sifive,test1"
instead of a "sifive,test0".  It's backwards compatible, so the compat string
in the device trees should look something like

    compatible = "sifive,test1", "sifive,test0";

With that it should be fine, pending the change to our hardware to reserve this
as an error code within "sifive,test0" devices.

Do you mind also submitting a device tree binding to Linux that describes this
device?  That way we can all stay on the same page about what the pair of
interfaces do.

Thanks!


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-07-09  9:48   ` [Qemu-riscv] " Palmer Dabbelt
@ 2019-07-14  3:38     ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-07-14  3:38 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers

On Tue, Jul 9, 2019 at 5:48 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> > This adds a reset opcode for sifive_test device to trigger a system
> > reset for testing purpose.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  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);
>
> Poking around the hardware side, it looks like we're silently ignoring all
> codes aside from 0x3333 and 0x5555.  As a result there's really no way to test
> if this added reset is going to work, so this should be a "sifive,test1"
> instead of a "sifive,test0".  It's backwards compatible, so the compat string
> in the device trees should look something like
>
>     compatible = "sifive,test1", "sifive,test0";
>
> With that it should be fine, pending the change to our hardware to reserve this
> as an error code within "sifive,test0" devices.
>
> Do you mind also submitting a device tree binding to Linux that describes this
> device?  That way we can all stay on the same page about what the pair of
> interfaces do.

Thanks for the confirmation. I will try to create a device tree
binding for Linux.

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-07-14  3:38     ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-07-14  3:38 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Alistair Francis

On Tue, Jul 9, 2019 at 5:48 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> > This adds a reset opcode for sifive_test device to trigger a system
> > reset for testing purpose.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  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);
>
> Poking around the hardware side, it looks like we're silently ignoring all
> codes aside from 0x3333 and 0x5555.  As a result there's really no way to test
> if this added reset is going to work, so this should be a "sifive,test1"
> instead of a "sifive,test0".  It's backwards compatible, so the compat string
> in the device trees should look something like
>
>     compatible = "sifive,test1", "sifive,test0";
>
> With that it should be fine, pending the change to our hardware to reserve this
> as an error code within "sifive,test0" devices.
>
> Do you mind also submitting a device tree binding to Linux that describes this
> device?  That way we can all stay on the same page about what the pair of
> interfaces do.

Thanks for the confirmation. I will try to create a device tree
binding for Linux.

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-06-14 15:15 ` [Qemu-riscv] " Bin Meng
@ 2019-07-20  1:47   ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-07-20  1:47 UTC (permalink / raw)
  To: bmeng.cn; +Cc: Alistair Francis, qemu-riscv, qemu-devel

On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> This adds a reset opcode for sifive_test device to trigger a system
> reset for testing purpose.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  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 <palmer@sifive.com>

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.

Thanks!


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-07-20  1:47   ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-07-20  1:47 UTC (permalink / raw)
  To: bmeng.cn; +Cc: qemu-riscv, qemu-devel, Alistair Francis

On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> This adds a reset opcode for sifive_test device to trigger a system
> reset for testing purpose.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
>  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 <palmer@sifive.com>

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.

Thanks!


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-07-20  1:47   ` [Qemu-riscv] " Palmer Dabbelt
@ 2019-07-23  5:30     ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-07-23  5:30 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers

Hi Palmer,

On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> > This adds a reset opcode for sifive_test device to trigger a system
> > reset for testing purpose.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  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 <palmer@sifive.com>

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?

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-07-23  5:30     ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-07-23  5:30 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Alistair Francis

Hi Palmer,

On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> > This adds a reset opcode for sifive_test device to trigger a system
> > reset for testing purpose.
> >
> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> > ---
> >
> >  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 <palmer@sifive.com>

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?

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-07-23  5:30     ` [Qemu-riscv] " Bin Meng
@ 2019-07-29 19:47       ` Palmer Dabbelt
  -1 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-07-29 19:47 UTC (permalink / raw)
  To: bmeng.cn; +Cc: Alistair Francis, qemu-riscv, qemu-devel

On Mon, 22 Jul 2019 22:30:15 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi Palmer,
>
> On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
>> > This adds a reset opcode for sifive_test device to trigger a system
>> > reset for testing purpose.
>> >
>> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> > ---
>> >
>> >  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 <palmer@sifive.com>
>
> 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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-07-29 19:47       ` Palmer Dabbelt
  0 siblings, 0 replies; 26+ messages in thread
From: Palmer Dabbelt @ 2019-07-29 19:47 UTC (permalink / raw)
  To: bmeng.cn; +Cc: qemu-riscv, qemu-devel, Alistair Francis

On Mon, 22 Jul 2019 22:30:15 PDT (-0700), bmeng.cn@gmail.com wrote:
> Hi Palmer,
>
> On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
>> > This adds a reset opcode for sifive_test device to trigger a system
>> > reset for testing purpose.
>> >
>> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>> > ---
>> >
>> >  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 <palmer@sifive.com>
>
> 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.


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality
  2019-07-29 19:47       ` [Qemu-riscv] " Palmer Dabbelt
@ 2019-07-30 16:37         ` Bin Meng
  -1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-07-30 16:37 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alistair Francis, open list:RISC-V, qemu-devel@nongnu.org Developers

On Tue, Jul 30, 2019 at 3:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 22 Jul 2019 22:30:15 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Palmer,
> >
> > On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
> >>
> >> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> >> > This adds a reset opcode for sifive_test device to trigger a system
> >> > reset for testing purpose.
> >> >
> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> > ---
> >> >
> >> >  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 <palmer@sifive.com>
> >
> > 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.

OK, thanks for the clarifications!

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [Qemu-riscv] [PATCH] riscv: sifive_test: Add reset functionality
@ 2019-07-30 16:37         ` Bin Meng
  0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2019-07-30 16:37 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: open list:RISC-V, qemu-devel@nongnu.org Developers, Alistair Francis

On Tue, Jul 30, 2019 at 3:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Mon, 22 Jul 2019 22:30:15 PDT (-0700), bmeng.cn@gmail.com wrote:
> > Hi Palmer,
> >
> > On Sat, Jul 20, 2019 at 9:47 AM Palmer Dabbelt <palmer@sifive.com> wrote:
> >>
> >> On Fri, 14 Jun 2019 08:15:51 PDT (-0700), bmeng.cn@gmail.com wrote:
> >> > This adds a reset opcode for sifive_test device to trigger a system
> >> > reset for testing purpose.
> >> >
> >> > Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> >> > ---
> >> >
> >> >  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 <palmer@sifive.com>
> >
> > 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.

OK, thanks for the clarifications!

Regards,
Bin


^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2019-07-30 16:38 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-14 15:15 [Qemu-devel] [PATCH] riscv: sifive_test: Add reset functionality Bin Meng
2019-06-14 15:15 ` [Qemu-riscv] " Bin Meng
2019-06-17 17:12 ` [Qemu-devel] " Alistair Francis
2019-06-17 17:12   ` [Qemu-riscv] " Alistair Francis
2019-06-19 13:42   ` Bin Meng
2019-06-19 13:42     ` [Qemu-riscv] " Bin Meng
2019-06-21  2:53     ` Palmer Dabbelt
2019-06-21  2:53       ` [Qemu-riscv] " Palmer Dabbelt
2019-06-21  5:40       ` Bin Meng
2019-06-21  5:40         ` [Qemu-riscv] " Bin Meng
2019-06-23 14:40         ` Palmer Dabbelt
2019-06-23 14:40           ` [Qemu-riscv] " Palmer Dabbelt
2019-06-26  1:45           ` Bin Meng
2019-06-26  1:45             ` [Qemu-riscv] " Bin Meng
2019-07-09  9:48 ` Palmer Dabbelt
2019-07-09  9:48   ` [Qemu-riscv] " Palmer Dabbelt
2019-07-14  3:38   ` [Qemu-devel] " Bin Meng
2019-07-14  3:38     ` [Qemu-riscv] " Bin Meng
2019-07-20  1:47 ` [Qemu-devel] " Palmer Dabbelt
2019-07-20  1:47   ` [Qemu-riscv] " Palmer Dabbelt
2019-07-23  5:30   ` [Qemu-devel] " Bin Meng
2019-07-23  5:30     ` [Qemu-riscv] " Bin Meng
2019-07-29 19:47     ` [Qemu-devel] " Palmer Dabbelt
2019-07-29 19:47       ` [Qemu-riscv] " Palmer Dabbelt
2019-07-30 16:37       ` [Qemu-devel] " Bin Meng
2019-07-30 16:37         ` [Qemu-riscv] " Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.