ACK/cmnt: [PATCH] powerpc/64: Fix HMI exception on LE with CONFIG_RELOCATABLE=y

[Stefan Bader]

On 20.07.2017 01:41, Manoj Iyer wrote:
> From: Michael Ellerman <mpe at ellerman.id.au>
> 
> Prior to commit 2337d207288f ("powerpc/64: CONFIG_RELOCATABLE support for hmi
> interrupts"), the branch from hmi_exception_early() to hmi_exception_realmode()
> was just a bl hmi_exception_realmode, which the linker would turn into a bl to
> the local entry point of hmi_exception_realmode. This was broken when
> CONFIG_RELOCATABLE=y because hmi_exception_realmode() is not in the low part of
> the kernel text that is copied down to 0x0.
> 
> But in fixing that, we added a new bug on little endian kernels. Because the
> branch is now a bctrl when CONFIG_RELOCATABLE=y, we branch to the global entry
> point of hmi_exception_realmode(). The global entry point must be called with
> r12 containing the address of hmi_exception_realmode(), because it uses that
> value to calculate the TOC value (r2).
> 
> This may manifest as a checkstop, because we take a junk value from r12 which
> came from HSRR1, add a small constant to it and then use that as the TOC
> pointer. The HSRR1 value will have 0x9 as the top nibble, which puts it above
> RAM and somewhere in MMIO space.
> 
> Fix it by changing the BRANCH_LINK_TO_FAR() macro to always use r12 to load the
> label we're branching to. This means r12 will be setup correctly on LE, fixing
> this bug, and r12 is also volatile across function calls on BE so it's a good
> choice anyway.
> 
> BugLink: https://launchpad.net/bugs/1684054
> 
> Fixes: 2337d207288f ("powerpc/64: CONFIG_RELOCATABLE support for hmi interrupts")
> Reported-by: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
> Acked-by: Nicholas Piggin <npiggin at gmail.com>
> Signed-off-by: Michael Ellerman <mpe at ellerman.id.au>
> (cherry picked from commit be5c5e843c4afa1c8397cb740b6032bd4142f32d)
> Signed-off-by: Manoj Iyer <manoj.iyer at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>

> ---

Added a nomination on the bug report for Zesty. Btw, reports for the kernel have
always been for the "linux" package. ;)

-Stefan

>  arch/powerpc/include/asm/exception-64s.h | 8 ++++----
>  arch/powerpc/kernel/exceptions-64s.S     | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
> index 14752eee3d0c..ed3beadd2cc5 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -236,9 +236,9 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
>  	mtctr	reg;							\
>  	bctr
>  
> -#define BRANCH_LINK_TO_FAR(reg, label)					\
> -	__LOAD_FAR_HANDLER(reg, label);					\
> -	mtctr	reg;							\
> +#define BRANCH_LINK_TO_FAR(label)					\
> +	__LOAD_FAR_HANDLER(r12, label);					\
> +	mtctr	r12;							\
>  	bctrl
>  
>  /*
> @@ -265,7 +265,7 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
>  #define BRANCH_TO_COMMON(reg, label)					\
>  	b	label
>  
> -#define BRANCH_LINK_TO_FAR(reg, label)					\
> +#define BRANCH_LINK_TO_FAR(label)					\
>  	bl	label
>  
>  #define BRANCH_TO_KVM(reg, label)					\
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 79318275e507..78456131385d 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -986,7 +986,7 @@ TRAMP_REAL_BEGIN(hmi_exception_early)
>  	EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN)
>  	EXCEPTION_PROLOG_COMMON_3(0xe60)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
> -	BRANCH_LINK_TO_FAR(r4, hmi_exception_realmode)
> +	BRANCH_LINK_TO_FAR(hmi_exception_realmode) /* Function call ABI */
>  	/* Windup the stack. */
>  	/* Move original HSRR0 and HSRR1 into the respective regs */
>  	ld	r9,_MSR(r1)
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20170721/71c465ea/attachment.sig>