* [kvm-unit-tests PATCH 0/2] Fix TOC value in exception handler
@ 2016-04-19 17:26 ` Laurent Vivier
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-19 17:26 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
As noticed by Thomas, if the TOC pointer (r2) is corrupted,
the code from the exception table cannot call the common exception handler
function.
This series tries to fix that by calling call_handler without using
the TOC pointer but a pointer to the function stored at an absolute
address in memory (as SLOF does), and then by restoring the value of
r2 before calling the user registered exception handler.
An easy way to to compute the TOC address seems to use directly the value
of the load address used by QEMU instead of computing it from the PC.
Laurent Vivier (2):
powerpc: use well known kernel start address
powerpc: restore TOC pointer
powerpc/boot_rom.S | 3 ++-
powerpc/cstart64.S | 23 +++++++++++++++++++----
powerpc/spapr.h | 6 ++++++
3 files changed, 27 insertions(+), 5 deletions(-)
create mode 100644 powerpc/spapr.h
--
2.5.5
^ permalink raw reply [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 0/2] Fix TOC value in exception handler
@ 2016-04-19 17:26 ` Laurent Vivier
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-19 17:26 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
As noticed by Thomas, if the TOC pointer (r2) is corrupted,
the code from the exception table cannot call the common exception handler
function.
This series tries to fix that by calling call_handler without using
the TOC pointer but a pointer to the function stored at an absolute
address in memory (as SLOF does), and then by restoring the value of
r2 before calling the user registered exception handler.
An easy way to to compute the TOC address seems to use directly the value
of the load address used by QEMU instead of computing it from the PC.
Laurent Vivier (2):
powerpc: use well known kernel start address
powerpc: restore TOC pointer
powerpc/boot_rom.S | 3 ++-
powerpc/cstart64.S | 23 +++++++++++++++++++----
powerpc/spapr.h | 6 ++++++
3 files changed, 27 insertions(+), 5 deletions(-)
create mode 100644 powerpc/spapr.h
--
2.5.5
^ permalink raw reply [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
2016-04-19 17:26 ` Laurent Vivier
@ 2016-04-19 17:26 ` Laurent Vivier
-1 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-19 17:26 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
As we know where QEMU will load the kernel, it seems an useless
pain to try to compute it instead of using the well known value.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/boot_rom.S | 3 ++-
powerpc/cstart64.S | 6 +++---
powerpc/spapr.h | 6 ++++++
3 files changed, 11 insertions(+), 4 deletions(-)
create mode 100644 powerpc/spapr.h
diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
index ae2c08d..266d61f 100644
--- a/powerpc/boot_rom.S
+++ b/powerpc/boot_rom.S
@@ -1,4 +1,5 @@
-#define SPAPR_KERNEL_LOAD_ADDR 0x400000
+#include "spapr.h"
+
.text
.globl start
start:
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index ceb6397..c47b67d 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -11,6 +11,8 @@
#include <asm/rtas.h>
#include <asm/ptrace.h>
+#include "spapr.h"
+
.section .init
/*
@@ -25,9 +27,7 @@ start:
* so we just linked at zero. This means the first thing to do is
* to find our stack and toc, and then do a relocate.
*/
- bl 0f
-0: mflr r31
- subi r31, r31, 0b - start /* QEMU's kernel load address */
+ LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
ld r1, (p_stack - start)(r31)
ld r2, (p_toc - start)(r31)
add r1, r1, r31
diff --git a/powerpc/spapr.h b/powerpc/spapr.h
new file mode 100644
index 0000000..b41aece
--- /dev/null
+++ b/powerpc/spapr.h
@@ -0,0 +1,6 @@
+#ifndef _ASMPOWERPC_SPAPR_H_
+#define _ASMPOWERPC_SPAPR_H_
+
+#define SPAPR_KERNEL_LOAD_ADDR 0x400000
+
+#endif /* _ASMPOWERPC_SPAPR_H_ */
--
2.5.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
@ 2016-04-19 17:26 ` Laurent Vivier
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-19 17:26 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
As we know where QEMU will load the kernel, it seems an useless
pain to try to compute it instead of using the well known value.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
powerpc/boot_rom.S | 3 ++-
powerpc/cstart64.S | 6 +++---
powerpc/spapr.h | 6 ++++++
3 files changed, 11 insertions(+), 4 deletions(-)
create mode 100644 powerpc/spapr.h
diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
index ae2c08d..266d61f 100644
--- a/powerpc/boot_rom.S
+++ b/powerpc/boot_rom.S
@@ -1,4 +1,5 @@
-#define SPAPR_KERNEL_LOAD_ADDR 0x400000
+#include "spapr.h"
+
.text
.globl start
start:
diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
index ceb6397..c47b67d 100644
--- a/powerpc/cstart64.S
+++ b/powerpc/cstart64.S
@@ -11,6 +11,8 @@
#include <asm/rtas.h>
#include <asm/ptrace.h>
+#include "spapr.h"
+
.section .init
/*
@@ -25,9 +27,7 @@ start:
* so we just linked at zero. This means the first thing to do is
* to find our stack and toc, and then do a relocate.
*/
- bl 0f
-0: mflr r31
- subi r31, r31, 0b - start /* QEMU's kernel load address */
+ LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
ld r1, (p_stack - start)(r31)
ld r2, (p_toc - start)(r31)
add r1, r1, r31
diff --git a/powerpc/spapr.h b/powerpc/spapr.h
new file mode 100644
index 0000000..b41aece
--- /dev/null
+++ b/powerpc/spapr.h
@@ -0,0 +1,6 @@
+#ifndef _ASMPOWERPC_SPAPR_H_
+#define _ASMPOWERPC_SPAPR_H_
+
+#define SPAPR_KERNEL_LOAD_ADDR 0x400000
+
+#endif /* _ASMPOWERPC_SPAPR_H_ */
--
2.5.5
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer
2016-04-19 17:26 ` Laurent Vivier
@ 2016-04-19 17:26 ` Laurent Vivier
-1 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-19 17:26 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
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>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer
@ 2016-04-19 17:26 ` Laurent Vivier
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-19 17:26 UTC (permalink / raw)
To: kvm, kvm-ppc; +Cc: drjones, thuth, dgibson, pbonzini, Laurent Vivier
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>
---
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
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
2016-04-19 17:26 ` Laurent Vivier
@ 2016-04-20 2:01 ` David Gibson
-1 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-04-20 2:01 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]
On Tue, 19 Apr 2016 19:26:27 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> As we know where QEMU will load the kernel, it seems an useless
> pain to try to compute it instead of using the well known value.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Hmm.. I'm a little dubious about whether it's a good idea to require
that kernel load address to remain fixed in qemu. I don't think it's
something we really have an interface guarantee for.
It might be ok given that we're just talking about the test shim here,
though.
> ---
> powerpc/boot_rom.S | 3 ++-
> powerpc/cstart64.S | 6 +++---
> powerpc/spapr.h | 6 ++++++
> 3 files changed, 11 insertions(+), 4 deletions(-)
> create mode 100644 powerpc/spapr.h
>
> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
> index ae2c08d..266d61f 100644
> --- a/powerpc/boot_rom.S
> +++ b/powerpc/boot_rom.S
> @@ -1,4 +1,5 @@
> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +#include "spapr.h"
> +
> .text
> .globl start
> start:
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index ceb6397..c47b67d 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -11,6 +11,8 @@
> #include <asm/rtas.h>
> #include <asm/ptrace.h>
>
> +#include "spapr.h"
> +
> .section .init
>
> /*
> @@ -25,9 +27,7 @@ start:
> * so we just linked at zero. This means the first thing to do is
> * to find our stack and toc, and then do a relocate.
> */
> - bl 0f
> -0: mflr r31
> - subi r31, r31, 0b - start /* QEMU's kernel load address */
> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> ld r1, (p_stack - start)(r31)
> ld r2, (p_toc - start)(r31)
> add r1, r1, r31
> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> new file mode 100644
> index 0000000..b41aece
> --- /dev/null
> +++ b/powerpc/spapr.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASMPOWERPC_SPAPR_H_
> +#define _ASMPOWERPC_SPAPR_H_
> +
> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +
> +#endif /* _ASMPOWERPC_SPAPR_H_ */
> --
> 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
@ 2016-04-20 2:01 ` David Gibson
0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-04-20 2:01 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]
On Tue, 19 Apr 2016 19:26:27 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> As we know where QEMU will load the kernel, it seems an useless
> pain to try to compute it instead of using the well known value.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Hmm.. I'm a little dubious about whether it's a good idea to require
that kernel load address to remain fixed in qemu. I don't think it's
something we really have an interface guarantee for.
It might be ok given that we're just talking about the test shim here,
though.
> ---
> powerpc/boot_rom.S | 3 ++-
> powerpc/cstart64.S | 6 +++---
> powerpc/spapr.h | 6 ++++++
> 3 files changed, 11 insertions(+), 4 deletions(-)
> create mode 100644 powerpc/spapr.h
>
> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
> index ae2c08d..266d61f 100644
> --- a/powerpc/boot_rom.S
> +++ b/powerpc/boot_rom.S
> @@ -1,4 +1,5 @@
> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +#include "spapr.h"
> +
> .text
> .globl start
> start:
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index ceb6397..c47b67d 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -11,6 +11,8 @@
> #include <asm/rtas.h>
> #include <asm/ptrace.h>
>
> +#include "spapr.h"
> +
> .section .init
>
> /*
> @@ -25,9 +27,7 @@ start:
> * so we just linked at zero. This means the first thing to do is
> * to find our stack and toc, and then do a relocate.
> */
> - bl 0f
> -0: mflr r31
> - subi r31, r31, 0b - start /* QEMU's kernel load address */
> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> ld r1, (p_stack - start)(r31)
> ld r2, (p_toc - start)(r31)
> add r1, r1, r31
> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> new file mode 100644
> index 0000000..b41aece
> --- /dev/null
> +++ b/powerpc/spapr.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASMPOWERPC_SPAPR_H_
> +#define _ASMPOWERPC_SPAPR_H_
> +
> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +
> +#endif /* _ASMPOWERPC_SPAPR_H_ */
> --
> 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
2016-04-20 2:01 ` David Gibson
@ 2016-04-20 5:48 ` Laurent Vivier
-1 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-20 5:48 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
On 20/04/2016 04:01, David Gibson wrote:
> On Tue, 19 Apr 2016 19:26:27 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> As we know where QEMU will load the kernel, it seems an useless
>> pain to try to compute it instead of using the well known value.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>
> Hmm.. I'm a little dubious about whether it's a good idea to require
> that kernel load address to remain fixed in qemu. I don't think it's
> something we really have an interface guarantee for.
>
> It might be ok given that we're just talking about the test shim here,
> though.
As it is hardcoded into boot_rom.S I see no obvious reason to not
hardcode it in cstart64.S.
Laurent
>> ---
>> powerpc/boot_rom.S | 3 ++-
>> powerpc/cstart64.S | 6 +++---
>> powerpc/spapr.h | 6 ++++++
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>> create mode 100644 powerpc/spapr.h
>>
>> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
>> index ae2c08d..266d61f 100644
>> --- a/powerpc/boot_rom.S
>> +++ b/powerpc/boot_rom.S
>> @@ -1,4 +1,5 @@
>> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
>> +#include "spapr.h"
>> +
>> .text
>> .globl start
>> start:
>> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
>> index ceb6397..c47b67d 100644
>> --- a/powerpc/cstart64.S
>> +++ b/powerpc/cstart64.S
>> @@ -11,6 +11,8 @@
>> #include <asm/rtas.h>
>> #include <asm/ptrace.h>
>>
>> +#include "spapr.h"
>> +
>> .section .init
>>
>> /*
>> @@ -25,9 +27,7 @@ start:
>> * so we just linked at zero. This means the first thing to do is
>> * to find our stack and toc, and then do a relocate.
>> */
>> - bl 0f
>> -0: mflr r31
>> - subi r31, r31, 0b - start /* QEMU's kernel load address */
>> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
>> ld r1, (p_stack - start)(r31)
>> ld r2, (p_toc - start)(r31)
>> add r1, r1, r31
>> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
>> new file mode 100644
>> index 0000000..b41aece
>> --- /dev/null
>> +++ b/powerpc/spapr.h
>> @@ -0,0 +1,6 @@
>> +#ifndef _ASMPOWERPC_SPAPR_H_
>> +#define _ASMPOWERPC_SPAPR_H_
>> +
>> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
>> +
>> +#endif /* _ASMPOWERPC_SPAPR_H_ */
>> --
>> 2.5.5
>>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
@ 2016-04-20 5:48 ` Laurent Vivier
0 siblings, 0 replies; 18+ messages in thread
From: Laurent Vivier @ 2016-04-20 5:48 UTC (permalink / raw)
To: David Gibson; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
On 20/04/2016 04:01, David Gibson wrote:
> On Tue, 19 Apr 2016 19:26:27 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> As we know where QEMU will load the kernel, it seems an useless
>> pain to try to compute it instead of using the well known value.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>
> Hmm.. I'm a little dubious about whether it's a good idea to require
> that kernel load address to remain fixed in qemu. I don't think it's
> something we really have an interface guarantee for.
>
> It might be ok given that we're just talking about the test shim here,
> though.
As it is hardcoded into boot_rom.S I see no obvious reason to not
hardcode it in cstart64.S.
Laurent
>> ---
>> powerpc/boot_rom.S | 3 ++-
>> powerpc/cstart64.S | 6 +++---
>> powerpc/spapr.h | 6 ++++++
>> 3 files changed, 11 insertions(+), 4 deletions(-)
>> create mode 100644 powerpc/spapr.h
>>
>> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
>> index ae2c08d..266d61f 100644
>> --- a/powerpc/boot_rom.S
>> +++ b/powerpc/boot_rom.S
>> @@ -1,4 +1,5 @@
>> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
>> +#include "spapr.h"
>> +
>> .text
>> .globl start
>> start:
>> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
>> index ceb6397..c47b67d 100644
>> --- a/powerpc/cstart64.S
>> +++ b/powerpc/cstart64.S
>> @@ -11,6 +11,8 @@
>> #include <asm/rtas.h>
>> #include <asm/ptrace.h>
>>
>> +#include "spapr.h"
>> +
>> .section .init
>>
>> /*
>> @@ -25,9 +27,7 @@ start:
>> * so we just linked at zero. This means the first thing to do is
>> * to find our stack and toc, and then do a relocate.
>> */
>> - bl 0f
>> -0: mflr r31
>> - subi r31, r31, 0b - start /* QEMU's kernel load address */
>> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
>> ld r1, (p_stack - start)(r31)
>> ld r2, (p_toc - start)(r31)
>> add r1, r1, r31
>> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
>> new file mode 100644
>> index 0000000..b41aece
>> --- /dev/null
>> +++ b/powerpc/spapr.h
>> @@ -0,0 +1,6 @@
>> +#ifndef _ASMPOWERPC_SPAPR_H_
>> +#define _ASMPOWERPC_SPAPR_H_
>> +
>> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
>> +
>> +#endif /* _ASMPOWERPC_SPAPR_H_ */
>> --
>> 2.5.5
>>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
2016-04-20 5:48 ` Laurent Vivier
@ 2016-04-20 5:55 ` David Gibson
-1 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-04-20 5:55 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2711 bytes --]
On Wed, 20 Apr 2016 07:48:49 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> On 20/04/2016 04:01, David Gibson wrote:
> > On Tue, 19 Apr 2016 19:26:27 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >
> >> As we know where QEMU will load the kernel, it seems an useless
> >> pain to try to compute it instead of using the well known value.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >
> > Hmm.. I'm a little dubious about whether it's a good idea to require
> > that kernel load address to remain fixed in qemu. I don't think it's
> > something we really have an interface guarantee for.
> >
> > It might be ok given that we're just talking about the test shim here,
> > though.
>
> As it is hardcoded into boot_rom.S I see no obvious reason to not
> hardcode it in cstart64.S.
True enough.
Reviewed-by: David Gibson <dgibson@redhat.com>
>
> Laurent
>
> >> ---
> >> powerpc/boot_rom.S | 3 ++-
> >> powerpc/cstart64.S | 6 +++---
> >> powerpc/spapr.h | 6 ++++++
> >> 3 files changed, 11 insertions(+), 4 deletions(-)
> >> create mode 100644 powerpc/spapr.h
> >>
> >> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
> >> index ae2c08d..266d61f 100644
> >> --- a/powerpc/boot_rom.S
> >> +++ b/powerpc/boot_rom.S
> >> @@ -1,4 +1,5 @@
> >> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> >> +#include "spapr.h"
> >> +
> >> .text
> >> .globl start
> >> start:
> >> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> >> index ceb6397..c47b67d 100644
> >> --- a/powerpc/cstart64.S
> >> +++ b/powerpc/cstart64.S
> >> @@ -11,6 +11,8 @@
> >> #include <asm/rtas.h>
> >> #include <asm/ptrace.h>
> >>
> >> +#include "spapr.h"
> >> +
> >> .section .init
> >>
> >> /*
> >> @@ -25,9 +27,7 @@ start:
> >> * so we just linked at zero. This means the first thing to do is
> >> * to find our stack and toc, and then do a relocate.
> >> */
> >> - bl 0f
> >> -0: mflr r31
> >> - subi r31, r31, 0b - start /* QEMU's kernel load address */
> >> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> >> ld r1, (p_stack - start)(r31)
> >> ld r2, (p_toc - start)(r31)
> >> add r1, r1, r31
> >> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> >> new file mode 100644
> >> index 0000000..b41aece
> >> --- /dev/null
> >> +++ b/powerpc/spapr.h
> >> @@ -0,0 +1,6 @@
> >> +#ifndef _ASMPOWERPC_SPAPR_H_
> >> +#define _ASMPOWERPC_SPAPR_H_
> >> +
> >> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> >> +
> >> +#endif /* _ASMPOWERPC_SPAPR_H_ */
> >> --
> >> 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
@ 2016-04-20 5:55 ` David Gibson
0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-04-20 5:55 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- Attachment #1: Type: text/plain, Size: 2711 bytes --]
On Wed, 20 Apr 2016 07:48:49 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> On 20/04/2016 04:01, David Gibson wrote:
> > On Tue, 19 Apr 2016 19:26:27 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >
> >> As we know where QEMU will load the kernel, it seems an useless
> >> pain to try to compute it instead of using the well known value.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >
> > Hmm.. I'm a little dubious about whether it's a good idea to require
> > that kernel load address to remain fixed in qemu. I don't think it's
> > something we really have an interface guarantee for.
> >
> > It might be ok given that we're just talking about the test shim here,
> > though.
>
> As it is hardcoded into boot_rom.S I see no obvious reason to not
> hardcode it in cstart64.S.
True enough.
Reviewed-by: David Gibson <dgibson@redhat.com>
>
> Laurent
>
> >> ---
> >> powerpc/boot_rom.S | 3 ++-
> >> powerpc/cstart64.S | 6 +++---
> >> powerpc/spapr.h | 6 ++++++
> >> 3 files changed, 11 insertions(+), 4 deletions(-)
> >> create mode 100644 powerpc/spapr.h
> >>
> >> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
> >> index ae2c08d..266d61f 100644
> >> --- a/powerpc/boot_rom.S
> >> +++ b/powerpc/boot_rom.S
> >> @@ -1,4 +1,5 @@
> >> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> >> +#include "spapr.h"
> >> +
> >> .text
> >> .globl start
> >> start:
> >> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> >> index ceb6397..c47b67d 100644
> >> --- a/powerpc/cstart64.S
> >> +++ b/powerpc/cstart64.S
> >> @@ -11,6 +11,8 @@
> >> #include <asm/rtas.h>
> >> #include <asm/ptrace.h>
> >>
> >> +#include "spapr.h"
> >> +
> >> .section .init
> >>
> >> /*
> >> @@ -25,9 +27,7 @@ start:
> >> * so we just linked at zero. This means the first thing to do is
> >> * to find our stack and toc, and then do a relocate.
> >> */
> >> - bl 0f
> >> -0: mflr r31
> >> - subi r31, r31, 0b - start /* QEMU's kernel load address */
> >> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> >> ld r1, (p_stack - start)(r31)
> >> ld r2, (p_toc - start)(r31)
> >> add r1, r1, r31
> >> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> >> new file mode 100644
> >> index 0000000..b41aece
> >> --- /dev/null
> >> +++ b/powerpc/spapr.h
> >> @@ -0,0 +1,6 @@
> >> +#ifndef _ASMPOWERPC_SPAPR_H_
> >> +#define _ASMPOWERPC_SPAPR_H_
> >> +
> >> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> >> +
> >> +#endif /* _ASMPOWERPC_SPAPR_H_ */
> >> --
> >> 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer
2016-04-19 17:26 ` Laurent Vivier
@ 2016-04-20 5:59 ` David Gibson
-1 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-04-20 5:59 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer
@ 2016-04-20 5:59 ` David Gibson
0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2016-04-20 5:59 UTC (permalink / raw)
To: Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, thuth, pbonzini
[-- 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 --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
2016-04-19 17:26 ` Laurent Vivier
@ 2016-04-20 10:15 ` Thomas Huth
-1 siblings, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2016-04-20 10:15 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 19.04.2016 19:26, Laurent Vivier wrote:
> As we know where QEMU will load the kernel, it seems an useless
> pain to try to compute it instead of using the well known value.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/boot_rom.S | 3 ++-
> powerpc/cstart64.S | 6 +++---
> powerpc/spapr.h | 6 ++++++
> 3 files changed, 11 insertions(+), 4 deletions(-)
> create mode 100644 powerpc/spapr.h
>
> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
> index ae2c08d..266d61f 100644
> --- a/powerpc/boot_rom.S
> +++ b/powerpc/boot_rom.S
> @@ -1,4 +1,5 @@
> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +#include "spapr.h"
> +
> .text
> .globl start
> start:
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index ceb6397..c47b67d 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -11,6 +11,8 @@
> #include <asm/rtas.h>
> #include <asm/ptrace.h>
>
> +#include "spapr.h"
> +
> .section .init
>
> /*
> @@ -25,9 +27,7 @@ start:
> * so we just linked at zero. This means the first thing to do is
> * to find our stack and toc, and then do a relocate.
> */
> - bl 0f
> -0: mflr r31
> - subi r31, r31, 0b - start /* QEMU's kernel load address */
> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> ld r1, (p_stack - start)(r31)
> ld r2, (p_toc - start)(r31)
> add r1, r1, r31
> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> new file mode 100644
> index 0000000..b41aece
> --- /dev/null
> +++ b/powerpc/spapr.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASMPOWERPC_SPAPR_H_
> +#define _ASMPOWERPC_SPAPR_H_
> +
> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +
> +#endif /* _ASMPOWERPC_SPAPR_H_ */
Should be fine for now (and in case we ever want to run from SLOF
instead, for example, we still can rework this again), so:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address
@ 2016-04-20 10:15 ` Thomas Huth
0 siblings, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2016-04-20 10:15 UTC (permalink / raw)
To: Laurent Vivier, kvm, kvm-ppc; +Cc: drjones, dgibson, pbonzini
On 19.04.2016 19:26, Laurent Vivier wrote:
> As we know where QEMU will load the kernel, it seems an useless
> pain to try to compute it instead of using the well known value.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> powerpc/boot_rom.S | 3 ++-
> powerpc/cstart64.S | 6 +++---
> powerpc/spapr.h | 6 ++++++
> 3 files changed, 11 insertions(+), 4 deletions(-)
> create mode 100644 powerpc/spapr.h
>
> diff --git a/powerpc/boot_rom.S b/powerpc/boot_rom.S
> index ae2c08d..266d61f 100644
> --- a/powerpc/boot_rom.S
> +++ b/powerpc/boot_rom.S
> @@ -1,4 +1,5 @@
> -#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +#include "spapr.h"
> +
> .text
> .globl start
> start:
> diff --git a/powerpc/cstart64.S b/powerpc/cstart64.S
> index ceb6397..c47b67d 100644
> --- a/powerpc/cstart64.S
> +++ b/powerpc/cstart64.S
> @@ -11,6 +11,8 @@
> #include <asm/rtas.h>
> #include <asm/ptrace.h>
>
> +#include "spapr.h"
> +
> .section .init
>
> /*
> @@ -25,9 +27,7 @@ start:
> * so we just linked at zero. This means the first thing to do is
> * to find our stack and toc, and then do a relocate.
> */
> - bl 0f
> -0: mflr r31
> - subi r31, r31, 0b - start /* QEMU's kernel load address */
> + LOAD_REG_IMMEDIATE(r31, SPAPR_KERNEL_LOAD_ADDR)
> ld r1, (p_stack - start)(r31)
> ld r2, (p_toc - start)(r31)
> add r1, r1, r31
> diff --git a/powerpc/spapr.h b/powerpc/spapr.h
> new file mode 100644
> index 0000000..b41aece
> --- /dev/null
> +++ b/powerpc/spapr.h
> @@ -0,0 +1,6 @@
> +#ifndef _ASMPOWERPC_SPAPR_H_
> +#define _ASMPOWERPC_SPAPR_H_
> +
> +#define SPAPR_KERNEL_LOAD_ADDR 0x400000
> +
> +#endif /* _ASMPOWERPC_SPAPR_H_ */
Should be fine for now (and in case we ever want to run from SLOF
instead, for example, we still can rework this again), so:
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer
2016-04-20 5:59 ` David Gibson
@ 2016-04-20 10:28 ` Thomas Huth
-1 siblings, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2016-04-20 10:28 UTC (permalink / raw)
To: David Gibson, Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]
On 20.04.2016 07:59, David Gibson wrote:
> 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.
In case there would be multiple TOCs, the previous exception handler
would not work at all anymore, since it does not set up r2 at all.
So for the current scope of kvm-unit-tests, I think this here is already
enough - we can still extend it later in case we ever need to support
multiple TOCs. So to me, this patch looks fine:
Reviewed-by: Thomas Huth <thuth@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer
@ 2016-04-20 10:28 ` Thomas Huth
0 siblings, 0 replies; 18+ messages in thread
From: Thomas Huth @ 2016-04-20 10:28 UTC (permalink / raw)
To: David Gibson, Laurent Vivier; +Cc: kvm, kvm-ppc, drjones, pbonzini
[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]
On 20.04.2016 07:59, David Gibson wrote:
> 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.
In case there would be multiple TOCs, the previous exception handler
would not work at all anymore, since it does not set up r2 at all.
So for the current scope of kvm-unit-tests, I think this here is already
enough - we can still extend it later in case we ever need to support
multiple TOCs. So to me, this patch looks fine:
Reviewed-by: Thomas Huth <thuth@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-04-20 10:28 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-19 17:26 [kvm-unit-tests PATCH 0/2] Fix TOC value in exception handler Laurent Vivier
2016-04-19 17:26 ` Laurent Vivier
2016-04-19 17:26 ` [kvm-unit-tests PATCH 1/2] powerpc: use well known kernel start address Laurent Vivier
2016-04-19 17:26 ` Laurent Vivier
2016-04-20 2:01 ` David Gibson
2016-04-20 2:01 ` David Gibson
2016-04-20 5:48 ` Laurent Vivier
2016-04-20 5:48 ` Laurent Vivier
2016-04-20 5:55 ` David Gibson
2016-04-20 5:55 ` David Gibson
2016-04-20 10:15 ` Thomas Huth
2016-04-20 10:15 ` Thomas Huth
2016-04-19 17:26 ` [kvm-unit-tests PATCH 2/2] powerpc: restore TOC pointer Laurent Vivier
2016-04-19 17:26 ` Laurent Vivier
2016-04-20 5:59 ` David Gibson
2016-04-20 5:59 ` David Gibson
2016-04-20 10:28 ` Thomas Huth
2016-04-20 10:28 ` Thomas Huth
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.