public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement
@ 2021-03-25 15:56 Andrew Jones
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 1/2] arm64: argc is an int Andrew Jones
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andrew Jones @ 2021-03-25 15:56 UTC (permalink / raw)
  To: kvm; +Cc: nikos.nikoleris, alexandru.elisei

Fix the loading of argc. Nikos reported an issue with its alignment
while testing target-efi (aligned to four bytes is OK, as long as we
only load four bytes like we should). Also, take a patch developed
while working on target-efi which can make debugging a bit more
convenient (by doing some subtraction for the test developer).

Andrew Jones (2):
  arm64: argc is an int
  arm64: Output PC load offset on unhandled exceptions

 arm/cstart64.S        | 2 +-
 arm/flat.lds          | 1 +
 lib/arm64/processor.c | 7 +++++++
 3 files changed, 9 insertions(+), 1 deletion(-)

-- 
2.26.3


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH kvm-unit-tests 1/2] arm64: argc is an int
  2021-03-25 15:56 [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones
@ 2021-03-25 15:56 ` Andrew Jones
  2021-03-25 16:15   ` Nikos Nikoleris
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 2/2] arm64: Output PC load offset on unhandled exceptions Andrew Jones
  2021-03-25 16:28 ` [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2021-03-25 15:56 UTC (permalink / raw)
  To: kvm; +Cc: nikos.nikoleris, alexandru.elisei

If argc isn't aligned to eight bytes then loading it as if it's
eight bytes wide is a bad idea. It's only four bytes wide, so we
should only load four bytes.

Reported-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/cstart64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arm/cstart64.S b/arm/cstart64.S
index 89321dad7aba..0a85338bcdae 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -100,7 +100,7 @@ start:
 1:
 	/* run the test */
 	adrp	x0, __argc
-	ldr	x0, [x0, :lo12:__argc]
+	ldr	w0, [x0, :lo12:__argc]
 	adrp	x1, __argv
 	add	x1, x1, :lo12:__argv
 	adrp	x2, __environ
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH kvm-unit-tests 2/2] arm64: Output PC load offset on unhandled exceptions
  2021-03-25 15:56 [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 1/2] arm64: argc is an int Andrew Jones
@ 2021-03-25 15:56 ` Andrew Jones
  2021-03-25 16:20   ` Nikos Nikoleris
  2021-03-25 16:28 ` [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2021-03-25 15:56 UTC (permalink / raw)
  To: kvm; +Cc: nikos.nikoleris, alexandru.elisei

Since for arm64 we can load the unit tests at different addresses,
then let's make it easier to debug by calculating the PC offset for
the user. The offset can then be directly used when looking at the
disassembly of the test's elf file.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/flat.lds          | 1 +
 lib/arm64/processor.c | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/arm/flat.lds b/arm/flat.lds
index 4d43cdfeab41..6ed377c0eaa0 100644
--- a/arm/flat.lds
+++ b/arm/flat.lds
@@ -1,6 +1,7 @@
 
 SECTIONS
 {
+    PROVIDE(_text = .);
     .text : { *(.init) *(.text) *(.text.*) }
     . = ALIGN(64K);
     PROVIDE(etext = .);
diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
index ef558625e284..831207c16587 100644
--- a/lib/arm64/processor.c
+++ b/lib/arm64/processor.c
@@ -99,12 +99,19 @@ bool get_far(unsigned int esr, unsigned long *far)
 	return false;
 }
 
+extern unsigned long _text;
+
 static void bad_exception(enum vector v, struct pt_regs *regs,
 			  unsigned int esr, bool esr_valid, bool bad_vector)
 {
 	unsigned long far;
 	bool far_valid = get_far(esr, &far);
 	unsigned int ec = esr >> ESR_EL1_EC_SHIFT;
+	uintptr_t text = (uintptr_t)&_text;
+
+	printf("Load address: %" PRIxPTR "\n", text);
+	printf("PC: %" PRIxPTR " PC offset: %" PRIxPTR "\n",
+	       (uintptr_t)regs->pc, (uintptr_t)regs->pc - text);
 
 	if (bad_vector) {
 		if (v < VECTOR_MAX)
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH kvm-unit-tests 1/2] arm64: argc is an int
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 1/2] arm64: argc is an int Andrew Jones
@ 2021-03-25 16:15   ` Nikos Nikoleris
  0 siblings, 0 replies; 6+ messages in thread
From: Nikos Nikoleris @ 2021-03-25 16:15 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: alexandru.elisei

On 25/03/2021 15:56, Andrew Jones wrote:
> If argc isn't aligned to eight bytes then loading it as if it's
> eight bytes wide is a bad idea. It's only four bytes wide, so we
> should only load four bytes.
> 
> Reported-by: Nikos Nikoleris <nikos.nikoleris@arm.com>
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Thanks for fixing this.

Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

> ---
>   arm/cstart64.S | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index 89321dad7aba..0a85338bcdae 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -100,7 +100,7 @@ start:
>   1:
>   	/* run the test */
>   	adrp	x0, __argc
> -	ldr	x0, [x0, :lo12:__argc]
> +	ldr	w0, [x0, :lo12:__argc]
>   	adrp	x1, __argv
>   	add	x1, x1, :lo12:__argv
>   	adrp	x2, __environ
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH kvm-unit-tests 2/2] arm64: Output PC load offset on unhandled exceptions
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 2/2] arm64: Output PC load offset on unhandled exceptions Andrew Jones
@ 2021-03-25 16:20   ` Nikos Nikoleris
  0 siblings, 0 replies; 6+ messages in thread
From: Nikos Nikoleris @ 2021-03-25 16:20 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: alexandru.elisei

On 25/03/2021 15:56, Andrew Jones wrote:
> Since for arm64 we can load the unit tests at different addresses,
> then let's make it easier to debug by calculating the PC offset for
> the user. The offset can then be directly used when looking at the
> disassembly of the test's elf file.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com
Reviewed-by: Nikos Nikoleris <nikos.nikoleris@arm.com>

> ---
>   arm/flat.lds          | 1 +
>   lib/arm64/processor.c | 7 +++++++
>   2 files changed, 8 insertions(+)
> 
> diff --git a/arm/flat.lds b/arm/flat.lds
> index 4d43cdfeab41..6ed377c0eaa0 100644
> --- a/arm/flat.lds
> +++ b/arm/flat.lds
> @@ -1,6 +1,7 @@
>   
>   SECTIONS
>   {
> +    PROVIDE(_text = .);
>       .text : { *(.init) *(.text) *(.text.*) }
>       . = ALIGN(64K);
>       PROVIDE(etext = .);
> diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
> index ef558625e284..831207c16587 100644
> --- a/lib/arm64/processor.c
> +++ b/lib/arm64/processor.c
> @@ -99,12 +99,19 @@ bool get_far(unsigned int esr, unsigned long *far)
>   	return false;
>   }
>   
> +extern unsigned long _text;
> +
>   static void bad_exception(enum vector v, struct pt_regs *regs,
>   			  unsigned int esr, bool esr_valid, bool bad_vector)
>   {
>   	unsigned long far;
>   	bool far_valid = get_far(esr, &far);
>   	unsigned int ec = esr >> ESR_EL1_EC_SHIFT;
> +	uintptr_t text = (uintptr_t)&_text;
> +
> +	printf("Load address: %" PRIxPTR "\n", text);
> +	printf("PC: %" PRIxPTR " PC offset: %" PRIxPTR "\n",
> +	       (uintptr_t)regs->pc, (uintptr_t)regs->pc - text);
>   
>   	if (bad_vector) {
>   		if (v < VECTOR_MAX)
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement
  2021-03-25 15:56 [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 1/2] arm64: argc is an int Andrew Jones
  2021-03-25 15:56 ` [PATCH kvm-unit-tests 2/2] arm64: Output PC load offset on unhandled exceptions Andrew Jones
@ 2021-03-25 16:28 ` Andrew Jones
  2 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2021-03-25 16:28 UTC (permalink / raw)
  To: kvm; +Cc: nikos.nikoleris, alexandru.elisei

On Thu, Mar 25, 2021 at 04:56:55PM +0100, Andrew Jones wrote:
> Fix the loading of argc. Nikos reported an issue with its alignment
> while testing target-efi (aligned to four bytes is OK, as long as we
> only load four bytes like we should). Also, take a patch developed
> while working on target-efi which can make debugging a bit more
> convenient (by doing some subtraction for the test developer).
> 
> Andrew Jones (2):
>   arm64: argc is an int
>   arm64: Output PC load offset on unhandled exceptions
> 
>  arm/cstart64.S        | 2 +-
>  arm/flat.lds          | 1 +
>  lib/arm64/processor.c | 7 +++++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> -- 
> 2.26.3
>

Thanks for the review, Nikos!

Applied to arm/queue

https://gitlab.com/rhdrjones/kvm-unit-tests/-/commits/arm/queue

Thanks,
drew 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-03-25 16:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-25 15:56 [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones
2021-03-25 15:56 ` [PATCH kvm-unit-tests 1/2] arm64: argc is an int Andrew Jones
2021-03-25 16:15   ` Nikos Nikoleris
2021-03-25 15:56 ` [PATCH kvm-unit-tests 2/2] arm64: Output PC load offset on unhandled exceptions Andrew Jones
2021-03-25 16:20   ` Nikos Nikoleris
2021-03-25 16:28 ` [PATCH kvm-unit-tests 0/2] arm64: One fix and one improvement Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox