[Qemu-devel] [PATCH 0/2] arm: aspeed: Allow the guest to exit

[Qemu-devel] [PATCH 0/2] arm: aspeed: Allow the guest to exit

[Joel Stanley]

This series adds a feature to the ASPEED machine that allows the guest
to cause itself to exit. It was tested with romulus-bmc and palmetto-bmc
with a Linux kernel as the guest.

To test:

 cd linux
 $ export ARCH=arm CROSS_COMPILE=arm-linux-gnueabi-
 $ make aspeed_g4_defconfig
 $ ./scripts/config -e CONFIG_POWER_RESET_SYSCON_POWEROFF -e CONFIG_POWER_RESET
 $ make -j$(nproc)
 $ qemu-system-arm -M palmetto-bmc -nographic \
    -kernel arch/arm/boot/zImage \
    -dtb arch/arm/boot/dts/aspeed-bmc-opp-palmetto.dtb \
    -initrd ~/buildroot-arm.cpio.xz

Joel Stanley (2):
  aspeed: Add syscon-poweroff to guest device tree
  aspeed/scu: Implement power off register

 hw/arm/aspeed.c      | 33 +++++++++++++++++++++++++++++++++
 hw/misc/aspeed_scu.c |  5 +++++
 2 files changed, 38 insertions(+)

-- 
2.19.1

[Qemu-devel] [PATCH 1/2] aspeed: Add syscon-poweroff to guest device tree

[Joel Stanley]

This adds a node to the guest's device tree that allows it to cause Qemu
to exit when the guest shuts down.

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

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 515898548284..00060d44ad51 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -21,8 +21,10 @@
 #include "hw/i2c/smbus.h"
 #include "qemu/log.h"
 #include "sysemu/block-backend.h"
+#include "sysemu/device_tree.h"
 #include "hw/loader.h"
 #include "qemu/error-report.h"
+#include <libfdt.h>
 
 static struct arm_boot_info aspeed_board_binfo = {
     .board_id = -1, /* device-tree-only board */
@@ -126,6 +128,36 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
     g_free(storage);
 }
 
+static void fdt_add_shutdown_node(void *fdt)
+{
+    const char *nodename = "/syscon-poweroff";
+    uint32_t phandle;
+    int offset;
+
+    /* Find the scu phandle */
+    offset = fdt_path_offset(fdt, "/ahb/apb/syscon@1e6e2000");
+    if (offset < 0) {
+        error_report("%s couldn't find syscon, guest shutdown unavailable: %s",
+                     __func__, fdt_strerror(offset));
+        return;
+    }
+    phandle = fdt_get_phandle(fdt, offset);
+
+    /* Add syscon-poweroff node and use 0x1A0, an un-used SCU register */
+    qemu_fdt_add_subnode(fdt, nodename);
+    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
+    qemu_fdt_setprop_cells(fdt, nodename, "regmap", phandle);
+    qemu_fdt_setprop_cells(fdt, nodename, "offset", 0x1A0);
+    qemu_fdt_setprop_cells(fdt, nodename, "value", 1);
+}
+
+static void aspeed_board_modify_dtb(const struct arm_boot_info *binfo,
+                                    void *fdt)
+{
+    fdt_add_shutdown_node(fdt);
+}
+
+
 static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
                                       Error **errp)
 {
@@ -228,6 +260,7 @@ static void aspeed_board_init(MachineState *machine,
     aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
     aspeed_board_binfo.ram_size = ram_size;
     aspeed_board_binfo.loader_start = sc->info->sdram_base;
+    aspeed_board_binfo.modify_dtb = aspeed_board_modify_dtb;
 
     if (cfg->i2c_init) {
         cfg->i2c_init(bmc);
-- 
2.19.1

Re: [Qemu-devel] [PATCH 1/2] aspeed: Add syscon-poweroff to guest device tree

[Cédric Le Goater]

On 12/11/18 4:10 AM, Joel Stanley wrote:
> This adds a node to the guest's device tree that allows it to cause Qemu
> to exit when the guest shuts down.

Do you think we could find a way to add the same node under U-Boot for 
tests using flash images ? or could we just add the node in DT ? Would
that be a problem on real system ?      


Nevertheless, this is fine.

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

Thanks,

C.


> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  hw/arm/aspeed.c | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index 515898548284..00060d44ad51 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -21,8 +21,10 @@
>  #include "hw/i2c/smbus.h"
>  #include "qemu/log.h"
>  #include "sysemu/block-backend.h"
> +#include "sysemu/device_tree.h"
>  #include "hw/loader.h"
>  #include "qemu/error-report.h"
> +#include <libfdt.h>
>  
>  static struct arm_boot_info aspeed_board_binfo = {
>      .board_id = -1, /* device-tree-only board */
> @@ -126,6 +128,36 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size,
>      g_free(storage);
>  }
>  
> +static void fdt_add_shutdown_node(void *fdt)
> +{
> +    const char *nodename = "/syscon-poweroff";
> +    uint32_t phandle;
> +    int offset;
> +
> +    /* Find the scu phandle */
> +    offset = fdt_path_offset(fdt, "/ahb/apb/syscon@1e6e2000");
> +    if (offset < 0) {
> +        error_report("%s couldn't find syscon, guest shutdown unavailable: %s",
> +                     __func__, fdt_strerror(offset));
> +        return;
> +    }
> +    phandle = fdt_get_phandle(fdt, offset);
> +
> +    /* Add syscon-poweroff node and use 0x1A0, an un-used SCU register */
> +    qemu_fdt_add_subnode(fdt, nodename);
> +    qemu_fdt_setprop_string(fdt, nodename, "compatible", "syscon-poweroff");
> +    qemu_fdt_setprop_cells(fdt, nodename, "regmap", phandle);
> +    qemu_fdt_setprop_cells(fdt, nodename, "offset", 0x1A0);
> +    qemu_fdt_setprop_cells(fdt, nodename, "value", 1);
> +}
> +
> +static void aspeed_board_modify_dtb(const struct arm_boot_info *binfo,
> +                                    void *fdt)
> +{
> +    fdt_add_shutdown_node(fdt);
> +}
> +
> +
>  static void aspeed_board_init_flashes(AspeedSMCState *s, const char *flashtype,
>                                        Error **errp)
>  {
> @@ -228,6 +260,7 @@ static void aspeed_board_init(MachineState *machine,
>      aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
>      aspeed_board_binfo.ram_size = ram_size;
>      aspeed_board_binfo.loader_start = sc->info->sdram_base;
> +    aspeed_board_binfo.modify_dtb = aspeed_board_modify_dtb;
>  
>      if (cfg->i2c_init) {
>          cfg->i2c_init(bmc);
> 

[Qemu-devel] [PATCH 2/2] aspeed/scu: Implement power off register

[Joel Stanley]

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)
 
 #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:
-- 
2.19.1

Re: [Qemu-devel] [PATCH 2/2] aspeed/scu: Implement power off register

[Cédric Le Goater]

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:
> 

Re: [Qemu-devel] [PATCH 2/2] aspeed/scu: Implement power off register

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH 2/2] aspeed/scu: Implement power off register

[Joel Stanley]

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