| Message ID | 20200326032420.27220-11-pasha.tatashin@soleen.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: MMU enabled kexec relocation | expand |
Hi Pavel, On 26/03/2020 03:24, Pavel Tatashin wrote: > Change argument types from unsigned long to a more descriptive > phys_addr_t. For 'entry', which is a physical addresses, sure... > diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h > index ed50e9587ad8..38cbd4068019 100644 > --- a/arch/arm64/kernel/cpu-reset.h > +++ b/arch/arm64/kernel/cpu-reset.h > @@ -10,17 +10,17 @@ > > #include <asm/virt.h> > > -void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry, > - unsigned long arg0, unsigned long arg1, unsigned long arg2); > +void __cpu_soft_restart(phys_addr_t el2_switch, phys_addr_t entry, > + phys_addr_t arg0, phys_addr_t arg1, phys_addr_t arg2); This looks weird because its re-using the hyp-stub API, because it might call the hyp-stub from the idmap. entry is passed in, so this isn't tied to kexec. Without tying it to kexec, how do you know arg2 is a physical address? I think it tried to be re-usable because 32bit has more users for this. arg0-2 are unsigned long because the hyp-stub is just moving general purpose registers around. This is to avoid casting? Sure, its only got one caller. This thing evolved because the platform-has-EL2 and kdump-while-KVM-was-running code was bolted on as they were discovered. > -static inline void __noreturn cpu_soft_restart(unsigned long entry, > - unsigned long arg0, > - unsigned long arg1, > - unsigned long arg2) > +static inline void __noreturn cpu_soft_restart(phys_addr_t entry, > + phys_addr_t arg0, > + phys_addr_t arg1, > + phys_addr_t arg2) > { > typeof(__cpu_soft_restart) *restart; > > - unsigned long el2_switch = !is_kernel_in_hyp_mode() && > + phys_addr_t el2_switch = !is_kernel_in_hyp_mode() && > is_hyp_mode_available(); What on earth happened here!? > restart = (void *)__pa_symbol(__cpu_soft_restart); Thanks, James
On Wed, Apr 29, 2020 at 1:01 PM James Morse <james.morse@arm.com> wrote: > > Hi Pavel, > > On 26/03/2020 03:24, Pavel Tatashin wrote: > > Change argument types from unsigned long to a more descriptive > > phys_addr_t. > > For 'entry', which is a physical addresses, sure... > > > diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h > > index ed50e9587ad8..38cbd4068019 100644 > > --- a/arch/arm64/kernel/cpu-reset.h > > +++ b/arch/arm64/kernel/cpu-reset.h > > @@ -10,17 +10,17 @@ > > > > #include <asm/virt.h> > > > > -void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry, > > - unsigned long arg0, unsigned long arg1, unsigned long arg2); > > > +void __cpu_soft_restart(phys_addr_t el2_switch, phys_addr_t entry, > > + phys_addr_t arg0, phys_addr_t arg1, phys_addr_t arg2); > > This looks weird because its re-using the hyp-stub API, because it might call the hyp-stub > from the idmap. entry is passed in, so this isn't tied to kexec. Without tying it to > kexec, how do you know arg2 is a physical address? > I think it tried to be re-usable because 32bit has more users for this. I will drop this patch. It was intended as a cleanup from suggestions in earlier versions of this series, but I see it is not really needed. Thank you, Pasha
diff --git a/arch/arm64/kernel/cpu-reset.h b/arch/arm64/kernel/cpu-reset.h index ed50e9587ad8..38cbd4068019 100644 --- a/arch/arm64/kernel/cpu-reset.h +++ b/arch/arm64/kernel/cpu-reset.h @@ -10,17 +10,17 @@ #include <asm/virt.h> -void __cpu_soft_restart(unsigned long el2_switch, unsigned long entry, - unsigned long arg0, unsigned long arg1, unsigned long arg2); +void __cpu_soft_restart(phys_addr_t el2_switch, phys_addr_t entry, + phys_addr_t arg0, phys_addr_t arg1, phys_addr_t arg2); -static inline void __noreturn cpu_soft_restart(unsigned long entry, - unsigned long arg0, - unsigned long arg1, - unsigned long arg2) +static inline void __noreturn cpu_soft_restart(phys_addr_t entry, + phys_addr_t arg0, + phys_addr_t arg1, + phys_addr_t arg2) { typeof(__cpu_soft_restart) *restart; - unsigned long el2_switch = !is_kernel_in_hyp_mode() && + phys_addr_t el2_switch = !is_kernel_in_hyp_mode() && is_hyp_mode_available(); restart = (void *)__pa_symbol(__cpu_soft_restart);
Change argument types from unsigned long to a more descriptive phys_addr_t. Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com> --- arch/arm64/kernel/cpu-reset.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)