[Qemu-devel] [PATCH 0/3] hw/arm/exynos: Fix poweroff

[Qemu-devel] [PATCH 0/3] hw/arm/exynos: Fix poweroff

[Krzysztof Kozlowski]

Hi,

Beside two minor cleanups, the patchset implements proper Exynos power off.
Still the GIC [1] fix is needed because the kernel will not reach power off
handlers without it.

[1] http://patchwork.ozlabs.org/patch/738320/

Best regards,
Krzysztof


Krzysztof Kozlowski (3):
  hw/arm/exynos: Use type define instead of hard-coded a9mpcore_priv
    string
  hw/intc/exynos: Constify array of combiner interrupts
  hw/arm/exynos: Add support for system poweroff

 hw/arm/exynos4210.c      |  3 ++-
 hw/intc/exynos4210_gic.c |  2 +-
 hw/misc/exynos4210_pmu.c | 14 ++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH 1/3] hw/arm/exynos: Use type define instead of hard-coded a9mpcore_priv string

[Krzysztof Kozlowski]

Use a define for a9mpcore_priv device type name instead of hard-coded
string.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 hw/arm/exynos4210.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 034fc8be9d76..a9e221c5b7fe 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -26,6 +26,7 @@
 #include "qemu-common.h"
 #include "qemu/log.h"
 #include "cpu.h"
+#include "hw/cpu/a9mpcore.h"
 #include "hw/boards.h"
 #include "sysemu/sysemu.h"
 #include "hw/sysbus.h"
@@ -212,7 +213,7 @@ static void exynos4210_init(Object *obj)
     }
 
     /* Private memory region and Internal GIC */
-    dev = qdev_create(NULL, "a9mpcore_priv");
+    dev = qdev_create(NULL, TYPE_A9MPCORE_PRIV);
     qdev_prop_set_uint32(dev, "num-cpu", EXYNOS4210_NCPUS);
     qdev_init_nofail(dev);
     busdev = SYS_BUS_DEVICE(dev);
-- 
2.9.3

[Qemu-devel] [PATCH 2/3] hw/intc/exynos: Constify array of combiner interrupts

[Krzysztof Kozlowski]

The static array of interrupt combiner mappings is not modified so it
can be made const for code safeness.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 hw/intc/exynos4210_gic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
index 222cfd6c6387..636dc6dbc1f9 100644
--- a/hw/intc/exynos4210_gic.c
+++ b/hw/intc/exynos4210_gic.c
@@ -116,7 +116,7 @@ enum ExtInt {
  * which is INTG16 in Internal Interrupt Combiner.
  */
 
-static uint32_t
+static const uint32_t
 combiner_grp_to_gic_id[64-EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
     /* int combiner groups 16-19 */
     { }, { }, { }, { },
-- 
2.9.3

[Qemu-devel] [PATCH 3/3] hw/arm/exynos: Add support for system poweroff

[Krzysztof Kozlowski]

On all Exynos-based boards, the system powers down itself by driving
PS_HOLD signal low.  Handle writing to respective PMU register to fix
power off failure:

    reboot: Power down
    Unable to poweroff system
    shutdown: 31 output lines suppressed due to ratelimiting
    Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000

    CPU: 0 PID: 1 Comm: shutdown Not tainted 4.11.0-rc8 #846
    Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
    [<c031050c>] (unwind_backtrace) from [<c030ba6c>] (show_stack+0x10/0x14)
    [<c030ba6c>] (show_stack) from [<c05b2800>] (dump_stack+0x88/0x9c)
    [<c05b2800>] (dump_stack) from [<c03d3140>] (panic+0xdc/0x268)
    [<c03d3140>] (panic) from [<c0343614>] (do_exit+0xa90/0xab4)
    [<c0343614>] (do_exit) from [<c035f2dc>] (SyS_reboot+0x164/0x1d0)
    [<c035f2dc>] (SyS_reboot) from [<c0307c80>] (ret_fast_syscall+0x0/0x3c)

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 hw/misc/exynos4210_pmu.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/misc/exynos4210_pmu.c b/hw/misc/exynos4210_pmu.c
index 63a8ccd35559..2046e6313f10 100644
--- a/hw/misc/exynos4210_pmu.c
+++ b/hw/misc/exynos4210_pmu.c
@@ -26,6 +26,7 @@
 
 #include "qemu/osdep.h"
 #include "hw/sysbus.h"
+#include "sysemu/sysemu.h"
 
 #ifndef DEBUG_PMU
 #define DEBUG_PMU           0
@@ -397,6 +398,12 @@ typedef struct Exynos4210PmuState {
     uint32_t reg[PMU_NUM_OF_REGISTERS];
 } Exynos4210PmuState;
 
+static void exynos4210_pmu_poweroff(void)
+{
+    PRINT_DEBUG("QEMU PMU: PS_HOLD bit down, powering off\n");
+    qemu_system_shutdown_request();
+}
+
 static uint64_t exynos4210_pmu_read(void *opaque, hwaddr offset,
                                     unsigned size)
 {
@@ -428,6 +435,13 @@ static void exynos4210_pmu_write(void *opaque, hwaddr offset,
             PRINT_DEBUG_EXTEND("%s <0x%04x> <- 0x%04x\n", reg_p->name,
                     (uint32_t)offset, (uint32_t)val);
             s->reg[i] = val;
+            if ((offset == PS_HOLD_CONTROL) && ((val & BIT(8)) == 0)) {
+                /*
+                 * We are interested only in setting data bit
+                 * of PS_HOLD_CONTROL register to indicate power off request.
+                 */
+                exynos4210_pmu_poweroff();
+            }
             return;
         }
         reg_p++;
-- 
2.9.3

Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/intc/exynos: Constify array of combiner interrupts

[Philippe Mathieu-Daudé]

On 05/07/2017 08:35 AM, Krzysztof Kozlowski wrote:
> The static array of interrupt combiner mappings is not modified so it
> can be made const for code safeness.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/intc/exynos4210_gic.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c
> index 222cfd6c6387..636dc6dbc1f9 100644
> --- a/hw/intc/exynos4210_gic.c
> +++ b/hw/intc/exynos4210_gic.c
> @@ -116,7 +116,7 @@ enum ExtInt {
>   * which is INTG16 in Internal Interrupt Combiner.
>   */
>
> -static uint32_t
> +static const uint32_t
>  combiner_grp_to_gic_id[64-EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
>      /* int combiner groups 16-19 */
>      { }, { }, { }, { },
>

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] hw/arm/exynos: Use type define instead of hard-coded a9mpcore_priv string

[Philippe Mathieu-Daudé]

On 05/07/2017 08:35 AM, Krzysztof Kozlowski wrote:
> Use a define for a9mpcore_priv device type name instead of hard-coded
> string.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/arm/exynos4210.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
> index 034fc8be9d76..a9e221c5b7fe 100644
> --- a/hw/arm/exynos4210.c
> +++ b/hw/arm/exynos4210.c
> @@ -26,6 +26,7 @@
>  #include "qemu-common.h"
>  #include "qemu/log.h"
>  #include "cpu.h"
> +#include "hw/cpu/a9mpcore.h"
>  #include "hw/boards.h"
>  #include "sysemu/sysemu.h"
>  #include "hw/sysbus.h"
> @@ -212,7 +213,7 @@ static void exynos4210_init(Object *obj)
>      }
>
>      /* Private memory region and Internal GIC */
> -    dev = qdev_create(NULL, "a9mpcore_priv");
> +    dev = qdev_create(NULL, TYPE_A9MPCORE_PRIV);
>      qdev_prop_set_uint32(dev, "num-cpu", EXYNOS4210_NCPUS);
>      qdev_init_nofail(dev);
>      busdev = SYS_BUS_DEVICE(dev);
>

Re: [Qemu-devel] [PATCH 3/3] hw/arm/exynos: Add support for system poweroff

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1851 bytes --]

On 05/07/2017 06:35 AM, Krzysztof Kozlowski wrote:
> On all Exynos-based boards, the system powers down itself by driving
> PS_HOLD signal low.  Handle writing to respective PMU register to fix
> power off failure:
> 
>     reboot: Power down
>     Unable to poweroff system
>     shutdown: 31 output lines suppressed due to ratelimiting
>     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
> 
>     CPU: 0 PID: 1 Comm: shutdown Not tainted 4.11.0-rc8 #846
>     Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>     [<c031050c>] (unwind_backtrace) from [<c030ba6c>] (show_stack+0x10/0x14)
>     [<c030ba6c>] (show_stack) from [<c05b2800>] (dump_stack+0x88/0x9c)
>     [<c05b2800>] (dump_stack) from [<c03d3140>] (panic+0xdc/0x268)
>     [<c03d3140>] (panic) from [<c0343614>] (do_exit+0xa90/0xab4)
>     [<c0343614>] (do_exit) from [<c035f2dc>] (SyS_reboot+0x164/0x1d0)
>     [<c035f2dc>] (SyS_reboot) from [<c0307c80>] (ret_fast_syscall+0x0/0x3c)
> 
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  hw/misc/exynos4210_pmu.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

> @@ -397,6 +398,12 @@ typedef struct Exynos4210PmuState {
>      uint32_t reg[PMU_NUM_OF_REGISTERS];
>  } Exynos4210PmuState;
>  
> +static void exynos4210_pmu_poweroff(void)
> +{
> +    PRINT_DEBUG("QEMU PMU: PS_HOLD bit down, powering off\n");
> +    qemu_system_shutdown_request();

Depending on whether your patch lands before or after mine, we'll have
to update this. It sounds like it is a guest-triggered reset, so my
patch would have this use SHUTDOWN_CAUSE_GUEST_SHUTDOWN.

https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01380.html

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/3] hw/arm/exynos: Add support for system poweroff

[Krzysztof Kozlowski]

On Mon, May 08, 2017 at 09:21:40AM -0500, Eric Blake wrote:
> On 05/07/2017 06:35 AM, Krzysztof Kozlowski wrote:
> > On all Exynos-based boards, the system powers down itself by driving
> > PS_HOLD signal low.  Handle writing to respective PMU register to fix
> > power off failure:
> > 
> >     reboot: Power down
> >     Unable to poweroff system
> >     shutdown: 31 output lines suppressed due to ratelimiting
> >     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
> > 
> >     CPU: 0 PID: 1 Comm: shutdown Not tainted 4.11.0-rc8 #846
> >     Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> >     [<c031050c>] (unwind_backtrace) from [<c030ba6c>] (show_stack+0x10/0x14)
> >     [<c030ba6c>] (show_stack) from [<c05b2800>] (dump_stack+0x88/0x9c)
> >     [<c05b2800>] (dump_stack) from [<c03d3140>] (panic+0xdc/0x268)
> >     [<c03d3140>] (panic) from [<c0343614>] (do_exit+0xa90/0xab4)
> >     [<c0343614>] (do_exit) from [<c035f2dc>] (SyS_reboot+0x164/0x1d0)
> >     [<c035f2dc>] (SyS_reboot) from [<c0307c80>] (ret_fast_syscall+0x0/0x3c)
> > 
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> >  hw/misc/exynos4210_pmu.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> 
> > @@ -397,6 +398,12 @@ typedef struct Exynos4210PmuState {
> >      uint32_t reg[PMU_NUM_OF_REGISTERS];
> >  } Exynos4210PmuState;
> >  
> > +static void exynos4210_pmu_poweroff(void)
> > +{
> > +    PRINT_DEBUG("QEMU PMU: PS_HOLD bit down, powering off\n");
> > +    qemu_system_shutdown_request();
> 
> Depending on whether your patch lands before or after mine, we'll have
> to update this. It sounds like it is a guest-triggered reset, so my
> patch would have this use SHUTDOWN_CAUSE_GUEST_SHUTDOWN.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg01380.html

Sure, I can rebase and fix the difference once your patchset gets in.

Best regards,
Krzysztof

Re: [Qemu-devel] [PATCH 3/3] hw/arm/exynos: Add support for system poweroff

[Peter Maydell]

On 7 May 2017 at 12:35, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On all Exynos-based boards, the system powers down itself by driving
> PS_HOLD signal low.  Handle writing to respective PMU register to fix
> power off failure:
>
>     reboot: Power down
>     Unable to poweroff system
>     shutdown: 31 output lines suppressed due to ratelimiting
>     Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000000
>
>     CPU: 0 PID: 1 Comm: shutdown Not tainted 4.11.0-rc8 #846
>     Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>     [<c031050c>] (unwind_backtrace) from [<c030ba6c>] (show_stack+0x10/0x14)
>     [<c030ba6c>] (show_stack) from [<c05b2800>] (dump_stack+0x88/0x9c)
>     [<c05b2800>] (dump_stack) from [<c03d3140>] (panic+0xdc/0x268)
>     [<c03d3140>] (panic) from [<c0343614>] (do_exit+0xa90/0xab4)
>     [<c0343614>] (do_exit) from [<c035f2dc>] (SyS_reboot+0x164/0x1d0)
>     [<c035f2dc>] (SyS_reboot) from [<c0307c80>] (ret_fast_syscall+0x0/0x3c)
>
> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

give or take the minor change mentioned by Eric if needed.

thanks
-- PMM