diff mbox series

[2/2] aspeed/scu: Implement power off register

Message ID 20181211031044.27628-3-joel@jms.id.au
State New
Headers show
Series arm: aspeed: Allow the guest to exit | expand

Commit Message

Joel Stanley Dec. 11, 2018, 3:10 a.m. UTC
This register does not exist in hardware. It is here to allow the guest
code to cause Qemu to exit when required.

The register address chosen is unused in the emulated machines
datasheets.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/misc/aspeed_scu.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Cédric Le Goater Dec. 11, 2018, 7:37 a.m. UTC | #1
On 12/11/18 4:10 AM, Joel Stanley wrote:
> This register does not exist in hardware. It is here to allow the guest
> code to cause Qemu to exit when required.
> 
> The register address chosen is unused in the emulated machines
> datasheets.

yes. I checked also. 

On the AST2600, 0x1A0 is now in the CPU scratch register range,
but I don't think this is problem.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.



> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  hw/misc/aspeed_scu.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c8217740efc1..aa17d032ba93 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,6 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> +#include "sysemu/sysemu.h"
>  #include "crypto/random.h"
>  #include "trace.h"
>  
> @@ -84,6 +85,7 @@
>  #define SRAM_DECODE_BASE1    TO_REG(0x194)
>  #define SRAM_DECODE_BASE2    TO_REG(0x198)
>  #define BMC_REV              TO_REG(0x19C)
> +#define POWEROFF             TO_REG(0x1A0)
>  #define BMC_DEV_ID           TO_REG(0x1A4)
>  
>  #define SCU_IO_REGION_SIZE 0x1000
> @@ -264,6 +266,9 @@ static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
>          }
>          /* Avoid assignment below, we've handled everything */
>          return;
> +    case POWEROFF:
> +        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +        break;
>      case FREQ_CNTR_EVAL:
>      case VGA_SCRATCH1 ... VGA_SCRATCH8:
>      case RNG_DATA:
>
Peter Maydell Jan. 3, 2019, 4:26 p.m. UTC | #2
On Tue, 11 Dec 2018 at 03:11, Joel Stanley <joel@jms.id.au> wrote:
>
> This register does not exist in hardware. It is here to allow the guest
> code to cause Qemu to exit when required.
>
> The register address chosen is unused in the emulated machines
> datasheets.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  hw/misc/aspeed_scu.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index c8217740efc1..aa17d032ba93 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -16,6 +16,7 @@
>  #include "qapi/visitor.h"
>  #include "qemu/bitops.h"
>  #include "qemu/log.h"
> +#include "sysemu/sysemu.h"
>  #include "crypto/random.h"
>  #include "trace.h"
>
> @@ -84,6 +85,7 @@
>  #define SRAM_DECODE_BASE1    TO_REG(0x194)
>  #define SRAM_DECODE_BASE2    TO_REG(0x198)
>  #define BMC_REV              TO_REG(0x19C)
> +#define POWEROFF             TO_REG(0x1A0)
>  #define BMC_DEV_ID           TO_REG(0x1A4)

I'm always a bit dubious about adding things to QEMU devices
which don't exist in the real hardware we're emulating. If we
do want to do that, I think we should clearly flag them up as
being QEMU-specific with suitable comments and naming of
the #define, etc.

thanks
-- PMM
Joel Stanley Jan. 24, 2019, 8:18 p.m. UTC | #3
On Fri, 4 Jan 2019 at 03:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 11 Dec 2018 at 03:11, Joel Stanley <joel@jms.id.au> wrote:
> >
> > This register does not exist in hardware. It is here to allow the guest
> > code to cause Qemu to exit when required.
> >
> > The register address chosen is unused in the emulated machines
> > datasheets.

> I'm always a bit dubious about adding things to QEMU devices
> which don't exist in the real hardware we're emulating. If we
> do want to do that, I think we should clearly flag them up as
> being QEMU-specific with suitable comments and naming of
> the #define, etc.

Since writing this patch I was made aware of -no-reboot. That flag
solves the problem I had so we can drop these patches for now.

Cheers,

Joel
diff mbox series

Patch

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index c8217740efc1..aa17d032ba93 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -16,6 +16,7 @@ 
 #include "qapi/visitor.h"
 #include "qemu/bitops.h"
 #include "qemu/log.h"
+#include "sysemu/sysemu.h"
 #include "crypto/random.h"
 #include "trace.h"
 
@@ -84,6 +85,7 @@ 
 #define SRAM_DECODE_BASE1    TO_REG(0x194)
 #define SRAM_DECODE_BASE2    TO_REG(0x198)
 #define BMC_REV              TO_REG(0x19C)
+#define POWEROFF             TO_REG(0x1A0)
 #define BMC_DEV_ID           TO_REG(0x1A4)
 
 #define SCU_IO_REGION_SIZE 0x1000
@@ -264,6 +266,9 @@  static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
         }
         /* Avoid assignment below, we've handled everything */
         return;
+    case POWEROFF:
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+        break;
     case FREQ_CNTR_EVAL:
     case VGA_SCRATCH1 ... VGA_SCRATCH8:
     case RNG_DATA: