public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: james.morse@arm•com (James Morse)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH v17 01/13] arm64: Add back cpu reset routines
Date: Thu, 09 Jun 2016 15:50:36 +0100	[thread overview]
Message-ID: <5759823C.3070208@arm.com> (raw)
In-Reply-To: <78ef589316b3f538938324efcbbb0361519c3393.1464974516.git.geoff@infradead.org>

Hi Geoff,

On 03/06/16 19:13, Geoff Levand wrote:
> Commit 68234df4ea7939f98431aa81113fbdce10c4a84b (arm64: kill flush_cache_all())
> removed the global arm64 routines cpu_reset() and cpu_soft_restart() needed by
> the arm64 kexec and kdump support.  Add simplified versions of those two
> routines back with some changes needed for kexec in the new files cpu_reset.S,
> and cpu_reset.h.
> 
> When a CPU is reset it needs to be put into the exception level it had when it
> entered the kernel. Update cpu_soft_restart() to accept an argument which
> signals if the reset address needs to be entered at EL1 or EL2, and add a
> new hypercall HVC_SOFT_RESTART which is used for the EL2 switch.
> 
> Signed-off-by: Geoff Levand <geoff@infradead•org>


> diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
> new file mode 100644
> index 0000000..c321957
> --- /dev/null
> +++ b/arch/arm64/kernel/cpu-reset.S

> +ENTRY(__cpu_soft_restart)
> +	/* Clear sctlr_el1 flags. */
> +	mrs	x12, sctlr_el1
> +	ldr	x13, =SCTLR_ELx_FLAGS
> +	bic	x12, x12, x13
> +	msr	sctlr_el1, x12
> +	isb
> +
> +	cbz	x0, 1f				// el2_switch?
> +	mov	x0, #HVC_SOFT_RESTART
> +	hvc	#0				// no return
> +
> +1:	mov	x18, x1				// entry
> +	mov	x0, x2				// arg0
> +	mov	x1, x3				// arg1
> +	mov	x2, x4				// arg2
> +	ret	x18

Why ret not br?


> +ENDPROC(__cpu_soft_restart)
> +
> +.popsection
> diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h
> new file mode 100644
> index 0000000..5a5ea0a
> --- /dev/null
> +++ b/arch/arm64/kernel/cpu-reset.h
> @@ -0,0 +1,29 @@
> +/*
> + * CPU reset routines
> + *
> + * Copyright (C) 2015 Huawei Futurewei Technologies.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _ARM64_CPU_RESET_H
> +#define _ARM64_CPU_RESET_H
> +
> +#include <asm/virt.h>
> +
> +void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry,
> +	unsigned long arg0, unsigned long arg1, unsigned long arg2);
> +
> +static inline void __noreturn cpu_soft_restart(unsigned long el2_switch,
> +	unsigned long entry, unsigned long arg0, unsigned long arg1,
> +	unsigned long arg2)

What is the last arg for? machine_kexec() passes zero, but
arm64_relocate_new_kernel() never reads this value..


> +{
> +	typeof(__cpu_soft_restart) *restart;
> +	restart = (void *)virt_to_phys(__cpu_soft_restart);
> +	restart(el2_switch, entry, arg0, arg1, arg2);

This confuses me each time I see it, I think it would be clearer if the
'cpu_install_idmap()' call were moved into this function. Any other user of this
function would need to do the same.

By the end of the series, the caller of this has:
> is_kernel_in_hyp_mode() ? 0 : (in_crash_kexec ? 0 : is_hyp_mode_available())
which is difficult to read, I had to write out the values to work it out.

I thinks it makes more sense to move the hyp-aware logic into this
cpu_soft_restart(), obviously kdump still needs a 'skip el2 jump' flag.


> +	unreachable();
> +}
> +
> +#endif
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index 8727f44..a129e57 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -71,8 +71,17 @@ el1_sync:
>  	msr	vbar_el2, x1
>  	b	9f
>  
> +2:	cmp	x0, #HVC_SOFT_RESTART
> +	b.ne	3f
> +	mov	x0, x2
> +	mov	x2, x4
> +	mov	x4, x1
> +	mov	x1, x3
> +	blr	x4

blr not branch? If we ever did return from here, wouldn't we run the 'entry'
function again at EL1?


> +	b	9f
> +
>  	/* Someone called kvm_call_hyp() against the hyp-stub... */
> -2:	mov     x0, #ARM_EXCEPTION_HYP_GONE
> +3:	mov	x0, #ARM_EXCEPTION_HYP_GONE
>  
>  9:	eret
>  ENDPROC(el1_sync)
> 

For what its worth:

Reviewed-by: James Morse <james.morse@arm•com>


Thanks,

James

  reply	other threads:[~2016-06-09 14:50 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-03 18:13 [PATCH v17 00/13] arm64 kexec kernel patches Geoff Levand
2016-06-03 18:13 ` [PATCH v17 02/13] arm64: Add cpus_are_stuck_in_kernel Geoff Levand
2016-06-09 14:51   ` James Morse
2016-06-09 18:38     ` Geoff Levand
2016-06-03 18:13 ` [PATCH v17 01/13] arm64: Add back cpu reset routines Geoff Levand
2016-06-09 14:50   ` James Morse [this message]
2016-06-09 18:25     ` Geoff Levand
2016-06-03 18:13 ` [PATCH v17 09/13] arm64: kdump: add kdump support Geoff Levand
2016-06-03 18:13 ` [PATCH v17 04/13] arm64/kexec: Add pr_debug output Geoff Levand
2016-06-03 18:13 ` [PATCH v17 13/13] Documentation: dt: usable-memory and elfcorehdr nodes for arm64 kexec Geoff Levand
2016-06-03 18:13 ` [PATCH v17 12/13] arm64: kdump: update a kernel doc Geoff Levand
2016-06-03 18:13 ` [PATCH v17 06/13] arm64: kdump: reserve memory for crash dump kernel Geoff Levand
2016-06-03 18:13 ` [PATCH v17 07/13] arm64: limit memory regions based on DT property, usable-memory Geoff Levand
2016-06-03 18:13 ` [PATCH v17 10/13] arm64: kdump: add VMCOREINFO for user-space coredump tools Geoff Levand
2016-06-09 15:09   ` Suzuki K Poulose
2016-06-09 15:17     ` Suzuki K Poulose
2016-06-09 23:19     ` AKASHI Takahiro
2016-06-03 18:13 ` [PATCH v17 03/13] arm64/kexec: Add core kexec support Geoff Levand
2016-06-03 18:13 ` [PATCH v17 05/13] arm64/kexec: Enable kexec in the arm64 defconfig Geoff Levand
2016-06-03 18:13 ` [PATCH v17 08/13] arm64: kdump: implement machine_crash_shutdown() Geoff Levand
2016-06-03 18:13 ` [PATCH v17 11/13] arm64: kdump: enable kdump in the arm64 defconfig Geoff Levand
2016-06-07  1:36 ` [PATCH v17 00/13] arm64 kexec kernel patches AKASHI Takahiro
2016-06-08  6:31   ` AKASHI Takahiro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5759823C.3070208@arm.com \
    --to=james.morse@arm$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox