| Message ID | 20210806133843.3642916-2-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | arm64: Add optimized memset/memcpy functions | expand |
Dear Stefan, In message <20210806133843.3642916-2-sr@denx.de> you wrote: > The optimized memset() ARM64 implementation cannot be used very early > on, since caches need to be enabled for this function to work. Otherwise > an exception occurs (only printed with very early DEBUG_UART enabled). > > This patch now implements a very simple memset() version, which will be > used in a few selected places in the ARM64 early boot process. ... We have already implementations of memset() elsewhere, for example in lib/string.c [hey, that's even the file you are modifying...], then again in lib/efi/efi_stub.c and also in lib/efi_loader/efi_freestanding.c - so do we really need another implementation? BTW: lib/efi_loader/efi_freestanding.c looks completely useless to me. Can we dump this? [Adding Heinich to Cc:] > +void *memset_simple(void *s, int c, size_t count) > +{ > + char *s8 = (char *)s; > + > + while (count--) > + *s8++ = c; > + > + return s; > +} In which way is this different from memset() as defined in the same file (further up) with CONFIG_TINY_MEMSET enabled? And even without this option the memset() implementation here should work. Or does it not? Best regards, Wolfgang Denk
On 8/6/21 5:36 PM, Wolfgang Denk wrote: > Dear Stefan, > > In message <20210806133843.3642916-2-sr@denx.de> you wrote: >> The optimized memset() ARM64 implementation cannot be used very early >> on, since caches need to be enabled for this function to work. Otherwise >> an exception occurs (only printed with very early DEBUG_UART enabled). >> >> This patch now implements a very simple memset() version, which will be >> used in a few selected places in the ARM64 early boot process. > ... > > We have already implementations of memset() elsewhere, for example in > lib/string.c [hey, that's even the file you are modifying...], then > again in lib/efi/efi_stub.c and also in > lib/efi_loader/efi_freestanding.c - so do we really need another > implementation? > > BTW: lib/efi_loader/efi_freestanding.c looks completely useless to > me. Can we dump this? [Adding Heinich to Cc:] EFI binaries are freestanding. They cannot use any U-Boot library function except the UEFI API exposed via the system table. We compile EFI binaries with GCC parameter -ffreestanding. As we do not include any GCC library it is assumed that functions like memset() are provided by the compiled source package. This is where efi_freestanding.c comes in. Please, have a look at scripts/Makefile.lib:420: targets += $(obj)/efi_crt0.o $(obj)/efi_reloc.o $(obj)/efi_freestanding.o where we link the efi_freestanding.o module into every EFI binary. Best regards Heinrich > > >> +void *memset_simple(void *s, int c, size_t count) >> +{ >> + char *s8 = (char *)s; >> + >> + while (count--) >> + *s8++ = c; >> + >> + return s; >> +} > > In which way is this different from memset() as defined in the same > file (further up) with CONFIG_TINY_MEMSET enabled? And even without > this option the memset() implementation here should work. Or does > it not? > > Best regards, > > Wolfgang Denk >
Dear Heinrich, In message <c79ed1e1-3a8b-fc72-8423-a1d8ebd811cd@gmx.de> you wrote: > > EFI binaries are freestanding. They cannot use any U-Boot library > function except the UEFI API exposed via the system table. I fail to see why they cannot link standard library functions provided elsewhere in the U-Boot code. Why do you have to reimplement exactly the same code? Best regards, Wolfgang Denk
On Sat, Aug 07, 2021 at 05:18:48PM +0200, Wolfgang Denk wrote: > Dear Heinrich, > > In message <c79ed1e1-3a8b-fc72-8423-a1d8ebd811cd@gmx.de> you wrote: > > > > EFI binaries are freestanding. They cannot use any U-Boot library > > function except the UEFI API exposed via the system table. > > I fail to see why they cannot link standard library functions > provided elsewhere in the U-Boot code. Why do you have to > reimplement exactly the same code? Because it's not part of U-Boot, it's part of the freestanding EFI selftest code.
diff --git a/include/linux/string.h b/include/linux/string.h index dd255f21633a..0ab1c557da3d 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -129,6 +129,8 @@ extern void * memchr(const void *,int,__kernel_size_t); void *memchr_inv(const void *, int, size_t); #endif +void *memset_simple(void *s, int c, size_t count); + unsigned long ustrtoul(const char *cp, char **endp, unsigned int base); unsigned long long ustrtoull(const char *cp, char **endp, unsigned int base); diff --git a/lib/string.c b/lib/string.c index ba176fb08f73..7092181f831f 100644 --- a/lib/string.c +++ b/lib/string.c @@ -543,6 +543,16 @@ __used void * memset(void * s,int c,size_t count) } #endif +void *memset_simple(void *s, int c, size_t count) +{ + char *s8 = (char *)s; + + while (count--) + *s8++ = c; + + return s; +} + #ifndef __HAVE_ARCH_MEMCPY /** * memcpy - Copy one area of memory to another
The optimized memset() ARM64 implementation cannot be used very early on, since caches need to be enabled for this function to work. Otherwise an exception occurs (only printed with very early DEBUG_UART enabled). This patch now implements a very simple memset() version, which will be used in a few selected places in the ARM64 early boot process. Signed-off-by: Stefan Roese <sr@denx.de> --- include/linux/string.h | 2 ++ lib/string.c | 10 ++++++++++ 2 files changed, 12 insertions(+)