| Message ID | 1560525351-590-1-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: sifive_test: Add reset functionality | expand |
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 > >
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
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.
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
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.
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
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!
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
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!
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
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.
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
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);
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(-)