xiaoxiang781216 commented on code in PR #19942: URL: https://github.com/apache/nuttx/pull/19942#discussion_r3896916313
########## include/nuttx/fdpic.h: ########## @@ -0,0 +1,129 @@ +/**************************************************************************** + * include/nuttx/fdpic.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FDPIC_H +#define __INCLUDE_NUTTX_FDPIC_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdint.h> + +#include <nuttx/arch.h> +#include <nuttx/compiler.h> + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +/* A function descriptor: what a function pointer is under FDPIC. The + * firmware branches to a code address; a module passes one of these. + */ + +struct fdpic_desc_s +{ + uintptr_t entry; /* Address of the code */ + uintptr_t got; /* Data base to install before branching */ +}; + +/**************************************************************************** + * Inline Functions + ****************************************************************************/ + +#ifdef CONFIG_FDPIC + +/**************************************************************************** + * Name: fdpic_base + * + * Description: + * The data base of the calling context, from the PIC base register. + * Non-zero means the caller is an FDPIC module, zero means firmware. + * + ****************************************************************************/ + +static inline uintptr_t fdpic_base(void) +{ + uintptr_t base; + + up_getpicbase(&base); + return base; +} + +/**************************************************************************** + * Name: fdpic_callback + * + * Description: + * Resolve a function pointer from a caller that may be an FDPIC module. + * Only the entry point is taken: the data base is already in the register. + * + * Input Parameters: + * fn - The pointer as it was received. + * + * Returned Value: + * An address that can be branched to directly. + * + ****************************************************************************/ + +static inline FAR void *fdpic_callback(FAR void *fn) +{ + if (fn != NULL && fdpic_base() != 0) + { + return (FAR void *)((FAR struct fdpic_desc_s *)fn)->entry; + } + + return fn; +} + +/**************************************************************************** + * Name: fdpic_invoke + * + * Description: + * Call a resolved module entry point with the module data base in the PIC + * base register. For a callback that runs on a shared thread, which + * carries no module base. Elsewhere fdpic_callback() is enough. + * + * Input Parameters: + * entry - The code address to enter, already resolved from the descriptor. + * arg - The one word argument. + * got - The module data base to install. + * + ****************************************************************************/ + +static inline void fdpic_invoke(uintptr_t entry, uintptr_t arg, + uintptr_t got) +{ + up_fdpic_invoke(entry, arg, got); Review Comment: ditto ########## include/nuttx/fdpic.h: ########## @@ -0,0 +1,129 @@ +/**************************************************************************** + * include/nuttx/fdpic.h + * + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. The + * ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + * + ****************************************************************************/ + +#ifndef __INCLUDE_NUTTX_FDPIC_H +#define __INCLUDE_NUTTX_FDPIC_H + +/**************************************************************************** + * Included Files + ****************************************************************************/ + +#include <nuttx/config.h> + +#include <stdint.h> + +#include <nuttx/arch.h> +#include <nuttx/compiler.h> + +/**************************************************************************** + * Public Types + ****************************************************************************/ + +/* A function descriptor: what a function pointer is under FDPIC. The + * firmware branches to a code address; a module passes one of these. + */ + +struct fdpic_desc_s +{ + uintptr_t entry; /* Address of the code */ + uintptr_t got; /* Data base to install before branching */ +}; + +/**************************************************************************** + * Inline Functions + ****************************************************************************/ + +#ifdef CONFIG_FDPIC + +/**************************************************************************** + * Name: fdpic_base + * + * Description: + * The data base of the calling context, from the PIC base register. + * Non-zero means the caller is an FDPIC module, zero means firmware. + * + ****************************************************************************/ + +static inline uintptr_t fdpic_base(void) +{ + uintptr_t base; + + up_getpicbase(&base); + return base; +} + +/**************************************************************************** + * Name: fdpic_callback + * + * Description: + * Resolve a function pointer from a caller that may be an FDPIC module. + * Only the entry point is taken: the data base is already in the register. + * + * Input Parameters: + * fn - The pointer as it was received. + * + * Returned Value: + * An address that can be branched to directly. + * + ****************************************************************************/ + +static inline FAR void *fdpic_callback(FAR void *fn) +{ + if (fn != NULL && fdpic_base() != 0) + { + return (FAR void *)((FAR struct fdpic_desc_s *)fn)->entry; + } + + return fn; +} + +/**************************************************************************** + * Name: fdpic_invoke + * + * Description: + * Call a resolved module entry point with the module data base in the PIC + * base register. For a callback that runs on a shared thread, which + * carries no module base. Elsewhere fdpic_callback() is enough. + * + * Input Parameters: + * entry - The code address to enter, already resolved from the descriptor. + * arg - The one word argument. + * got - The module data base to install. + * + ****************************************************************************/ + +static inline void fdpic_invoke(uintptr_t entry, uintptr_t arg, Review Comment: should we order the parameters by arg, entry, got ########## arch/arm/include/arch.h: ########## @@ -79,6 +79,45 @@ do { \ ); \ } while (0) +#ifdef CONFIG_FDPIC + +/**************************************************************************** + * Name: up_fdpic_invoke + * + * Description: + * Call a module entry point with the module data base in the PIC base + * register. Put the caller data base back after the call. + * + * Input Parameters: + * entry - The code address to enter. + * arg - The one word argument, passed in r0. + * got - The module data base to install. + * + ****************************************************************************/ + +static inline void up_fdpic_invoke(uintptr_t entry, uintptr_t arg, Review Comment: change the order to arg, entry, got? ########## include/nuttx/lib/elf.h: ########## @@ -44,6 +44,15 @@ # define CONFIG_LIBC_ELF_MAXDEPEND 0 #endif +/* A compacting filesystem gives its media address with a pin that holds the + * blocks in place. The loader holds the pin through a file reference, + * because the unload runs on another task. Flat build only. + */ + +#if defined(CONFIG_FS_XIPFS) && defined(CONFIG_BUILD_FLAT) Review Comment: protected build should work too ########## include/nuttx/lib/elf.h: ########## @@ -44,6 +44,15 @@ # define CONFIG_LIBC_ELF_MAXDEPEND 0 #endif +/* A compacting filesystem gives its media address with a pin that holds the + * blocks in place. The loader holds the pin through a file reference, + * because the unload runs on another task. Flat build only. + */ + +#if defined(CONFIG_FS_XIPFS) && defined(CONFIG_BUILD_FLAT) +# define HAVE_LIBC_ELF_PIN 1 Review Comment: let's add Kconfig something like FS_PIN, and let CONFIG_FS_XIPFS select it ########## include/nuttx/lib/elf.h: ########## @@ -278,6 +278,20 @@ struct mod_loadinfo_s FAR struct file *pinfile; #endif + /* The object's data base, from DT_PLTGOT. An FDPIC module runs with this + * in the PIC base register. + */ + + uintptr_t gotaddr; Review Comment: can we merge gotaddr and gotindex into one ########## include/nuttx/lib/elf.h: ########## @@ -278,6 +278,20 @@ struct mod_loadinfo_s FAR struct file *pinfile; #endif + /* The object's data base, from DT_PLTGOT. An FDPIC module runs with this + * in the PIC base register. + */ + + uintptr_t gotaddr; + + /* Pool of function descriptors behind the writable segment. Reserved + * when the segment is sized, and bounded by the relocation count. + */ + + uintptr_t descpool; Review Comment: change to the real type ########## libs/libc/elf/elf_load.c: ########## @@ -539,6 +547,106 @@ static inline int libelf_loadfile(FAR struct mod_loadinfo_s *loadinfo) return OK; } +/**************************************************************************** + * Name: libelf_xipacquire + * + * Description: + * Ask the filesystem for the address of this file on its media, so the + * read-only part of the object can run where it lies. Ask for a pin + * first: a compacting filesystem is not safe without one. Do not ask at + * all if this build cannot hold a pin. + * + * Returned Value: + * Zero if an address was obtained, a negated errno otherwise. Callers + * that can live without one may ignore the failure. + * + ****************************************************************************/ + +#ifdef HAVE_LIBC_ELF_PIN +static int libelf_pinhold(FAR struct mod_loadinfo_s *loadinfo) +{ + FAR struct file *filep; + int ret; + + /* The descriptor belongs to the task that called the loader, and the + * unload runs on another task. Hold the file instead. + */ + + loadinfo->pinfile = lib_zalloc(sizeof(struct file)); + if (loadinfo->pinfile == NULL) + { + return -ENOMEM; + } + + ret = file_get(loadinfo->filfd, &filep); + if (ret >= 0) + { + ret = file_dup2(filep, loadinfo->pinfile); + file_put(filep); + } + + if (ret < 0) + { + lib_free(loadinfo->pinfile); + loadinfo->pinfile = NULL; + } + + return ret; +} + +/**************************************************************************** + * Name: libelf_pinrelease + * + * Description: + * Give back an XIP pin and the file it was held through, so the + * filesystem can reclaim the extent. + * + ****************************************************************************/ + +void libelf_pinrelease(FAR struct file **pinfile) Review Comment: move to public section ########## include/nuttx/lib/elf.h: ########## @@ -252,6 +262,22 @@ struct mod_loadinfo_s * skip the copy. */ + /* FDPIC state. + * + * fdpic - True if e_ident[EI_OSABI] marked this an FDPIC object. + * textpin - True if a filesystem pin holds the read-only segment, which + * the loader gives back at unload. + */ + + bool fdpic; + bool textpin; + +#ifdef HAVE_LIBC_ELF_PIN + /* The file the pin is held through, handed to the module once it loads. */ + + FAR struct file *pinfile; Review Comment: remove textpin, let's check pinfile != NULL instead ########## libs/libc/elf/elf_loadhdrs.c: ########## @@ -66,6 +66,21 @@ int libelf_loadhdrs(FAR struct mod_loadinfo_s *loadinfo) /* Verify that there are sections */ + /* An FDPIC object announces itself in the OS/ABI byte. Note it once. */ + + loadinfo->fdpic = (loadinfo->ehdr.e_ident[EI_OSABI] == ELFOSABI_ARM_FDPIC); Review Comment: could we avoid check arm specifc flag in the common code ########## libs/libc/elf/elf_load.c: ########## @@ -364,6 +365,13 @@ static inline int libelf_loadfile(FAR struct mod_loadinfo_s *loadinfo) { if (phdr->p_flags & PF_X) { + if (loadinfo->fdpic && loadinfo->xipbase != 0) Review Comment: check xipbase is enough -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
