Re: [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer

[David Gibson <dgibson@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2789 bytes --]

On Tue, 19 Apr 2016 19:26:28 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> As the TOC pointer can be corrupted by the main program,
> we must restore it in the exception handler.
> 
> As we know where we are loaded, we can now compute it easily.
> 
> To compute it only in the common part of the exception handler
> (call_handler), store the address of call_handler at an absolute
> address in memory to be able to call the handler from the exception
> table (as SLOF does).
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

So, this looks ok as long as the unit tests are built with a single
TOC.  The more stricly correct approach would be to actually load the
correct TOC pointer for the specific handler function you're going to
jump to.

That would need different paths for ABIv1 and v2, though (in practice
ppc64 vs. ppc64le).  With ABIv1 (ppc64) you'd need to load the TOC
pointer from the procedure descriptor into r2 before jumping to the
code address.  With ABIv2 (ppc64le), I believe there's both a "far" and
"near" entry point to the function - the "far" one has a few
instructions to load the correct TOC before continuing onto the "near"
version.  I forget which entry point the plain symbol references.  If
it's the "far" one, the current code is probably already correct.

> ---
>  powerpc/cstart64.S | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index c47b67d..1ddfa13 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -13,6 +13,8 @@
>  
>  #include "spapr.h"
>  
> +#define P_HANDLER	0x2ff8
> +
>  .section .init
>  
>  /*
> @@ -46,6 +48,11 @@ start:
>  	add	r4, r4, r31
>  	bl	relocate
>  
> +	/* compute address of call_handler */
> +
> +	LOAD_REG_ADDR(r4, call_handler)
> +	std	r4, P_HANDLER(0)
> +
>  	/* relocate vector table to base address 0x0 (MSR_IP = 0) */
>  
>  	/* source: r4, dest end: r5, destination: r6 */
> @@ -166,6 +173,12 @@ call_handler:
>  	mfsrr1	r0
>  	std	r0, _MSR(r1)
>  
> +	/* restore TOC pointer */
> +
> +	LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> +	ld	r2, (p_toc - start)(r31)
> +	add	r2, r2, r31
> +
>  	/* FIXME: build stack frame */
>  
>  	/* call generic handler */
> @@ -221,7 +234,7 @@ call_handler:
>  	mfctr	r0
>  	std	r0,_CTR(r1)
>  
> -	LOAD_REG_ADDR(r0, call_handler)
> +	ld	r0, P_HANDLER(0)
>  	mtctr	r0
>  
>  	li	r0,\vec
> @@ -245,3 +258,5 @@ VECTOR(0x900)
>  	.align 7
>  	.globl __end_interrupts
>  __end_interrupts:
> +	.org	P_HANDLER
> +	.llong	0
> -- 
> 2.5.5
> 


-- 
David Gibson <dgibson@redhat.com>
Senior Software Engineer, Virtualization, Red Hat

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]