| Message ID | 20190328120658.11083-1-jgross@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | xen: simplify suspend/resume handling | expand |
Hello Juergen, On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: > > Especially in the scheduler area (schedule.c, cpupool.c) there is a > rather complex handling involved when doing suspend and resume. > > This can be simplified a lot by not performing a complete cpu down and > up cycle for the non-boot cpus, but keeping the pure software related > state and freeing it only in case a cpu didn't come up again during > resume. > > In summary not only the complexity can be reduced, but the failure > tolerance will be even better with this series: With a dedicated hook > for failing cpus when resuming it is now possible to survive e.g. a > cpupool being left without any cpu after resume by moving its domains > to cpupool0. > > Juergen Gross (6): > xen/sched: call cpu_disable_scheduler() via cpu notifier > xen: add helper for calling notifier_call_chain() to common/cpu.c > xen: add new cpu notifier action CPU_RESUME_FAILED > xen: don't free percpu areas during suspend > xen/cpupool: simplify suspend/resume handling > xen/sched: don't disable scheduler on cpus during suspend > > xen/arch/arm/smpboot.c | 4 - > xen/arch/x86/percpu.c | 3 +- > xen/arch/x86/smpboot.c | 3 - > xen/common/cpu.c | 61 +++++++------- > xen/common/cpupool.c | 131 ++++++++++++----------------- > xen/common/schedule.c | 203 +++++++++++++++++++-------------------------- > xen/include/xen/cpu.h | 29 ++++--- > xen/include/xen/sched-if.h | 1 - > 8 files changed, 190 insertions(+), 245 deletions(-) > I tested your patch series on ARM64 platform. We had issue with hard affinity - there was assertion failure in sched_credit2 code during suspension if one of the vCPUs is pinned to non-0 pCPU. Seems, your patch series fixes the issue during suspend. But now I'm seeing crash during resume: (XEN) suspend.c:198: Resume (XEN) Enabling non-boot CPUs ... (XEN) Bringing up CPU1 (XEN) CPU1 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry (XEN) CPU 1 booted. (XEN) Bringing up CPU2 (XEN) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x0 on CPU1 via TTBR 0x00000000781a8000 (XEN) 0TH[0x0] = 0x00000000781b0f7f (XEN) 1ST[0x0] = 0x00000000781aaf7f (XEN) 2ND[0x0] = 0x0000000000000000 (XEN) CPU1: Unexpected Trap: Data Abort (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- (XEN) CPU: 1 (XEN) PC: 0000000000233660 _spin_lock+0x1c/0x88 (XEN) LR: 000000000023365c (XEN) SP: 000080037ffc7d50 (XEN) CPSR: 600002c9 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0000000000000006 X1: 0000000000000000 X2: 0000000000000000 (XEN) X3: 0000000000000002 X4: 000080037fc94480 X5: 0000000000000000 (XEN) X6: 0000000000000080 X7: 000080037ffb0000 X8: 00000000002a1000 (XEN) X9: 000000000000000a X10: 000080037ffc7bf8 X11: 0000000000000031 (XEN) X12: 0000000000000001 X13: 000000000027fff0 X14: 0000000000000020 (XEN) X15: 0000000000000000 X16: 0000000000000000 X17: 0000000000000000 (XEN) X18: 0000000000000000 X19: 0000000000000000 X20: 0000000000000000 (XEN) X21: 000080037ffd0108 X22: 0000000000000001 X23: 000000000033bc88 (XEN) X24: 0000000000336020 X25: 0000000000000000 X26: 0000000000000001 (XEN) X27: 0000000000336000 X28: 0000000000000000 FP: 000080037ffc7d50 (XEN) (XEN) VTCR_EL2: 80023558 (XEN) VTTBR_EL2: 0000000000000000 (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN) HCR_EL2: 0000000000000038 (XEN) TTBR0_EL2: 00000000781a8000 (XEN) (XEN) ESR_EL2: 96000006 (XEN) HPFAR_EL2: 0000000000000000 (XEN) FAR_EL2: 0000000000000000 (XEN) (XEN) Xen stack trace from sp=000080037ffc7d50: (XEN) 000080037ffc7d70 00000000002336e8 000080037ffd2000 000000000023e00c (XEN) 000080037ffc7d80 000000000022e90c 000080037ffc7e10 0000000000232af8 (XEN) 0000000000000001 00000000002fbb00 ffffffffffffffff 000000000033cf20 (XEN) 00000000002a0680 0000000000000001 0000000000000001 0000000000000001 (XEN) 0000000000000000 000080037ffc7e90 000080037ffc7e50 00000000ffffffc8 (XEN) 000000000029f008 00000000002ffc41 000080037ffc7e90 0000000000263c68 (XEN) 000080037ffc7e50 0000000000232b6c 0000000000000001 0000000000000002 (XEN) 0000000000000001 00000000002fbb80 0000000000336448 00000000002fbb00 (XEN) 000080037ffc7e60 0000000000257230 000080037ffc7e90 0000000000263c6c (XEN) 0000000000000001 0000000077e80000 0000000000000000 0000000000000001 (XEN) 0000000000000000 0000000000000001 0000000000000001 0000ffff0000ffff (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 (XEN) Xen call trace: (XEN) [<0000000000233660>] _spin_lock+0x1c/0x88 (PC) (XEN) [<000000000023365c>] _spin_lock+0x18/0x88 (LR) (XEN) [<00000000002336e8>] _spin_lock_irq+0x1c/0x24 (XEN) [<000000000022e90c>] schedule.c#schedule+0xe8/0x74c (XEN) [<0000000000232af8>] softirq.c#__do_softirq+0xcc/0xe4 (XEN) [<0000000000232b6c>] do_softirq+0x14/0x1c (XEN) [<0000000000257230>] idle_loop+0x174/0x188 (XEN) [<0000000000263c6c>] start_secondary+0x1f4/0x200 (XEN) [<0000000000000001>] 0000000000000001 (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 1: (XEN) CPU1: Unexpected Trap: Data Abort (XEN) **************************************** (XEN) (XEN) Reboot in five seconds... (XEN) (XEN) **************************************** (XEN) Panic on CPU 0: (XEN) PSCI cpu off failed for CPU0 err=-3 (XEN) **************************************** (XEN) (XEN) Reboot in five seconds... (XEN) CPU2 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry (XEN) CPU 2 booted. (XEN) Data Abort Trap. Syndrome=0x6 (XEN) Walking Hypervisor VA 0x0 on CPU2 via TTBR 0x00000000781a8000 (XEN) 0TH[0x0] = 0x00000000781b0f7f (XEN) 1ST[0x0] = 0x00000000781aaf7f (XEN) 2ND[0x0] = 0x0000000000000000 (XEN) CPU2: Unexpected Trap: Data Abort (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- (XEN) CPU: 2 (XEN) PC: 0000000000233660 _spin_lock+0x1c/0x88 (XEN) LR: 000000000023365c (XEN) SP: 000080037ff77d50 (XEN) CPSR: a00002c9 MODE:64-bit EL2h (Hypervisor, handler) (XEN) X0: 0000000000000006 X1: 00000000fffffffe X2: 0000000000000000 (XEN) X3: 0000000000000002 X4: 000080037fc42480 X5: 0000000000000000 (XEN) X6: 0000000000000080 X7: 000080037ffb0000 X8: 00000000002a1000 (XEN) X9: 000000000000000a X10: 000080037ff77bf8 X11: 0000000000000032 (XEN) X12: 0000000000000001 X13: 000000000027fff0 X14: 0000000000000020 (XEN) X15: 0000000000000000 X16: 0000000000000000 X17: 0000000000000000 (XEN) X18: 0000000000000000 X19: 0000000000000000 X20: 0000000000000000 (XEN) X21: 000080037ff7e108 X22: 0000000000000002 X23: 000000000033bc88 (XEN) X24: 0000000000336020 X25: 0000000000000000 X26: 0000000000000002 (XEN) X27: 0000000000336000 X28: 0000000000000000 FP: 000080037ff77d50 (XEN) (XEN) VTCR_EL2: 80023558 (XEN) VTTBR_EL2: 0000000000000000 (XEN) (XEN) SCTLR_EL2: 30cd183d (XEN) HCR_EL2: 0000000000000038 (XEN) TTBR0_EL2: 00000000781a8000 (XEN) (XEN) ESR_EL2: 96000006 (XEN) HPFAR_EL2: 0000000000000000 (XEN) FAR_EL2: 0000000000000000 (XEN) (XEN) Xen stack trace from sp=000080037ff77d50: (XEN) 000080037ff77d70 00000000002336e8 000080037ff7d000 000000000023e00c (XEN) 000080037ff77d80 000000000022e90c 000080037ff77e10 0000000000232af8 (XEN) 0000000000000002 00000000002fbb00 ffffffffffffffff 000000000033cf20 (XEN) 00000000002a0680 0000000000000001 0000000000000001 0000000000000001 (XEN) 0000000000000000 000080037ff77e90 000080037ff77e50 00000000ffffffc8 (XEN) 000000000029f008 00000000002ffc41 000080037ff77e90 0000000000263c68 (XEN) 000080037ff77e50 0000000000232b6c 0000000000000002 0000000000000004 (XEN) 0000000000000002 00000000002fbc00 0000000000336448 00000000002fbb00 (XEN) 000080037ff77e60 0000000000257230 000080037ff77e90 0000000000263c6c (XEN) 0000000000000002 0000000077e80000 0000000000000000 0000000000000001 (XEN) 0000000000000000 0000000000000002 0000000000000001 effffffffffaffff (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 (XEN) 0000000000000000 0000000000000000 (XEN) Xen call trace: (XEN) [<0000000000233660>] _spin_lock+0x1c/0x88 (PC) (XEN) [<000000000023365c>] _spin_lock+0x18/0x88 (LR) (XEN) [<00000000002336e8>] _spin_lock_irq+0x1c/0x24 (XEN) [<000000000022e90c>] schedule.c#schedule+0xe8/0x74c (XEN) [<0000000000232af8>] softirq.c#__do_softirq+0xcc/0xe4 (XEN) [<0000000000232b6c>] do_softirq+0x14/0x1c (XEN) [<0000000000257230>] idle_loop+0x174/0x188 (XEN) [<0000000000263c6c>] start_secondary+0x1f4/0x200 (XEN) [<0000000000000002>] 0000000000000002 (XEN) (XEN) (XEN) **************************************** (XEN) Panic on CPU 2: (XEN) CPU2: Unexpected Trap: Data Abort (XEN) **************************************** (XEN) (XEN) Reboot in five seconds...
On 28/03/2019 14:01, Volodymyr Babchuk wrote: > Hello Juergen, > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: >> >> Especially in the scheduler area (schedule.c, cpupool.c) there is a >> rather complex handling involved when doing suspend and resume. >> >> This can be simplified a lot by not performing a complete cpu down and >> up cycle for the non-boot cpus, but keeping the pure software related >> state and freeing it only in case a cpu didn't come up again during >> resume. >> >> In summary not only the complexity can be reduced, but the failure >> tolerance will be even better with this series: With a dedicated hook >> for failing cpus when resuming it is now possible to survive e.g. a >> cpupool being left without any cpu after resume by moving its domains >> to cpupool0. >> >> Juergen Gross (6): >> xen/sched: call cpu_disable_scheduler() via cpu notifier >> xen: add helper for calling notifier_call_chain() to common/cpu.c >> xen: add new cpu notifier action CPU_RESUME_FAILED >> xen: don't free percpu areas during suspend >> xen/cpupool: simplify suspend/resume handling >> xen/sched: don't disable scheduler on cpus during suspend >> >> xen/arch/arm/smpboot.c | 4 - >> xen/arch/x86/percpu.c | 3 +- >> xen/arch/x86/smpboot.c | 3 - >> xen/common/cpu.c | 61 +++++++------- >> xen/common/cpupool.c | 131 ++++++++++++----------------- >> xen/common/schedule.c | 203 +++++++++++++++++++-------------------------- >> xen/include/xen/cpu.h | 29 ++++--- >> xen/include/xen/sched-if.h | 1 - >> 8 files changed, 190 insertions(+), 245 deletions(-) >> > > I tested your patch series on ARM64 platform. We had issue with hard > affinity - there was assertion failure in sched_credit2 code during > suspension if one of the vCPUs is pinned to non-0 pCPU. > > Seems, your patch series fixes the issue during suspend. But now I'm > seeing crash during resume: Oh, I wasn't aware the suspend/resume is possible on ARM, too. I need to modify arch/arm/percpu.c like on x86. Could you test the attached patch if it is working? Juergen diff --git a/xen/arch/arm/percpu.c b/xen/arch/arm/percpu.c index 25442c48fe..597027c6c9 100644 --- a/xen/arch/arm/percpu.c +++ b/xen/arch/arm/percpu.c @@ -58,11 +58,14 @@ static int cpu_percpu_callback( switch ( action ) { case CPU_UP_PREPARE: - rc = init_percpu_area(cpu); + if ( system_state != SYS_STATE_resume ) + rc = init_percpu_area(cpu); break; case CPU_UP_CANCELED: case CPU_DEAD: - free_percpu_area(cpu); + case CPU_RESUME_FAILED: + if ( system_state != SYS_STATE_suspend ) + free_percpu_area(cpu); break; default: break;
On 3/28/19 1:21 PM, Juergen Gross wrote: > On 28/03/2019 14:01, Volodymyr Babchuk wrote: >> Hello Juergen, >> >> On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: >>> >>> Especially in the scheduler area (schedule.c, cpupool.c) there is a >>> rather complex handling involved when doing suspend and resume. >>> >>> This can be simplified a lot by not performing a complete cpu down and >>> up cycle for the non-boot cpus, but keeping the pure software related >>> state and freeing it only in case a cpu didn't come up again during >>> resume. >>> >>> In summary not only the complexity can be reduced, but the failure >>> tolerance will be even better with this series: With a dedicated hook >>> for failing cpus when resuming it is now possible to survive e.g. a >>> cpupool being left without any cpu after resume by moving its domains >>> to cpupool0. >>> >>> Juergen Gross (6): >>> xen/sched: call cpu_disable_scheduler() via cpu notifier >>> xen: add helper for calling notifier_call_chain() to common/cpu.c >>> xen: add new cpu notifier action CPU_RESUME_FAILED >>> xen: don't free percpu areas during suspend >>> xen/cpupool: simplify suspend/resume handling >>> xen/sched: don't disable scheduler on cpus during suspend >>> >>> xen/arch/arm/smpboot.c | 4 - >>> xen/arch/x86/percpu.c | 3 +- >>> xen/arch/x86/smpboot.c | 3 - >>> xen/common/cpu.c | 61 +++++++------- >>> xen/common/cpupool.c | 131 ++++++++++++----------------- >>> xen/common/schedule.c | 203 +++++++++++++++++++-------------------------- >>> xen/include/xen/cpu.h | 29 ++++--- >>> xen/include/xen/sched-if.h | 1 - >>> 8 files changed, 190 insertions(+), 245 deletions(-) >>> >> >> I tested your patch series on ARM64 platform. We had issue with hard >> affinity - there was assertion failure in sched_credit2 code during >> suspension if one of the vCPUs is pinned to non-0 pCPU. >> >> Seems, your patch series fixes the issue during suspend. But now I'm >> seeing crash during resume: > > Oh, I wasn't aware the suspend/resume is possible on ARM, too. We don't have suspend/resume on Arm... There are series on the ML but I would not consider then bug free (there was the first posting). Cheers,
Hi, On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: > Hello Juergen, > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: >> >> Especially in the scheduler area (schedule.c, cpupool.c) there is a >> rather complex handling involved when doing suspend and resume. >> >> This can be simplified a lot by not performing a complete cpu down and >> up cycle for the non-boot cpus, but keeping the pure software related >> state and freeing it only in case a cpu didn't come up again during >> resume. >> >> In summary not only the complexity can be reduced, but the failure >> tolerance will be even better with this series: With a dedicated hook >> for failing cpus when resuming it is now possible to survive e.g. a >> cpupool being left without any cpu after resume by moving its domains >> to cpupool0. >> >> Juergen Gross (6): >> xen/sched: call cpu_disable_scheduler() via cpu notifier >> xen: add helper for calling notifier_call_chain() to common/cpu.c >> xen: add new cpu notifier action CPU_RESUME_FAILED >> xen: don't free percpu areas during suspend >> xen/cpupool: simplify suspend/resume handling >> xen/sched: don't disable scheduler on cpus during suspend >> >> xen/arch/arm/smpboot.c | 4 - >> xen/arch/x86/percpu.c | 3 +- >> xen/arch/x86/smpboot.c | 3 - >> xen/common/cpu.c | 61 +++++++------- >> xen/common/cpupool.c | 131 ++++++++++++----------------- >> xen/common/schedule.c | 203 +++++++++++++++++++-------------------------- >> xen/include/xen/cpu.h | 29 ++++--- >> xen/include/xen/sched-if.h | 1 - >> 8 files changed, 190 insertions(+), 245 deletions(-) >> > > I tested your patch series on ARM64 platform. We had issue with hard > affinity - there was assertion failure in sched_credit2 code during > suspension if one of the vCPUs is pinned to non-0 pCPU. When you report an error, please make clear what commit you are using and whether you have patches applied on top. In this case, we have no support of suspend/resume on Arm today. So bug report around suspend/resume is a bit confusing to have. It is also more difficult to help when you don't have the full picture as a bug may be in your code and upstream Xen. I saw Juergen suggested a fix, please carry it in whatever series you have. > (XEN) **************************************** > (XEN) Panic on CPU 0: > (XEN) PSCI cpu off failed for CPU0 err=-3 > (XEN) **************************************** PSCI CPU off failing is never a good news. Here, the command has been denied by PSCI monitor. But... why does CPU off is actually called on CPU0? Shouldn't we have turned off the platform instead? > (XEN) > (XEN) Reboot in five seconds... Are the logs below actually a mistaken paste? > (XEN) CPU2 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry > (XEN) CPU 2 booted. > (XEN) Data Abort Trap. Syndrome=0x6 > (XEN) Walking Hypervisor VA 0x0 on CPU2 via TTBR 0x00000000781a8000 > (XEN) 0TH[0x0] = 0x00000000781b0f7f > (XEN) 1ST[0x0] = 0x00000000781aaf7f > (XEN) 2ND[0x0] = 0x0000000000000000 > (XEN) CPU2: Unexpected Trap: Data Abort > (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- > (XEN) CPU: 2 > (XEN) PC: 0000000000233660 _spin_lock+0x1c/0x88 > (XEN) LR: 000000000023365c > (XEN) SP: 000080037ff77d50 > (XEN) CPSR: a00002c9 MODE:64-bit EL2h (Hypervisor, handler) > (XEN) X0: 0000000000000006 X1: 00000000fffffffe X2: 0000000000000000 > (XEN) X3: 0000000000000002 X4: 000080037fc42480 X5: 0000000000000000 > (XEN) X6: 0000000000000080 X7: 000080037ffb0000 X8: 00000000002a1000 > (XEN) X9: 000000000000000a X10: 000080037ff77bf8 X11: 0000000000000032 > (XEN) X12: 0000000000000001 X13: 000000000027fff0 X14: 0000000000000020 > (XEN) X15: 0000000000000000 X16: 0000000000000000 X17: 0000000000000000 > (XEN) X18: 0000000000000000 X19: 0000000000000000 X20: 0000000000000000 > (XEN) X21: 000080037ff7e108 X22: 0000000000000002 X23: 000000000033bc88 > (XEN) X24: 0000000000336020 X25: 0000000000000000 X26: 0000000000000002 > (XEN) X27: 0000000000336000 X28: 0000000000000000 FP: 000080037ff77d50 > (XEN) > (XEN) VTCR_EL2: 80023558 > (XEN) VTTBR_EL2: 0000000000000000 > (XEN) > (XEN) SCTLR_EL2: 30cd183d > (XEN) HCR_EL2: 0000000000000038 > (XEN) TTBR0_EL2: 00000000781a8000 > (XEN) > (XEN) ESR_EL2: 96000006 > (XEN) HPFAR_EL2: 0000000000000000 > (XEN) FAR_EL2: 0000000000000000 > (XEN) > (XEN) Xen stack trace from sp=000080037ff77d50: > (XEN) 000080037ff77d70 00000000002336e8 000080037ff7d000 000000000023e00c > (XEN) 000080037ff77d80 000000000022e90c 000080037ff77e10 0000000000232af8 > (XEN) 0000000000000002 00000000002fbb00 ffffffffffffffff 000000000033cf20 > (XEN) 00000000002a0680 0000000000000001 0000000000000001 0000000000000001 > (XEN) 0000000000000000 000080037ff77e90 000080037ff77e50 00000000ffffffc8 > (XEN) 000000000029f008 00000000002ffc41 000080037ff77e90 0000000000263c68 > (XEN) 000080037ff77e50 0000000000232b6c 0000000000000002 0000000000000004 > (XEN) 0000000000000002 00000000002fbc00 0000000000336448 00000000002fbb00 > (XEN) 000080037ff77e60 0000000000257230 000080037ff77e90 0000000000263c6c > (XEN) 0000000000000002 0000000077e80000 0000000000000000 0000000000000001 > (XEN) 0000000000000000 0000000000000002 0000000000000001 effffffffffaffff > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > (XEN) 0000000000000000 0000000000000000 > (XEN) Xen call trace: > (XEN) [<0000000000233660>] _spin_lock+0x1c/0x88 (PC) > (XEN) [<000000000023365c>] _spin_lock+0x18/0x88 (LR) > (XEN) [<00000000002336e8>] _spin_lock_irq+0x1c/0x24 > (XEN) [<000000000022e90c>] schedule.c#schedule+0xe8/0x74c > (XEN) [<0000000000232af8>] softirq.c#__do_softirq+0xcc/0xe4 > (XEN) [<0000000000232b6c>] do_softirq+0x14/0x1c > (XEN) [<0000000000257230>] idle_loop+0x174/0x188 > (XEN) [<0000000000263c6c>] start_secondary+0x1f4/0x200 > (XEN) [<0000000000000002>] 0000000000000002 > (XEN) > (XEN) > (XEN) **************************************** > (XEN) Panic on CPU 2: > (XEN) CPU2: Unexpected Trap: Data Abort > (XEN) **************************************** > (XEN) > (XEN) Reboot in five seconds... Cheers,
Hi, On Thu, 28 Mar 2019 at 15:21, Juergen Gross <jgross@suse.com> wrote: > > On 28/03/2019 14:01, Volodymyr Babchuk wrote: > > Hello Juergen, > > > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: > >> > >> Especially in the scheduler area (schedule.c, cpupool.c) there is a > >> rather complex handling involved when doing suspend and resume. > >> > >> This can be simplified a lot by not performing a complete cpu down and > >> up cycle for the non-boot cpus, but keeping the pure software related > >> state and freeing it only in case a cpu didn't come up again during > >> resume. > >> > >> In summary not only the complexity can be reduced, but the failure > >> tolerance will be even better with this series: With a dedicated hook > >> for failing cpus when resuming it is now possible to survive e.g. a > >> cpupool being left without any cpu after resume by moving its domains > >> to cpupool0. > >> > >> Juergen Gross (6): > >> xen/sched: call cpu_disable_scheduler() via cpu notifier > >> xen: add helper for calling notifier_call_chain() to common/cpu.c > >> xen: add new cpu notifier action CPU_RESUME_FAILED > >> xen: don't free percpu areas during suspend > >> xen/cpupool: simplify suspend/resume handling > >> xen/sched: don't disable scheduler on cpus during suspend > >> > >> xen/arch/arm/smpboot.c | 4 - > >> xen/arch/x86/percpu.c | 3 +- > >> xen/arch/x86/smpboot.c | 3 - > >> xen/common/cpu.c | 61 +++++++------- > >> xen/common/cpupool.c | 131 ++++++++++++----------------- > >> xen/common/schedule.c | 203 +++++++++++++++++++-------------------------- > >> xen/include/xen/cpu.h | 29 ++++--- > >> xen/include/xen/sched-if.h | 1 - > >> 8 files changed, 190 insertions(+), 245 deletions(-) > >> > > > > I tested your patch series on ARM64 platform. We had issue with hard > > affinity - there was assertion failure in sched_credit2 code during > > suspension if one of the vCPUs is pinned to non-0 pCPU. > > > > Seems, your patch series fixes the issue during suspend. But now I'm > > seeing crash during resume: > > Oh, I wasn't aware the suspend/resume is possible on ARM, too. It is not upstreamed yet, but yes, it is possible. > I need to modify arch/arm/percpu.c like on x86. > Could you test the attached patch if it is working? Yes, it is fixes the issue. Thank you.
On 28/03/2019 14:33, Julien Grall wrote: > Hi, > > On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: >> Hello Juergen, >> >> On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: >>> >>> Especially in the scheduler area (schedule.c, cpupool.c) there is a >>> rather complex handling involved when doing suspend and resume. >>> >>> This can be simplified a lot by not performing a complete cpu down and >>> up cycle for the non-boot cpus, but keeping the pure software related >>> state and freeing it only in case a cpu didn't come up again during >>> resume. >>> >>> In summary not only the complexity can be reduced, but the failure >>> tolerance will be even better with this series: With a dedicated hook >>> for failing cpus when resuming it is now possible to survive e.g. a >>> cpupool being left without any cpu after resume by moving its domains >>> to cpupool0. >>> >>> Juergen Gross (6): >>> xen/sched: call cpu_disable_scheduler() via cpu notifier >>> xen: add helper for calling notifier_call_chain() to common/cpu.c >>> xen: add new cpu notifier action CPU_RESUME_FAILED >>> xen: don't free percpu areas during suspend >>> xen/cpupool: simplify suspend/resume handling >>> xen/sched: don't disable scheduler on cpus during suspend >>> >>> xen/arch/arm/smpboot.c | 4 - >>> xen/arch/x86/percpu.c | 3 +- >>> xen/arch/x86/smpboot.c | 3 - >>> xen/common/cpu.c | 61 +++++++------- >>> xen/common/cpupool.c | 131 ++++++++++++----------------- >>> xen/common/schedule.c | 203 >>> +++++++++++++++++++-------------------------- >>> xen/include/xen/cpu.h | 29 ++++--- >>> xen/include/xen/sched-if.h | 1 - >>> 8 files changed, 190 insertions(+), 245 deletions(-) >>> >> >> I tested your patch series on ARM64 platform. We had issue with hard >> affinity - there was assertion failure in sched_credit2 code during >> suspension if one of the vCPUs is pinned to non-0 pCPU. > When you report an error, please make clear what commit you are using > and whether you have patches applied on top. > > In this case, we have no support of suspend/resume on Arm today. So bug > report around suspend/resume is a bit confusing to have. It is also more > difficult to help when you don't have the full picture as a bug may be > in your code and upstream Xen. > > I saw Juergen suggested a fix, please carry it in whatever series you have. > >> (XEN) **************************************** >> (XEN) Panic on CPU 0: >> (XEN) PSCI cpu off failed for CPU0 err=-3 >> (XEN) **************************************** > > PSCI CPU off failing is never a good news. Here, the command has been > denied by PSCI monitor. But... why does CPU off is actually called on > CPU0? Shouldn't we have turned off the platform instead? Could it be that a scheduler lock is no longer reachable as the percpu memory of another cpu has been released and allocated again? That would be one of the possible results of my series. Juergen
On 28/03/2019 13:37, Juergen Gross wrote: > On 28/03/2019 14:33, Julien Grall wrote: >> Hi, >> >> On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: >>> Hello Juergen, >>> >>> On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: >>>> >>>> Especially in the scheduler area (schedule.c, cpupool.c) there is a >>>> rather complex handling involved when doing suspend and resume. >>>> >>>> This can be simplified a lot by not performing a complete cpu down and >>>> up cycle for the non-boot cpus, but keeping the pure software related >>>> state and freeing it only in case a cpu didn't come up again during >>>> resume. >>>> >>>> In summary not only the complexity can be reduced, but the failure >>>> tolerance will be even better with this series: With a dedicated hook >>>> for failing cpus when resuming it is now possible to survive e.g. a >>>> cpupool being left without any cpu after resume by moving its domains >>>> to cpupool0. >>>> >>>> Juergen Gross (6): >>>> xen/sched: call cpu_disable_scheduler() via cpu notifier >>>> xen: add helper for calling notifier_call_chain() to common/cpu.c >>>> xen: add new cpu notifier action CPU_RESUME_FAILED >>>> xen: don't free percpu areas during suspend >>>> xen/cpupool: simplify suspend/resume handling >>>> xen/sched: don't disable scheduler on cpus during suspend >>>> >>>> xen/arch/arm/smpboot.c | 4 - >>>> xen/arch/x86/percpu.c | 3 +- >>>> xen/arch/x86/smpboot.c | 3 - >>>> xen/common/cpu.c | 61 +++++++------- >>>> xen/common/cpupool.c | 131 ++++++++++++----------------- >>>> xen/common/schedule.c | 203 >>>> +++++++++++++++++++-------------------------- >>>> xen/include/xen/cpu.h | 29 ++++--- >>>> xen/include/xen/sched-if.h | 1 - >>>> 8 files changed, 190 insertions(+), 245 deletions(-) >>>> >>> >>> I tested your patch series on ARM64 platform. We had issue with hard >>> affinity - there was assertion failure in sched_credit2 code during >>> suspension if one of the vCPUs is pinned to non-0 pCPU. >> When you report an error, please make clear what commit you are using >> and whether you have patches applied on top. >> >> In this case, we have no support of suspend/resume on Arm today. So bug >> report around suspend/resume is a bit confusing to have. It is also more >> difficult to help when you don't have the full picture as a bug may be >> in your code and upstream Xen. >> >> I saw Juergen suggested a fix, please carry it in whatever series you have. >> >>> (XEN) **************************************** >>> (XEN) Panic on CPU 0: >>> (XEN) PSCI cpu off failed for CPU0 err=-3 >>> (XEN) **************************************** >> >> PSCI CPU off failing is never a good news. Here, the command has been >> denied by PSCI monitor. But... why does CPU off is actually called on >> CPU0? Shouldn't we have turned off the platform instead? > > Could it be that a scheduler lock is no longer reachable as the percpu > memory of another cpu has been released and allocated again? That would > be one of the possible results of my series. The data abort shown before the panic is potentially the percpu issue. But I don't think it will have the effect to try to turn off CPU0. This looks more an issue in the machine_halt/machine_restart path. Indeed CPU off may rightfully return -3 (DENIED) if the Trusted-OS reside on this CPU. We technically should have checked before that the CPU could be turned off. But it looks like we are missing this code. I vaguely remember to already have pointed out that issue in the past. Cheers, > > > Juergen >
Hello Julien, On Thu, 28 Mar 2019 at 15:33, Julien Grall <julien.grall@arm.com> wrote: > > Hi, > > On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: > > Hello Juergen, > > > > On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: > >> > >> Especially in the scheduler area (schedule.c, cpupool.c) there is a > >> rather complex handling involved when doing suspend and resume. > >> > >> This can be simplified a lot by not performing a complete cpu down and > >> up cycle for the non-boot cpus, but keeping the pure software related > >> state and freeing it only in case a cpu didn't come up again during > >> resume. > >> > >> In summary not only the complexity can be reduced, but the failure > >> tolerance will be even better with this series: With a dedicated hook > >> for failing cpus when resuming it is now possible to survive e.g. a > >> cpupool being left without any cpu after resume by moving its domains > >> to cpupool0. > >> > >> Juergen Gross (6): > >> xen/sched: call cpu_disable_scheduler() via cpu notifier > >> xen: add helper for calling notifier_call_chain() to common/cpu.c > >> xen: add new cpu notifier action CPU_RESUME_FAILED > >> xen: don't free percpu areas during suspend > >> xen/cpupool: simplify suspend/resume handling > >> xen/sched: don't disable scheduler on cpus during suspend > >> > >> xen/arch/arm/smpboot.c | 4 - > >> xen/arch/x86/percpu.c | 3 +- > >> xen/arch/x86/smpboot.c | 3 - > >> xen/common/cpu.c | 61 +++++++------- > >> xen/common/cpupool.c | 131 ++++++++++++----------------- > >> xen/common/schedule.c | 203 +++++++++++++++++++-------------------------- > >> xen/include/xen/cpu.h | 29 ++++--- > >> xen/include/xen/sched-if.h | 1 - > >> 8 files changed, 190 insertions(+), 245 deletions(-) > >> > > > > I tested your patch series on ARM64 platform. We had issue with hard > > affinity - there was assertion failure in sched_credit2 code during > > suspension if one of the vCPUs is pinned to non-0 pCPU. > When you report an error, please make clear what commit you are using > and whether you have patches applied on top. Sure. > In this case, we have no support of suspend/resume on Arm today. So bug > report around suspend/resume is a bit confusing to have. It is also more > difficult to help when you don't have the full picture as a bug may be > in your code and upstream Xen. Agree. But in this case, changes were done to the common code mostly. I assumed that it would be good to check and report it for Arm, even if Arm suspend/resume is not upstreamed yet. Besides, this patch series fixed another issue in the common suspend/resume code. > I saw Juergen suggested a fix, please carry it in whatever series you have. Yes, this patch fixes the issue. We are using patch series by Mirela, that you mentioned earlier, by the way. > > (XEN) **************************************** > > (XEN) Panic on CPU 0: > > (XEN) PSCI cpu off failed for CPU0 err=-3 > > (XEN) **************************************** > > PSCI CPU off failing is never a good news. Here, the command has been > denied by PSCI monitor. But... why does CPU off is actually called on > CPU0? Shouldn't we have turned off the platform instead? I think, this is because CPU1 is performing machine_restart(), so it asked CPU0 to halt itself. > > > (XEN) > > (XEN) Reboot in five seconds... > > Are the logs below actually a mistaken paste? No, this is what I'm seeing in my serial console. > > (XEN) CPU2 will call ARM_SMCCC_ARCH_WORKAROUND_1 on exception entry > > (XEN) CPU 2 booted. > > (XEN) Data Abort Trap. Syndrome=0x6 > > (XEN) Walking Hypervisor VA 0x0 on CPU2 via TTBR 0x00000000781a8000 > > (XEN) 0TH[0x0] = 0x00000000781b0f7f > > (XEN) 1ST[0x0] = 0x00000000781aaf7f > > (XEN) 2ND[0x0] = 0x0000000000000000 > > (XEN) CPU2: Unexpected Trap: Data Abort > > (XEN) ----[ Xen-4.13-unstable arm64 debug=y Not tainted ]---- > > (XEN) CPU: 2 > > (XEN) PC: 0000000000233660 _spin_lock+0x1c/0x88 > > (XEN) LR: 000000000023365c > > (XEN) SP: 000080037ff77d50 > > (XEN) CPSR: a00002c9 MODE:64-bit EL2h (Hypervisor, handler) > > (XEN) X0: 0000000000000006 X1: 00000000fffffffe X2: 0000000000000000 > > (XEN) X3: 0000000000000002 X4: 000080037fc42480 X5: 0000000000000000 > > (XEN) X6: 0000000000000080 X7: 000080037ffb0000 X8: 00000000002a1000 > > (XEN) X9: 000000000000000a X10: 000080037ff77bf8 X11: 0000000000000032 > > (XEN) X12: 0000000000000001 X13: 000000000027fff0 X14: 0000000000000020 > > (XEN) X15: 0000000000000000 X16: 0000000000000000 X17: 0000000000000000 > > (XEN) X18: 0000000000000000 X19: 0000000000000000 X20: 0000000000000000 > > (XEN) X21: 000080037ff7e108 X22: 0000000000000002 X23: 000000000033bc88 > > (XEN) X24: 0000000000336020 X25: 0000000000000000 X26: 0000000000000002 > > (XEN) X27: 0000000000336000 X28: 0000000000000000 FP: 000080037ff77d50 > > (XEN) > > (XEN) VTCR_EL2: 80023558 > > (XEN) VTTBR_EL2: 0000000000000000 > > (XEN) > > (XEN) SCTLR_EL2: 30cd183d > > (XEN) HCR_EL2: 0000000000000038 > > (XEN) TTBR0_EL2: 00000000781a8000 > > (XEN) > > (XEN) ESR_EL2: 96000006 > > (XEN) HPFAR_EL2: 0000000000000000 > > (XEN) FAR_EL2: 0000000000000000 > > (XEN) > > (XEN) Xen stack trace from sp=000080037ff77d50: > > (XEN) 000080037ff77d70 00000000002336e8 000080037ff7d000 000000000023e00c > > (XEN) 000080037ff77d80 000000000022e90c 000080037ff77e10 0000000000232af8 > > (XEN) 0000000000000002 00000000002fbb00 ffffffffffffffff 000000000033cf20 > > (XEN) 00000000002a0680 0000000000000001 0000000000000001 0000000000000001 > > (XEN) 0000000000000000 000080037ff77e90 000080037ff77e50 00000000ffffffc8 > > (XEN) 000000000029f008 00000000002ffc41 000080037ff77e90 0000000000263c68 > > (XEN) 000080037ff77e50 0000000000232b6c 0000000000000002 0000000000000004 > > (XEN) 0000000000000002 00000000002fbc00 0000000000336448 00000000002fbb00 > > (XEN) 000080037ff77e60 0000000000257230 000080037ff77e90 0000000000263c6c > > (XEN) 0000000000000002 0000000077e80000 0000000000000000 0000000000000001 > > (XEN) 0000000000000000 0000000000000002 0000000000000001 effffffffffaffff > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 0000000000000000 0000000000000000 > > (XEN) 0000000000000000 0000000000000000 > > (XEN) Xen call trace: > > (XEN) [<0000000000233660>] _spin_lock+0x1c/0x88 (PC) > > (XEN) [<000000000023365c>] _spin_lock+0x18/0x88 (LR) > > (XEN) [<00000000002336e8>] _spin_lock_irq+0x1c/0x24 > > (XEN) [<000000000022e90c>] schedule.c#schedule+0xe8/0x74c > > (XEN) [<0000000000232af8>] softirq.c#__do_softirq+0xcc/0xe4 > > (XEN) [<0000000000232b6c>] do_softirq+0x14/0x1c > > (XEN) [<0000000000257230>] idle_loop+0x174/0x188 > > (XEN) [<0000000000263c6c>] start_secondary+0x1f4/0x200 > > (XEN) [<0000000000000002>] 0000000000000002 > > (XEN) > > (XEN) > > (XEN) **************************************** > > (XEN) Panic on CPU 2: > > (XEN) CPU2: Unexpected Trap: Data Abort > > (XEN) **************************************** > > (XEN) > > (XEN) Reboot in five seconds... > > Cheers, > > -- > Julien Grall
Hi, On 3/28/19 1:56 PM, Volodymyr Babchuk wrote: > On Thu, 28 Mar 2019 at 15:33, Julien Grall <julien.grall@arm.com> wrote: >> On 3/28/19 1:01 PM, Volodymyr Babchuk wrote: >>> On Thu, 28 Mar 2019 at 14:09, Juergen Gross <jgross@suse.com> wrote: > >>> (XEN) **************************************** >>> (XEN) Panic on CPU 0: >>> (XEN) PSCI cpu off failed for CPU0 err=-3 >>> (XEN) **************************************** >> >> PSCI CPU off failing is never a good news. Here, the command has been >> denied by PSCI monitor. But... why does CPU off is actually called on >> CPU0? Shouldn't we have turned off the platform instead? > I think, this is because CPU1 is performing machine_restart(), so it > asked CPU0 to halt itself. The PSCI call SYSTEM_OFF/SYSTEM_RESET requires all the CPUs to be in a known state. The PSCI spec actually suggest to turn all the CPUs but one off. Another alternative is to put them in a quiescent state. CPU_OFF will return -3 (i.e DENIED) if the Trusted OS is Uniprocessor and resides on the core that you are about to turn off. I assume your platform have a TOS UP and currently resides on CPU0. SYSTEM_OFF/SYSTEM_RESET call can be done from any CPUs. If we want to avoid the problem with CPU_OFF, then the best options is to put the all CPUs but one in a quiescent state (something like while (1) cpu_relax/wfi). On a side node, we should probably want to remove the panic in call_psci_cpu_off as this will be used by the suspend code. Indeed, the trusted OS may not reside on the boot CPU, so you may hit the panic as well. > >> >>> (XEN) >>> (XEN) Reboot in five seconds... >> >> Are the logs below actually a mistaken paste? > No, this is what I'm seeing in my serial console. I guess this is happening because we recurse in machine_halt(). We probably want to drop any panic in that code path. Cheers,
On Thu, 2019-03-28 at 15:56 +0200, Volodymyr Babchuk wrote: > On Thu, 28 Mar 2019 at 15:33, Julien Grall <julien.grall@arm.com> > wrote: > > > Are the logs below actually a mistaken paste? > No, this is what I'm seeing in my serial console. > Err, sorry, I'm a bit confused now... So, if you use the ARM suspend/resume patches + Juergen series + Juergen fixup patch in this subthread, do you still have issues or is everything allright? If you still have issues, which splat are you seeing? Regards, Dario
Hello Dario, On Thu, 28 Mar 2019 at 16:54, Dario Faggioli <dfaggioli@suse.com> wrote: > > On Thu, 2019-03-28 at 15:56 +0200, Volodymyr Babchuk wrote: > > On Thu, 28 Mar 2019 at 15:33, Julien Grall <julien.grall@arm.com> > > wrote: > > > > > Are the logs below actually a mistaken paste? > > No, this is what I'm seeing in my serial console. > > > Err, sorry, I'm a bit confused now... > > So, if you use the ARM suspend/resume patches + Juergen series + > Juergen fixup patch in this subthread, do you still have issues or is > everything allright? No, Juergen's fixup fixed the issue. So, everything is perfectly fine. That was discussion of unrelated issue in ARM platform code. I think, we need to drop off x86 folks from CC. Sorry for the noise.