Re: [kvm-unit-tests PATCH 04/13] treewide: lib/stack: Make base_address arch specific

["Nicholas Piggin" <npiggin@gmail.com>]

On Thu Feb 29, 2024 at 1:04 AM AEST, Andrew Jones wrote:
> Calculating the offset of an address is image specific, which is
> architecture specific. Until now, all architectures and architecture
> configurations which select CONFIG_RELOC were able to subtract
> _etext, but the EFI configuration of riscv cannot (it must subtract
> ImageBase). Make this function architecture specific, since the
> architecture's image layout already is.

arch_base_address()?

How about a default implementation unlesss HAVE_ARCH_BASE_ADDRESS?

Thanks,
Nick

>
> Signed-off-by: Andrew Jones <andrew.jones@linux.dev>
> ---
>  lib/arm64/stack.c | 17 +++++++++++++++++
>  lib/riscv/stack.c | 18 ++++++++++++++++++
>  lib/stack.c       | 19 ++-----------------
>  lib/stack.h       |  2 ++
>  lib/x86/stack.c   | 17 +++++++++++++++++
>  5 files changed, 56 insertions(+), 17 deletions(-)
>
> diff --git a/lib/arm64/stack.c b/lib/arm64/stack.c
> index f5eb57fd8892..3369031a74f7 100644
> --- a/lib/arm64/stack.c
> +++ b/lib/arm64/stack.c
> @@ -6,6 +6,23 @@
>  #include <stdbool.h>
>  #include <stack.h>
>  
> +#ifdef CONFIG_RELOC
> +extern char _text, _etext;
> +
> +bool base_address(const void *rebased_addr, unsigned long *addr)
> +{
> +	unsigned long ra = (unsigned long)rebased_addr;
> +	unsigned long start = (unsigned long)&_text;
> +	unsigned long end = (unsigned long)&_etext;
> +
> +	if (ra < start || ra >= end)
> +		return false;
> +
> +	*addr = ra - start;
> +	return true;
> +}
> +#endif
> +
>  extern char vector_stub_start, vector_stub_end;
>  
>  int arch_backtrace_frame(const void *frame, const void **return_addrs,
> diff --git a/lib/riscv/stack.c b/lib/riscv/stack.c
> index d865594b9671..a143c22a570a 100644
> --- a/lib/riscv/stack.c
> +++ b/lib/riscv/stack.c
> @@ -2,6 +2,24 @@
>  #include <libcflat.h>
>  #include <stack.h>
>  
> +#ifdef CONFIG_RELOC
> +extern char ImageBase, _text, _etext;
> +
> +bool base_address(const void *rebased_addr, unsigned long *addr)
> +{
> +	unsigned long ra = (unsigned long)rebased_addr;
> +	unsigned long base = (unsigned long)&ImageBase;
> +	unsigned long start = (unsigned long)&_text;
> +	unsigned long end = (unsigned long)&_etext;
> +
> +	if (ra < start || ra >= end)
> +		return false;
> +
> +	*addr = ra - base;
> +	return true;
> +}
> +#endif
> +
>  int arch_backtrace_frame(const void *frame, const void **return_addrs,
>  			 int max_depth, bool current_frame)
>  {
> diff --git a/lib/stack.c b/lib/stack.c
> index dd6bfa8dac6e..e5099e207388 100644
> --- a/lib/stack.c
> +++ b/lib/stack.c
> @@ -11,23 +11,8 @@
>  
>  #define MAX_DEPTH 20
>  
> -#ifdef CONFIG_RELOC
> -extern char _text, _etext;
> -
> -static bool base_address(const void *rebased_addr, unsigned long *addr)
> -{
> -	unsigned long ra = (unsigned long)rebased_addr;
> -	unsigned long start = (unsigned long)&_text;
> -	unsigned long end = (unsigned long)&_etext;
> -
> -	if (ra < start || ra >= end)
> -		return false;
> -
> -	*addr = ra - start;
> -	return true;
> -}
> -#else
> -static bool base_address(const void *rebased_addr, unsigned long *addr)
> +#ifndef CONFIG_RELOC
> +bool base_address(const void *rebased_addr, unsigned long *addr)
>  {
>  	*addr = (unsigned long)rebased_addr;
>  	return true;
> diff --git a/lib/stack.h b/lib/stack.h
> index 6edc84344b51..f8def4ad4d49 100644
> --- a/lib/stack.h
> +++ b/lib/stack.h
> @@ -10,6 +10,8 @@
>  #include <libcflat.h>
>  #include <asm/stack.h>
>  
> +bool base_address(const void *rebased_addr, unsigned long *addr);
> +
>  #ifdef HAVE_ARCH_BACKTRACE_FRAME
>  extern int arch_backtrace_frame(const void *frame, const void **return_addrs,
>  				int max_depth, bool current_frame);
> diff --git a/lib/x86/stack.c b/lib/x86/stack.c
> index 58ab6c4b293a..7ba73becbd69 100644
> --- a/lib/x86/stack.c
> +++ b/lib/x86/stack.c
> @@ -1,6 +1,23 @@
>  #include <libcflat.h>
>  #include <stack.h>
>  
> +#ifdef CONFIG_RELOC
> +extern char _text, _etext;
> +
> +bool base_address(const void *rebased_addr, unsigned long *addr)
> +{
> +	unsigned long ra = (unsigned long)rebased_addr;
> +	unsigned long start = (unsigned long)&_text;
> +	unsigned long end = (unsigned long)&_etext;
> +
> +	if (ra < start || ra >= end)
> +		return false;
> +
> +	*addr = ra - start;
> +	return true;
> +}
> +#endif
> +
>  int arch_backtrace_frame(const void *frame, const void **return_addrs,
>  			 int max_depth, bool current_frame)
>  {