Hi Heinrich On Thu, 10 Nov 2022 at 15:46, Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote: > > On 11/10/22 14:31, Ilias Apalodimas wrote: > > UEFI specification requires pointers that are passed to protocol member > > functions to be aligned. There's a u16_strdup in that function which > > doesn't make sense otherwise Add a comment so no one removes it > > accidentally > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org> > > --- > > lib/efi_loader/efi_file.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c > > index 8480ed3007c7..5c254ccdd64d 100644 > > --- a/lib/efi_loader/efi_file.c > > +++ b/lib/efi_loader/efi_file.c > > @@ -1135,6 +1135,11 @@ struct efi_file_handle *efi_file_from_path(struct > > efi_device_path *fp) > > return NULL; > > } > > > > + /* > > + * UEFI specification requires pointers that are passed to > > + * protocol member functions to be aligned. So memcpy it > > + * unconditionally > > + */ > > filename = u16_strdup(fdp->str); > > On ARM we enable unaligned access which is supported by the CPU. On > RISC-V unaligned access is emulated by OpenSBI which is very slow. > Therefore copying make sense. > > u16_strdup() calls u16_strsize() which itself is not caring about > alignment. So this u16_strdup does not resolve all alignment issues. > > We could use the length field of the file path node to determine the > length of the string to be copied and invoke > > malloc(fdp->length - 4). > memcpy(,, fdp->length - 4). > > This would be better performance wise on RISC-V.
Sure that makes sense. But the comment is for EFI functions that have that string as an argument. Will you pick the comment and I can send that on a followup patch? Thanks /Ilias > > Best regards > > Heinrich > > > if (!filename) > > return NULL; >