xiaoxiang781216 commented on code in PR #19673:
URL: https://github.com/apache/nuttx/pull/19673#discussion_r3742506726
##########
libs/libc/elf/elf.h:
##########
@@ -356,4 +356,24 @@ int libelf_addrenv_restore(FAR struct mod_loadinfo_s
*loadinfo);
void libelf_addrenv_free(FAR struct mod_loadinfo_s *loadinfo);
#endif /* CONFIG_ARCH_ADDRENV */
+
+#ifdef HAVE_LIBC_ELF_PIN
+/****************************************************************************
+ * Name: libelf_pinrelease
+ *
+ * Description:
+ * Give back an XIP pin taken while loading, and the file it was held
+ * through. Does nothing if no pin was taken.
+ *
+ * Input Parameters:
+ * pinfile - The held file, cleared on return.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+void libelf_pinrelease(FAR struct file **pinfile);
Review Comment:
let mmap pin the storage and ummap to unpin it.
##########
include/nuttx/lib/elf.h:
##########
@@ -252,6 +264,28 @@ struct mod_loadinfo_s
* skip the copy.
*/
+ /* FDPIC state.
+ *
+ * An FDPIC object places its two PT_LOAD segments independently: the
+ * read-only one is mapped where it already sits on the media and the
+ * writable one is copied to RAM, once per running instance. That is
+ * what lets several instances share one copy of the text.
+ *
+ * fdpic - True if e_ident[EI_OSABI] marked this an FDPIC object.
+ * textpin - True if the read-only segment is held by a filesystem pin
+ * that has to be dropped at unload, rather than by an
+ * address the filesystem simply handed over.
+ */
+
+ bool fdpic;
+ bool textpin;
+
Review Comment:
change to pic? the difference between pic and fdpic is that the normal pic
doesn't support so.
##########
include/nuttx/lib/elf.h:
##########
@@ -186,6 +186,18 @@ struct module_s
uint16_t nsect; /* Number of entries in sectalloc array
*/
#endif
int dynamic; /* Module is a dynamic shared object */
+ bool fdpic; /* Module is an FDPIC object: its two
+ * segments were placed separately and
+ * the text is media, not an allocation
+ */
+ uintptr_t gotaddr; /* An FDPIC object's data base, to
Review Comment:
could we check gotaddr |= 0 and remove fdpic
##########
include/nuttx/lib/elf.h:
##########
@@ -252,6 +264,28 @@ struct mod_loadinfo_s
* skip the copy.
*/
+ /* FDPIC state.
+ *
+ * An FDPIC object places its two PT_LOAD segments independently: the
+ * read-only one is mapped where it already sits on the media and the
+ * writable one is copied to RAM, once per running instance. That is
+ * what lets several instances share one copy of the text.
+ *
+ * fdpic - True if e_ident[EI_OSABI] marked this an FDPIC object.
+ * textpin - True if the read-only segment is held by a filesystem pin
+ * that has to be dropped at unload, rather than by an
+ * address the filesystem simply handed over.
+ */
+
+ 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:
why not let mmap to hold the pin
##########
libs/libc/elf/elf_load.c:
##########
@@ -350,6 +367,16 @@ static inline int libelf_loadfile(FAR struct
mod_loadinfo_s *loadinfo)
{
if (phdr->p_flags & PF_X)
{
+ if (loadinfo->fdpic)
Review Comment:
should we fallback to copy if the fs or mtd doesn't support xip.
##########
include/nuttx/lib/elf.h:
##########
@@ -44,6 +44,17 @@
# define CONFIG_LIBC_ELF_MAXDEPEND 0
#endif
+/* Holding an XIP pin past the load means holding the file itself: the pin is
+ * released when the module is unloaded, which happens on a task other than
+ * the one that loaded it, so a descriptor from that task's group cannot
+ * serve. That needs the file interface, which is why CONFIG_FDPIC depends
+ * on the flat build.
+ */
+
+#ifdef CONFIG_FDPIC
Review Comment:
check CONFIG_PIC
##########
libs/libc/elf/elf_load.c:
##########
@@ -525,6 +552,123 @@ 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 that
+ * the read-only part of the object can be used where it lies instead of
+ * being copied.
+ *
+ * Two mechanisms exist and they are not interchangeable. A compacting
+ * filesystem can move a file's blocks, so it hands out an address only
+ * together with a pin that holds them still, and expects the pin back;
+ * xipfs is the one in tree. A filesystem whose layout never changes has
+ * nothing to hold and answers FIOC_XIPBASE with a bare address; romfs
Review Comment:
but we can unify FIOC_XIPBASE and pin into mmap/unmap.
##########
tools/fdpic/nuttx-fdpic.mk:
##########
@@ -0,0 +1,221 @@
+############################################################################
+# tools/fdpic/nuttx-fdpic.mk
+#
+# 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.
+#
+############################################################################
+
+############################################################################
+# tools/fdpic/nuttx-fdpic.mk
+#
+# 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.
+#
+############################################################################
+
+# nuttx-fdpic.mk -- build out-of-tree FDPIC modules for NuttX
Review Comment:
why not modify the original elf generation scirpt in place
##########
binfmt/Kconfig:
##########
@@ -60,6 +60,35 @@ config ELF_STACKSIZE
default DEFAULT_TASK_STACKSIZE
---help---
This is the default stack size that will be used when starting
ELF binaries.
+
+config ELF_FDPIC
+ bool "FDPIC modules"
+ default n
+ select PIC
+ depends on ARCH_ARMV7M || ARCH_ARMV8M
Review Comment:
remove the dependence
##########
binfmt/Kconfig:
##########
@@ -61,11 +61,12 @@ config ELF_STACKSIZE
---help---
This is the default stack size that will be used when starting
ELF binaries.
-config ELF_FDPIC
+config FDPIC
Review Comment:
move to the firdt patch which add ELF_FDPIC
##########
include/nuttx/fdpic.h:
##########
@@ -0,0 +1,178 @@
+/****************************************************************************
+ * 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 base firmware is not built FDPIC, so to it a function pointer is a
+ * code address and it simply branches there. An FDPIC module passes the
+ * address of one of these instead, because its code and data are placed
+ * independently and a bare code address would leave the callee unable to
+ * find its own data.
+ */
+
+struct fdpic_desc_s
+{
+ uintptr_t entry; /* Address of the code */
+ uintptr_t got; /* Data base to install before branching */
+};
+
+/****************************************************************************
+ * Inline Functions
+ ****************************************************************************/
+
+#ifdef CONFIG_ELF_FDPIC
+
+/****************************************************************************
+ * Name: fdpic_base
+ *
+ * Description:
+ * The data base of the calling context, read from the PIC base register.
+ * Non-zero means the caller is an FDPIC module; zero means base firmware,
+ * because up_initial_state() only installs a value for a task that has a
+ * D-Space.
+ *
+ * This is what lets a shared entry point tell the two apart without being
+ * told, so that a plain kernel task calling qsort() is unaffected.
+ *
+ ****************************************************************************/
+
+static inline uintptr_t fdpic_base(void)
+{
+ uintptr_t base;
+
+ up_getpicbase(&base);
+ return base;
+}
+
+/****************************************************************************
+ * Name: fdpic_callback
+ *
+ * Description:
+ * Resolve a function pointer that arrived from a caller which may be an
+ * FDPIC module.
+ *
+ * Only the entry point is taken from the descriptor. The data base is
+ * already correct in the register: the base firmware is built with that
+ * register reserved, so the module's own base survives the call in, and
+ * any task the module creates inherits its D-Space.
+ *
+ * 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's data base in the
+ * PIC base register, and restore the caller's afterwards.
+ *
+ * This is for the one case where the register cannot already be right: a
+ * callback a module registered that runs on a shared thread -- the
+ * signal-notification work queue -- which carries no module's base. The
+ * base is captured at registration time, in the module's own context, and
+ * installed here around the call. Everywhere else the callback runs on a
+ * task that inherited the module's D-Space and fdpic_callback() suffices.
+ *
+ * A context switch or interrupt during the call is safe: the register is
+ * REG_PIC in the saved context, so it is preserved across a switch, and
+ * the base firmware is built with it reserved so no handler disturbs it.
+ *
+ * Input Parameters:
+ * entry - The code address to enter, already resolved from the descriptor.
+ * arg - The single word argument, passed in r0.
+ * got - The module data base to install.
+ *
+ ****************************************************************************/
+
+static inline void fdpic_invoke(uintptr_t entry, uintptr_t arg,
+ uintptr_t got)
+{
+ register uintptr_t r0v __asm__ ("r0") = arg;
Review Comment:
not work for other arch, should keep jn arch.h or arch/elf.h
--
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]
