[PATCH] firmware: Move memcpy/memset mapping to fw_base.S

[Jessica Clarke]

On 22 Dec 2021, at 12:44, Anup Patel <anup.patel at wdc.com> wrote:
> 
> Some of the external firmwares using OpenSBI as library are facing
> issues with the weak memcpy() and memset() aliases in libsbi.a so
> we move these to fw_base.S. This way mapping of implicit memcpy()
> or memset() calls to sbi_memcpy() or sbi_memset() will only be done
> for OpenSBI firmwares.
> (Refer, https://github.com/riscv-software-src/opensbi/issues/234)

Consumers that want to override the definitions in libsbi should just
make sure -lsbi/libsbi.a appears after the object or library containing
the strong definition.

If you really want to support people that can’t get their link order
right (which then hurts people who *do* and want to just use libsbi’s
memcpy/memset) then these should be strong aliases and/or defined in C;
there’s zero reason for it to be in assembly and, as a downstream that
would need to modify this code if written in assembly, hurts
portability. And if for some reason you still insist on assembly then
you should be using tail here, not j, as j is only supported for jumps
within a file, not across files.

Jess

> Signed-off-by: Anup Patel <anup.patel at wdc.com>
> ---
> firmware/fw_base.S   | 14 ++++++++++++++
> lib/sbi/sbi_string.c |  6 ------
> 2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index 1569e60..6197a7f 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -561,6 +561,20 @@ fw_platform_init:
> 	add	a0, a1, zero
> 	ret
> 
> +	/* Map implicit memcpy() added by compiler to sbi_memcpy() */
> +	.section .text
> +	.align 3
> +	.globl memcpy
> +memcpy:
> +	j	sbi_memcpy
> +
> +	/* Map implicit memset() added by compiler to sbi_memset() */
> +	.section .text
> +	.align 3
> +	.globl memset
> +memset:
> +	j	sbi_memset
> +
> .macro	TRAP_SAVE_AND_SETUP_SP_T0
> 	/* Swap TP and MSCRATCH */
> 	csrrw	tp, CSR_MSCRATCH, tp
> diff --git a/lib/sbi/sbi_string.c b/lib/sbi/sbi_string.c
> index c67c02e..c87bce9 100644
> --- a/lib/sbi/sbi_string.c
> +++ b/lib/sbi/sbi_string.c
> @@ -122,9 +122,6 @@ void *sbi_memset(void *s, int c, size_t count)
> 	return s;
> }
> 
> -void *memset(void *s, int c, size_t count) \
> -__attribute__((weak, alias("sbi_memset")));
> -
> void *sbi_memcpy(void *dest, const void *src, size_t count)
> {
> 	char *temp1	  = dest;
> @@ -138,9 +135,6 @@ void *sbi_memcpy(void *dest, const void *src, size_t count)
> 	return dest;
> }
> 
> -void *memcpy(void *dest, const void *src, size_t count) \
> -__attribute__((weak, alias("sbi_memcpy")));
> -
> void *sbi_memmove(void *dest, const void *src, size_t count)
> {
> 	char *temp1	  = (char *)dest;
> -- 
> 2.25.1
> 
> 
> -- 
> opensbi mailing list
> opensbi at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi