2017-04-24 17:59:02

by Julien Grall

[permalink] [raw]
Subject: [PATCH 0/3 v2] xen: Implement EFI reset_system callback

Hi all,

This small patch series implements EFI reset_system callback when using EFI
Xen. Without this, it will not be possible to reboot/power off ARM64 DOM0
when using ACPI.

Cheers,

Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>

Julien Grall (3):
xen: Export xen_reboot
arm/xen: Consolidate calls to shutdown hypercall in a single helper
xen: Implement EFI reset_system callback

arch/arm/xen/efi.c | 2 +-
arch/arm/xen/enlighten.c | 16 ++++++++++------
arch/x86/xen/efi.c | 2 +-
arch/x86/xen/enlighten.c | 2 +-
drivers/xen/efi.c | 18 ++++++++++++++++++
include/xen/xen-ops.h | 5 +++++
6 files changed, 36 insertions(+), 9 deletions(-)

--
2.11.0


2017-04-24 17:59:16

by Julien Grall

[permalink] [raw]
Subject: [PATCH 2/3 v2] arm/xen: Consolidate calls to shutdown hypercall in a single helper

Signed-off-by: Julien Grall <[email protected]>

---
Changes in v2:
- Patch added
---
arch/arm/xen/enlighten.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 81e3217b12d3..ba7f4c8f5c3e 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -191,20 +191,24 @@ static int xen_dying_cpu(unsigned int cpu)
return 0;
}

-static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
+void xen_reboot(int reason)
{
- struct sched_shutdown r = { .reason = SHUTDOWN_reboot };
+ struct sched_shutdown r = { .reason = reason };
int rc;
+
rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
BUG_ON(rc);
}

+static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
+{
+ xen_reboot(SHUTDOWN_reboot);
+}
+
+
static void xen_power_off(void)
{
- struct sched_shutdown r = { .reason = SHUTDOWN_poweroff };
- int rc;
- rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
- BUG_ON(rc);
+ xen_reboot(SHUTDOWN_poweroff);
}

static irqreturn_t xen_arm_callback(int irq, void *arg)
--
2.11.0

2017-04-24 17:59:25

by Julien Grall

[permalink] [raw]
Subject: [PATCH 3/3 v2] xen: Implement EFI reset_system callback

When rebooting DOM0 with ACPI on ARM64, the kernel is crashing with the stack
trace [1].

This is happening because when EFI runtimes are enabled, the reset code
(see machine_restart) will first try to use EFI restart method.

However, the EFI restart code is expecting the reset_system callback to
be always set. This is not the case for Xen and will lead to crash.

The EFI restart helper is used in multiple places and some of them don't
not have fallback (see machine_power_off). So implement reset_system
callback as a call to xen_reboot when using EFI Xen.

[ 36.999270] reboot: Restarting system
[ 37.002921] Internal error: Attempting to execute userspace memory: 86000004 [#1] PREEMPT SMP
[ 37.011460] Modules linked in:
[ 37.014598] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 4.11.0-rc1-00003-g1e248b60a39b-dirty #506
[ 37.023903] Hardware name: (null) (DT)
[ 37.027734] task: ffff800902068000 task.stack: ffff800902064000
[ 37.033739] PC is at 0x0
[ 37.036359] LR is at efi_reboot+0x94/0xd0
[ 37.040438] pc : [<0000000000000000>] lr : [<ffff00000880f2c4>] pstate: 404001c5
[ 37.047920] sp : ffff800902067cf0
[ 37.051314] x29: ffff800902067cf0 x28: ffff800902068000
[ 37.056709] x27: ffff000008992000 x26: 000000000000008e
[ 37.062104] x25: 0000000000000123 x24: 0000000000000015
[ 37.067499] x23: 0000000000000000 x22: ffff000008e6e250
[ 37.072894] x21: ffff000008e6e000 x20: 0000000000000000
[ 37.078289] x19: ffff000008e5d4c8 x18: 0000000000000010
[ 37.083684] x17: 0000ffffa7c27470 x16: 00000000deadbeef
[ 37.089079] x15: 0000000000000006 x14: ffff000088f42bef
[ 37.094474] x13: ffff000008f42bfd x12: ffff000008e706c0
[ 37.099870] x11: ffff000008e70000 x10: 0000000005f5e0ff
[ 37.105265] x9 : ffff800902067a50 x8 : 6974726174736552
[ 37.110660] x7 : ffff000008cc6fb8 x6 : ffff000008cc6fb0
[ 37.116055] x5 : ffff000008c97dd8 x4 : 0000000000000000
[ 37.121453] x3 : 0000000000000000 x2 : 0000000000000000
[ 37.126845] x1 : 0000000000000000 x0 : 0000000000000000
[ 37.132239]
[ 37.133808] Process systemd-shutdow (pid: 1, stack limit = 0xffff800902064000)
[ 37.141118] Stack: (0xffff800902067cf0 to 0xffff800902068000)
[ 37.146949] 7ce0: ffff800902067d40 ffff000008085334
[ 37.154869] 7d00: 0000000000000000 ffff000008f3b000 ffff800902067d40 ffff0000080852e0
[ 37.162787] 7d20: ffff000008cc6fb0 ffff000008cc6fb8 ffff000008c7f580 ffff000008c97dd8
[ 37.170706] 7d40: ffff800902067d60 ffff0000080e2c2c 0000000000000000 0000000001234567
[ 37.178624] 7d60: ffff800902067d80 ffff0000080e2ee8 0000000000000000 ffff0000080e2df4
[ 37.186544] 7d80: 0000000000000000 ffff0000080830f0 0000000000000000 00008008ff1c1000
[ 37.194462] 7da0: ffffffffffffffff 0000ffffa7c4b1cc 0000000000000000 0000000000000024
[ 37.202380] 7dc0: ffff800902067dd0 0000000000000005 0000fffff24743c8 0000000000000004
[ 37.210299] 7de0: 0000fffff2475f03 0000000000000010 0000fffff2474418 0000000000000005
[ 37.218218] 7e00: 0000fffff2474578 000000000000000a 0000aaaad6b722c0 0000000000000001
[ 37.226136] 7e20: 0000000000000123 0000000000000038 ffff800902067e50 ffff0000081e7294
[ 37.234055] 7e40: ffff800902067e60 ffff0000081e935c ffff800902067e60 ffff0000081e9388
[ 37.241973] 7e60: ffff800902067eb0 ffff0000081ea388 0000000000000000 00008008ff1c1000
[ 37.249892] 7e80: ffffffffffffffff 0000ffffa7c4a79c 0000000000000000 ffff000000020000
[ 37.257810] 7ea0: 0000010000000004 0000000000000000 0000000000000000 ffff0000080830f0
[ 37.265729] 7ec0: fffffffffee1dead 0000000028121969 0000000001234567 0000000000000000
[ 37.273651] 7ee0: ffffffffffffffff 8080000000800000 0000800000008080 feffa9a9d4ff2d66
[ 37.281567] 7f00: 000000000000008e feffa9a9d5b60e0f 7f7fffffffff7f7f 0101010101010101
[ 37.289485] 7f20: 0000000000000010 0000000000000008 000000000000003a 0000ffffa7ccf588
[ 37.297404] 7f40: 0000aaaad6b87d00 0000ffffa7c4b1b0 0000fffff2474be0 0000aaaad6b88000
[ 37.305326] 7f60: 0000fffff2474fb0 0000000001234567 0000000000000000 0000000000000000
[ 37.313240] 7f80: 0000000000000000 0000000000000001 0000aaaad6b70d4d 0000000000000000
[ 37.321159] 7fa0: 0000000000000001 0000fffff2474ea0 0000aaaad6b5e2e0 0000fffff2474e80
[ 37.329078] 7fc0: 0000ffffa7c4b1cc 0000000000000000 fffffffffee1dead 000000000000008e
[ 37.336997] 7fe0: 0000000000000000 0000000000000000 9ce839cffee77eab fafdbf9f7ed57f2f
[ 37.344911] Call trace:
[ 37.347437] Exception stack(0xffff800902067b20 to 0xffff800902067c50)
[ 37.353970] 7b20: ffff000008e5d4c8 0001000000000000 0000000080f82000 0000000000000000
[ 37.361883] 7b40: ffff800902067b60 ffff000008e17000 ffff000008f44c68 00000001081081b4
[ 37.369802] 7b60: ffff800902067bf0 ffff000008108478 0000000000000000 ffff000008c235b0
[ 37.377721] 7b80: ffff800902067ce0 0000000000000000 0000000000000000 0000000000000015
[ 37.385643] 7ba0: 0000000000000123 000000000000008e ffff000008992000 ffff800902068000
[ 37.393557] 7bc0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
[ 37.401477] 7be0: 0000000000000000 ffff000008c97dd8 ffff000008cc6fb0 ffff000008cc6fb8
[ 37.409396] 7c00: 6974726174736552 ffff800902067a50 0000000005f5e0ff ffff000008e70000
[ 37.417318] 7c20: ffff000008e706c0 ffff000008f42bfd ffff000088f42bef 0000000000000006
[ 37.425234] 7c40: 00000000deadbeef 0000ffffa7c27470
[ 37.430190] [< (null)>] (null)
[ 37.434982] [<ffff000008085334>] machine_restart+0x6c/0x70
[ 37.440550] [<ffff0000080e2c2c>] kernel_restart+0x6c/0x78
[ 37.446030] [<ffff0000080e2ee8>] SyS_reboot+0x130/0x228
[ 37.451337] [<ffff0000080830f0>] el0_svc_naked+0x24/0x28
[ 37.456737] Code: bad PC value
[ 37.459891] ---[ end trace 76e2fc17e050aecd ]---

Signed-off-by: Julien Grall <[email protected]>

--

Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]

The x86 code has theoritically a similar issue, altought EFI does not
seem to be the preferred method. I have only built test it on x86.

This should also probably be fixed in stable tree.

Changes in v2:
- Implement xen_efi_reset_system using xen_reboot
- Move xen_efi_reset_system in drivers/xen/efi.c
---
arch/arm/xen/efi.c | 2 +-
arch/x86/xen/efi.c | 2 +-
drivers/xen/efi.c | 18 ++++++++++++++++++
include/xen/xen-ops.h | 3 +++
4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/arm/xen/efi.c b/arch/arm/xen/efi.c
index 16db419f9e90..b4d78959cadf 100644
--- a/arch/arm/xen/efi.c
+++ b/arch/arm/xen/efi.c
@@ -35,6 +35,6 @@ void __init xen_efi_runtime_setup(void)
efi.update_capsule = xen_efi_update_capsule;
efi.query_capsule_caps = xen_efi_query_capsule_caps;
efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
- efi.reset_system = NULL; /* Functionality provided by Xen. */
+ efi.reset_system = xen_efi_reset_system;
}
EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 3be012115853..30bb2e80cfe7 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -81,7 +81,7 @@ static const struct efi efi_xen __initconst = {
.update_capsule = xen_efi_update_capsule,
.query_capsule_caps = xen_efi_query_capsule_caps,
.get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = NULL, /* Functionality provided by Xen. */
+ .reset_system = xen_efi_reset_system,
.set_virtual_address_map = NULL, /* Not used under Xen. */
.flags = 0 /* Initialized later. */
};
diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
index 22f71ffd3406..9243a9051078 100644
--- a/drivers/xen/efi.c
+++ b/drivers/xen/efi.c
@@ -26,6 +26,7 @@
#include <xen/interface/xen.h>
#include <xen/interface/platform.h>
#include <xen/xen.h>
+#include <xen/xen-ops.h>

#include <asm/page.h>

@@ -263,3 +264,20 @@ efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
return efi_data(op).status;
}
EXPORT_SYMBOL_GPL(xen_efi_query_capsule_caps);
+
+void xen_efi_reset_system(int reset_type, efi_status_t status,
+ unsigned long data_size, efi_char16_t *data)
+{
+ switch (reset_type) {
+ case EFI_RESET_COLD:
+ case EFI_RESET_WARM:
+ xen_reboot(SHUTDOWN_reboot);
+ break;
+ case EFI_RESET_SHUTDOWN:
+ xen_reboot(SHUTDOWN_poweroff);
+ break;
+ default:
+ BUG();
+ }
+}
+EXPORT_SYMBOL_GPL(xen_efi_reset_system);
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index fc5ddb472f86..197bb4866327 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -122,6 +122,9 @@ efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
unsigned long count, u64 *max_size,
int *reset_type);
+void xen_efi_reset_system(int reset_type, efi_status_t status,
+ unsigned long data_size, efi_char16_t *data);
+

#ifdef CONFIG_PREEMPT

--
2.11.0

2017-04-24 17:59:43

by Julien Grall

[permalink] [raw]
Subject: [PATCH 1/3 v2] xen: Export xen_reboot

The helper xen_reboot will be called by the EFI code in a later patch.

Note that the ARM version does not yet exist and will be added in a
later patch too.

Signed-off-by: Julien Grall <[email protected]>

---

Cc: Boris Ostrovsky <[email protected]>
Cc: Juergen Gross <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: [email protected]

Changes in v2:
- Patch added
---
arch/x86/xen/enlighten.c | 2 +-
include/xen/xen-ops.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index ec1d5c46e58f..563f2d963a04 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1294,7 +1294,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
.end_context_switch = xen_end_context_switch,
};

-static void xen_reboot(int reason)
+void xen_reboot(int reason)
{
struct sched_shutdown r = { .reason = reason };
int cpu;
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index b5486e648607..fc5ddb472f86 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -22,6 +22,8 @@ void xen_timer_resume(void);
void xen_arch_resume(void);
void xen_arch_suspend(void);

+void xen_reboot(int reason);
+
void xen_resume_notifier_register(struct notifier_block *nb);
void xen_resume_notifier_unregister(struct notifier_block *nb);

--
2.11.0

2017-04-24 18:23:00

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] xen: Export xen_reboot

On 04/24/2017 01:58 PM, Julien Grall wrote:
> The helper xen_reboot will be called by the EFI code in a later patch.
>
> Note that the ARM version does not yet exist and will be added in a
> later patch too.
>
> Signed-off-by: Julien Grall <[email protected]>

I don't think these changes are worth a whole patch. They can be folded
into the third patch.

-boris

>
> ---
>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: Juergen Gross <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: [email protected]
>
> Changes in v2:
> - Patch added
> ---
> arch/x86/xen/enlighten.c | 2 +-
> include/xen/xen-ops.h | 2 ++
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index ec1d5c46e58f..563f2d963a04 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1294,7 +1294,7 @@ static const struct pv_cpu_ops xen_cpu_ops __initconst = {
> .end_context_switch = xen_end_context_switch,
> };
>
> -static void xen_reboot(int reason)
> +void xen_reboot(int reason)
> {
> struct sched_shutdown r = { .reason = reason };
> int cpu;
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index b5486e648607..fc5ddb472f86 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -22,6 +22,8 @@ void xen_timer_resume(void);
> void xen_arch_resume(void);
> void xen_arch_suspend(void);
>
> +void xen_reboot(int reason);
> +
> void xen_resume_notifier_register(struct notifier_block *nb);
> void xen_resume_notifier_unregister(struct notifier_block *nb);
>

2017-04-24 18:24:46

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH 3/3 v2] xen: Implement EFI reset_system callback

On 04/24/2017 01:58 PM, Julien Grall wrote:
> When rebooting DOM0 with ACPI on ARM64, the kernel is crashing with the stack
> trace [1].
>
> This is happening because when EFI runtimes are enabled, the reset code
> (see machine_restart) will first try to use EFI restart method.
>
> However, the EFI restart code is expecting the reset_system callback to
> be always set. This is not the case for Xen and will lead to crash.
>
> The EFI restart helper is used in multiple places and some of them don't
> not have fallback (see machine_power_off). So implement reset_system
> callback as a call to xen_reboot when using EFI Xen.

Reviewed-by: Boris Ostrovsky <[email protected]>


2017-04-24 19:31:00

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH 2/3 v2] arm/xen: Consolidate calls to shutdown hypercall in a single helper

On Mon, 24 Apr 2017, Julien Grall wrote:
> Signed-off-by: Julien Grall <[email protected]>

Reviewed-by: Stefano Stabellini <[email protected]>


> ---
> Changes in v2:
> - Patch added
> ---
> arch/arm/xen/enlighten.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index 81e3217b12d3..ba7f4c8f5c3e 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -191,20 +191,24 @@ static int xen_dying_cpu(unsigned int cpu)
> return 0;
> }
>
> -static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
> +void xen_reboot(int reason)
> {
> - struct sched_shutdown r = { .reason = SHUTDOWN_reboot };
> + struct sched_shutdown r = { .reason = reason };
> int rc;
> +
> rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
> BUG_ON(rc);
> }
>
> +static void xen_restart(enum reboot_mode reboot_mode, const char *cmd)
> +{
> + xen_reboot(SHUTDOWN_reboot);
> +}
> +
> +
> static void xen_power_off(void)
> {
> - struct sched_shutdown r = { .reason = SHUTDOWN_poweroff };
> - int rc;
> - rc = HYPERVISOR_sched_op(SCHEDOP_shutdown, &r);
> - BUG_ON(rc);
> + xen_reboot(SHUTDOWN_poweroff);
> }
>
> static irqreturn_t xen_arm_callback(int irq, void *arg)
> --
> 2.11.0
>

2017-04-24 19:36:06

by Stefano Stabellini

[permalink] [raw]
Subject: Re: [PATCH 3/3 v2] xen: Implement EFI reset_system callback

On Mon, 24 Apr 2017, Julien Grall wrote:
> When rebooting DOM0 with ACPI on ARM64, the kernel is crashing with the stack
> trace [1].
>
> This is happening because when EFI runtimes are enabled, the reset code
> (see machine_restart) will first try to use EFI restart method.
>
> However, the EFI restart code is expecting the reset_system callback to
> be always set. This is not the case for Xen and will lead to crash.
>
> The EFI restart helper is used in multiple places and some of them don't
> not have fallback (see machine_power_off). So implement reset_system
> callback as a call to xen_reboot when using EFI Xen.
>
> [ 36.999270] reboot: Restarting system
> [ 37.002921] Internal error: Attempting to execute userspace memory: 86000004 [#1] PREEMPT SMP
> [ 37.011460] Modules linked in:
> [ 37.014598] CPU: 0 PID: 1 Comm: systemd-shutdow Not tainted 4.11.0-rc1-00003-g1e248b60a39b-dirty #506
> [ 37.023903] Hardware name: (null) (DT)
> [ 37.027734] task: ffff800902068000 task.stack: ffff800902064000
> [ 37.033739] PC is at 0x0
> [ 37.036359] LR is at efi_reboot+0x94/0xd0
> [ 37.040438] pc : [<0000000000000000>] lr : [<ffff00000880f2c4>] pstate: 404001c5
> [ 37.047920] sp : ffff800902067cf0
> [ 37.051314] x29: ffff800902067cf0 x28: ffff800902068000
> [ 37.056709] x27: ffff000008992000 x26: 000000000000008e
> [ 37.062104] x25: 0000000000000123 x24: 0000000000000015
> [ 37.067499] x23: 0000000000000000 x22: ffff000008e6e250
> [ 37.072894] x21: ffff000008e6e000 x20: 0000000000000000
> [ 37.078289] x19: ffff000008e5d4c8 x18: 0000000000000010
> [ 37.083684] x17: 0000ffffa7c27470 x16: 00000000deadbeef
> [ 37.089079] x15: 0000000000000006 x14: ffff000088f42bef
> [ 37.094474] x13: ffff000008f42bfd x12: ffff000008e706c0
> [ 37.099870] x11: ffff000008e70000 x10: 0000000005f5e0ff
> [ 37.105265] x9 : ffff800902067a50 x8 : 6974726174736552
> [ 37.110660] x7 : ffff000008cc6fb8 x6 : ffff000008cc6fb0
> [ 37.116055] x5 : ffff000008c97dd8 x4 : 0000000000000000
> [ 37.121453] x3 : 0000000000000000 x2 : 0000000000000000
> [ 37.126845] x1 : 0000000000000000 x0 : 0000000000000000
> [ 37.132239]
> [ 37.133808] Process systemd-shutdow (pid: 1, stack limit = 0xffff800902064000)
> [ 37.141118] Stack: (0xffff800902067cf0 to 0xffff800902068000)
> [ 37.146949] 7ce0: ffff800902067d40 ffff000008085334
> [ 37.154869] 7d00: 0000000000000000 ffff000008f3b000 ffff800902067d40 ffff0000080852e0
> [ 37.162787] 7d20: ffff000008cc6fb0 ffff000008cc6fb8 ffff000008c7f580 ffff000008c97dd8
> [ 37.170706] 7d40: ffff800902067d60 ffff0000080e2c2c 0000000000000000 0000000001234567
> [ 37.178624] 7d60: ffff800902067d80 ffff0000080e2ee8 0000000000000000 ffff0000080e2df4
> [ 37.186544] 7d80: 0000000000000000 ffff0000080830f0 0000000000000000 00008008ff1c1000
> [ 37.194462] 7da0: ffffffffffffffff 0000ffffa7c4b1cc 0000000000000000 0000000000000024
> [ 37.202380] 7dc0: ffff800902067dd0 0000000000000005 0000fffff24743c8 0000000000000004
> [ 37.210299] 7de0: 0000fffff2475f03 0000000000000010 0000fffff2474418 0000000000000005
> [ 37.218218] 7e00: 0000fffff2474578 000000000000000a 0000aaaad6b722c0 0000000000000001
> [ 37.226136] 7e20: 0000000000000123 0000000000000038 ffff800902067e50 ffff0000081e7294
> [ 37.234055] 7e40: ffff800902067e60 ffff0000081e935c ffff800902067e60 ffff0000081e9388
> [ 37.241973] 7e60: ffff800902067eb0 ffff0000081ea388 0000000000000000 00008008ff1c1000
> [ 37.249892] 7e80: ffffffffffffffff 0000ffffa7c4a79c 0000000000000000 ffff000000020000
> [ 37.257810] 7ea0: 0000010000000004 0000000000000000 0000000000000000 ffff0000080830f0
> [ 37.265729] 7ec0: fffffffffee1dead 0000000028121969 0000000001234567 0000000000000000
> [ 37.273651] 7ee0: ffffffffffffffff 8080000000800000 0000800000008080 feffa9a9d4ff2d66
> [ 37.281567] 7f00: 000000000000008e feffa9a9d5b60e0f 7f7fffffffff7f7f 0101010101010101
> [ 37.289485] 7f20: 0000000000000010 0000000000000008 000000000000003a 0000ffffa7ccf588
> [ 37.297404] 7f40: 0000aaaad6b87d00 0000ffffa7c4b1b0 0000fffff2474be0 0000aaaad6b88000
> [ 37.305326] 7f60: 0000fffff2474fb0 0000000001234567 0000000000000000 0000000000000000
> [ 37.313240] 7f80: 0000000000000000 0000000000000001 0000aaaad6b70d4d 0000000000000000
> [ 37.321159] 7fa0: 0000000000000001 0000fffff2474ea0 0000aaaad6b5e2e0 0000fffff2474e80
> [ 37.329078] 7fc0: 0000ffffa7c4b1cc 0000000000000000 fffffffffee1dead 000000000000008e
> [ 37.336997] 7fe0: 0000000000000000 0000000000000000 9ce839cffee77eab fafdbf9f7ed57f2f
> [ 37.344911] Call trace:
> [ 37.347437] Exception stack(0xffff800902067b20 to 0xffff800902067c50)
> [ 37.353970] 7b20: ffff000008e5d4c8 0001000000000000 0000000080f82000 0000000000000000
> [ 37.361883] 7b40: ffff800902067b60 ffff000008e17000 ffff000008f44c68 00000001081081b4
> [ 37.369802] 7b60: ffff800902067bf0 ffff000008108478 0000000000000000 ffff000008c235b0
> [ 37.377721] 7b80: ffff800902067ce0 0000000000000000 0000000000000000 0000000000000015
> [ 37.385643] 7ba0: 0000000000000123 000000000000008e ffff000008992000 ffff800902068000
> [ 37.393557] 7bc0: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> [ 37.401477] 7be0: 0000000000000000 ffff000008c97dd8 ffff000008cc6fb0 ffff000008cc6fb8
> [ 37.409396] 7c00: 6974726174736552 ffff800902067a50 0000000005f5e0ff ffff000008e70000
> [ 37.417318] 7c20: ffff000008e706c0 ffff000008f42bfd ffff000088f42bef 0000000000000006
> [ 37.425234] 7c40: 00000000deadbeef 0000ffffa7c27470
> [ 37.430190] [< (null)>] (null)
> [ 37.434982] [<ffff000008085334>] machine_restart+0x6c/0x70
> [ 37.440550] [<ffff0000080e2c2c>] kernel_restart+0x6c/0x78
> [ 37.446030] [<ffff0000080e2ee8>] SyS_reboot+0x130/0x228
> [ 37.451337] [<ffff0000080830f0>] el0_svc_naked+0x24/0x28
> [ 37.456737] Code: bad PC value
> [ 37.459891] ---[ end trace 76e2fc17e050aecd ]---
>
> Signed-off-by: Julien Grall <[email protected]>

Reviewed-by: Stefano Stabellini <[email protected]>


> --
>
> Cc: Boris Ostrovsky <[email protected]>
> Cc: Juergen Gross <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: [email protected]
>
> The x86 code has theoritically a similar issue, altought EFI does not
> seem to be the preferred method. I have only built test it on x86.
>
> This should also probably be fixed in stable tree.
>
> Changes in v2:
> - Implement xen_efi_reset_system using xen_reboot
> - Move xen_efi_reset_system in drivers/xen/efi.c
> ---
> arch/arm/xen/efi.c | 2 +-
> arch/x86/xen/efi.c | 2 +-
> drivers/xen/efi.c | 18 ++++++++++++++++++
> include/xen/xen-ops.h | 3 +++
> 4 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/xen/efi.c b/arch/arm/xen/efi.c
> index 16db419f9e90..b4d78959cadf 100644
> --- a/arch/arm/xen/efi.c
> +++ b/arch/arm/xen/efi.c
> @@ -35,6 +35,6 @@ void __init xen_efi_runtime_setup(void)
> efi.update_capsule = xen_efi_update_capsule;
> efi.query_capsule_caps = xen_efi_query_capsule_caps;
> efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> - efi.reset_system = NULL; /* Functionality provided by Xen. */
> + efi.reset_system = xen_efi_reset_system;
> }
> EXPORT_SYMBOL_GPL(xen_efi_runtime_setup);
> diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> index 3be012115853..30bb2e80cfe7 100644
> --- a/arch/x86/xen/efi.c
> +++ b/arch/x86/xen/efi.c
> @@ -81,7 +81,7 @@ static const struct efi efi_xen __initconst = {
> .update_capsule = xen_efi_update_capsule,
> .query_capsule_caps = xen_efi_query_capsule_caps,
> .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> - .reset_system = NULL, /* Functionality provided by Xen. */
> + .reset_system = xen_efi_reset_system,
> .set_virtual_address_map = NULL, /* Not used under Xen. */
> .flags = 0 /* Initialized later. */
> };
> diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c
> index 22f71ffd3406..9243a9051078 100644
> --- a/drivers/xen/efi.c
> +++ b/drivers/xen/efi.c
> @@ -26,6 +26,7 @@
> #include <xen/interface/xen.h>
> #include <xen/interface/platform.h>
> #include <xen/xen.h>
> +#include <xen/xen-ops.h>
>
> #include <asm/page.h>
>
> @@ -263,3 +264,20 @@ efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
> return efi_data(op).status;
> }
> EXPORT_SYMBOL_GPL(xen_efi_query_capsule_caps);
> +
> +void xen_efi_reset_system(int reset_type, efi_status_t status,
> + unsigned long data_size, efi_char16_t *data)
> +{
> + switch (reset_type) {
> + case EFI_RESET_COLD:
> + case EFI_RESET_WARM:
> + xen_reboot(SHUTDOWN_reboot);
> + break;
> + case EFI_RESET_SHUTDOWN:
> + xen_reboot(SHUTDOWN_poweroff);
> + break;
> + default:
> + BUG();
> + }
> +}
> +EXPORT_SYMBOL_GPL(xen_efi_reset_system);
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index fc5ddb472f86..197bb4866327 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -122,6 +122,9 @@ efi_status_t xen_efi_update_capsule(efi_capsule_header_t **capsules,
> efi_status_t xen_efi_query_capsule_caps(efi_capsule_header_t **capsules,
> unsigned long count, u64 *max_size,
> int *reset_type);
> +void xen_efi_reset_system(int reset_type, efi_status_t status,
> + unsigned long data_size, efi_char16_t *data);
> +
>
> #ifdef CONFIG_PREEMPT
>
> --
> 2.11.0
>

2017-04-25 04:02:30

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] xen: Export xen_reboot

On 24/04/17 20:21, Boris Ostrovsky wrote:
> On 04/24/2017 01:58 PM, Julien Grall wrote:
>> The helper xen_reboot will be called by the EFI code in a later patch.
>>
>> Note that the ARM version does not yet exist and will be added in a
>> later patch too.
>>
>> Signed-off-by: Julien Grall <[email protected]>
>
> I don't think these changes are worth a whole patch. They can be folded
> into the third patch.

No, the 2nd patch needs this, too.

So:

Reviewed-by: Juergen Gross <[email protected]>


Thanks, Juergen

2017-04-28 18:00:07

by Jürgen Groß

[permalink] [raw]
Subject: Re: [PATCH 1/3 v2] xen: Export xen_reboot

On 25/04/17 06:02, Juergen Gross wrote:
> On 24/04/17 20:21, Boris Ostrovsky wrote:
>> On 04/24/2017 01:58 PM, Julien Grall wrote:
>>> The helper xen_reboot will be called by the EFI code in a later patch.
>>>
>>> Note that the ARM version does not yet exist and will be added in a
>>> later patch too.
>>>
>>> Signed-off-by: Julien Grall <[email protected]>
>>
>> I don't think these changes are worth a whole patch. They can be folded
>> into the third patch.
>
> No, the 2nd patch needs this, too.
>
> So:
>
> Reviewed-by: Juergen Gross <[email protected]>

Julien, this patch no longer applies to the for-linus-4.12 branch of
xen/tip. Can you please rebase?


Juergen