You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "pussuw (via GitHub)" <gi...@apache.org> on 2023/01/30 11:29:06 UTC
[GitHub] [nuttx] pussuw opened a new pull request, #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
pussuw opened a new pull request, #8355:
URL: https://github.com/apache/nuttx/pull/8355
## Summary
This moves address environments from the group structure into the TCB. The reason for doing this is that the group is destroyed too soon when a process exits and this leaves the system without valid MMU mappings until the next context switch.
Additionally, the way up_addrenv_select(&newenv, &oldenv); + up_addrenv_restore(&oldenv); are used to temporarily select a different address environment for a process is prone to a system crash.
How?
If a context switch happens between those calls, the context restore routine (call to group_addrenv(NULL);) will restore the wrong mappings (tcb->group.tg_addrenv is restored, when newenv was in use), as the logic has not been notified of this temporary change of address environments. Using sched_lock() here is not enough, as the process can block itself and then the disaster can happen again.
## Impact
Affects address environment handling for every platform (ARM/RISC-V) that supports them
## Testing
icicle:knsh and sabre-6quad (qemu)
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1098287198
##########
sched/addrenv/addrenv.c:
##########
@@ -52,6 +53,39 @@
static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_destroy
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+ *
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void addrenv_destroy(FAR void *arg)
+{
+ FAR struct addrenv_s *addrenv = (FAR struct addrenv_s *)arg;
+
+ /* Destroy the address environment */
+
+ up_addrenv_destroy(&addrenv->pgdir);
Review Comment:
change to pgdir to addrenv
##########
sched/addrenv/addrenv.c:
##########
@@ -79,13 +79,42 @@ static void addrenv_destroy(FAR void *arg)
/* Destroy the address environment */
- up_addrenv_destroy(&addrenv->pgdir);
+ up_addrenv_destroy(&addrenv->addrenv);
Review Comment:
move to the second patch
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097133662
##########
binfmt/binfmt_execmodule.c:
##########
@@ -164,23 +164,23 @@ int exec_module(FAR const struct binary_s *binp,
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
- ret = up_addrenv_select(&binp->addrenv, &oldenv);
+ ret = addrenv_select((struct addrenv_s *)&binp->addrenv);
Review Comment:
should we change binp prototype to reflect the change?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#issuecomment-1410142995
Also, the CI output is full of this:
drivers/mmcsd/Kconfig:32:warning: 'MMCSD_IOCSUPPORT': number is invalid
drivers/mmcsd/Kconfig:32:warning: 'MMCSD_IOCSUPPORT': number is invalid
drivers/mmcsd/Kconfig:32:warning: 'MMCSD_IOCSUPPORT': number is invalid
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095451693
##########
include/nuttx/addrenv.h:
##########
@@ -359,6 +367,117 @@ int addrenv_switch(FAR struct tcb_s *tcb);
int addrenv_attach(FAR struct tcb_s *tcb,
FAR const struct arch_addrenv_s *addrenv);
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process.
+ * tcb - The tcb of the child process.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_take
+ *
+ * Description:
+ * Take a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+void addrenv_take(FAR struct addrenv_s *addrenv);
+
+/****************************************************************************
+ * Name: addrenv_take
+ *
+ * Description:
+ * Take a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+void addrenv_take(FAR struct addrenv_s *addrenv);
Review Comment:
Done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095087265
##########
include/nuttx/sched.h:
##########
@@ -532,6 +526,12 @@ struct tcb_s
FAR struct task_group_s *group; /* Pointer to shared task group data */
+ /* Address Environment ****************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+ FAR struct addrenv_s *mm_own; /* Task (group) own memory mappings */
Review Comment:
Should I change mm_curr to addrenv_curr too ?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1094979080
##########
arch/arm/src/armv7-a/arm_addrenv.c:
##########
@@ -98,8 +98,6 @@
#include <assert.h>
#include <debug.h>
-#include <nuttx/addrenv.h>
Review Comment:
move to the first patch
##########
include/nuttx/sched.h:
##########
@@ -532,6 +526,12 @@ struct tcb_s
FAR struct task_group_s *group; /* Pointer to shared task group data */
+ /* Address Environment ****************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+ FAR struct addrenv_s *mm_own; /* Task (group) own memory mappings */
Review Comment:
mm_own->addrenv
##########
include/nuttx/addrenv.h:
##########
@@ -359,6 +367,117 @@ int addrenv_switch(FAR struct tcb_s *tcb);
int addrenv_attach(FAR struct tcb_s *tcb,
FAR const struct arch_addrenv_s *addrenv);
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process.
+ * tcb - The tcb of the child process.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_take
+ *
+ * Description:
+ * Take a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+void addrenv_take(FAR struct addrenv_s *addrenv);
+
+/****************************************************************************
+ * Name: addrenv_take
+ *
+ * Description:
+ * Take a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+void addrenv_take(FAR struct addrenv_s *addrenv);
Review Comment:
remove dup with line 423
##########
sched/addrenv/addrenv.c:
##########
@@ -52,6 +53,39 @@
static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_dsr
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+ *
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void addrenv_dsr(FAR void *arg)
Review Comment:
addrenv_dsr->addrenv_destroy
##########
sched/sched/sched_releasetcb.c:
##########
@@ -154,7 +154,11 @@ int nxsched_release_tcb(FAR struct tcb_s *tcb, uint8_t ttype)
#ifdef CONFIG_ARCH_ADDRENV
/* Release this thread's reference to the address environment */
- ret = up_addrenv_detach(tcb->group, tcb);
+ ret = up_addrenv_detach(tcb);
+ if (ttype == TCB_FLAG_TTYPE_TASK)
Review Comment:
how to handle the main thread exit before other threads in the same task
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#issuecomment-1410142518
Build error seems to be something with the CI env:
unzip: cannot find zipfile directory in one of v8.3.3.zip or
v8.3.3.zip.zip, and cannot find v8.3.3.zip.ZIP, period.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097128890
##########
include/nuttx/addrenv.h:
##########
@@ -218,13 +225,27 @@
(CONFIG_ARCH_PGPOOL_VBASE + CONFIG_ARCH_PGPOOL_SIZE)
#endif
+/****************************************************************************
+ * Public Type Definitions
+ ****************************************************************************/
+
+struct tcb_s; /* Forward reference to TCB */
/****************************************************************************
* Public Types
****************************************************************************/
#ifndef __ASSEMBLY__
+struct addrenv_s
+{
+ struct arch_addrenv_s pgdir; /* The address environment page directory */
Review Comment:
In the 3rd patch
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097066421
##########
arch/arm/src/armv7-a/arm_addrenv.c:
##########
@@ -778,22 +671,18 @@ int up_addrenv_clone(const arch_addrenv_t *src,
* is created that needs to share the address environment of its task
* group.
*
- * NOTE: In some platforms, nothing will need to be done in this case.
- * Simply being a member of the group that has the address environment
- * may be sufficient.
- *
* Input Parameters:
- * group - The task group to which the new thread belongs.
- * tcb - The TCB of the thread needing the address environment.
+ * ptcb - The tcb of the parent task.
+ * tcb - The tcb of the thread needing the address environment.
*
* Returned Value:
* Zero (OK) on success; a negated errno value on failure.
*
****************************************************************************/
-int up_addrenv_attach(struct task_group_s *group, struct tcb_s *tcb)
+int up_addrenv_attach(struct tcb_s *ptcb, struct tcb_s *tcb)
Review Comment:
I'm not sure what this function is supposed to do. It is implemented as blank by all 3 platforms where it exists.
However, to keep the meaning of the original function ptcb is needed, "attach" means to join the parent's address environment.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 merged pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8355:
URL: https://github.com/apache/nuttx/pull/8355
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1096177338
##########
include/nuttx/addrenv.h:
##########
@@ -218,13 +220,24 @@
(CONFIG_ARCH_PGPOOL_VBASE + CONFIG_ARCH_PGPOOL_SIZE)
#endif
+/****************************************************************************
+ * Public Type Definitions
+ ****************************************************************************/
+
+struct tcb_s; /* Forward reference to TCB */
/****************************************************************************
* Public Types
****************************************************************************/
#ifndef __ASSEMBLY__
+struct addrenv_s
+{
+ struct arch_addrenv_s pgdir; /* The address environment page directory */
+};
+
+typedef struct addrenv_s addrenv_t;
Review Comment:
add blank line after
##########
include/nuttx/addrenv.h:
##########
@@ -269,6 +282,83 @@ struct addrenv_reserve_s
* Public Function Prototypes
****************************************************************************/
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype);
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Switch to an address environment
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task to switch to, or NULL to use the task at the
+ * head of the ready-to-run list.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly process. Called by exec() right
+ * before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct arch_addrenv_s *addrenv);
Review Comment:
arch_addrenv_s->addrenv_s?
##########
binfmt/binfmt_execmodule.c:
##########
@@ -164,23 +164,23 @@ int exec_module(FAR const struct binary_s *binp,
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
- ret = up_addrenv_select(&binp->addrenv, &oldenv);
+ ret = addrenv_select((struct addrenv_s *)&binp->addrenv);
Review Comment:
why need cast(`(struct addrenv_s *)`) here?
##########
arch/arm/src/armv7-a/arm_addrenv.c:
##########
@@ -778,22 +671,18 @@ int up_addrenv_clone(const arch_addrenv_t *src,
* is created that needs to share the address environment of its task
* group.
*
- * NOTE: In some platforms, nothing will need to be done in this case.
- * Simply being a member of the group that has the address environment
- * may be sufficient.
- *
* Input Parameters:
- * group - The task group to which the new thread belongs.
- * tcb - The TCB of the thread needing the address environment.
+ * ptcb - The tcb of the parent task.
+ * tcb - The tcb of the thread needing the address environment.
*
* Returned Value:
* Zero (OK) on success; a negated errno value on failure.
*
****************************************************************************/
-int up_addrenv_attach(struct task_group_s *group, struct tcb_s *tcb)
+int up_addrenv_attach(struct tcb_s *ptcb, struct tcb_s *tcb)
Review Comment:
should we remove ptcb?
##########
binfmt/binfmt_execmodule.c:
##########
@@ -164,23 +164,23 @@ int exec_module(FAR const struct binary_s *binp,
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
- ret = up_addrenv_select(&binp->addrenv, &oldenv);
+ ret = addrenv_select((struct addrenv_s *)&binp->addrenv);
if (ret < 0)
{
- berr("ERROR: up_addrenv_select() failed: %d\n", ret);
+ berr("ERROR: addrenv_select() failed: %d\n", ret);
goto errout_with_envp;
}
- ret = up_addrenv_vheap(&binp->addrenv, &vheap);
+ ret = up_addrenv_vheap(&binp->addrenv.pgdir, &vheap);
Review Comment:
can we add an local variable to avoid binp->addrenv.pgdir dup four times?
##########
binfmt/libelf/libelf_addrenv.c:
##########
@@ -89,27 +90,32 @@ int elf_addrenv_alloc(FAR struct elf_loadinfo_s *loadinfo, size_t textsize,
/* Create an address environment for the new ELF task */
- ret = up_addrenv_create(textsize, datasize, heapsize, &loadinfo->addrenv);
+ ret = up_addrenv_create(textsize, datasize, heapsize,
+ &loadinfo->addrenv.pgdir);
Review Comment:
add local variable for loadinfo->addrenv.pgdir?
##########
include/nuttx/addrenv.h:
##########
@@ -218,13 +225,27 @@
(CONFIG_ARCH_PGPOOL_VBASE + CONFIG_ARCH_PGPOOL_SIZE)
#endif
+/****************************************************************************
+ * Public Type Definitions
+ ****************************************************************************/
+
+struct tcb_s; /* Forward reference to TCB */
/****************************************************************************
* Public Types
****************************************************************************/
#ifndef __ASSEMBLY__
+struct addrenv_s
+{
+ struct arch_addrenv_s pgdir; /* The address environment page directory */
Review Comment:
should we use addrenv too?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095086783
##########
sched/sched/sched_releasetcb.c:
##########
@@ -154,7 +154,11 @@ int nxsched_release_tcb(FAR struct tcb_s *tcb, uint8_t ttype)
#ifdef CONFIG_ARCH_ADDRENV
/* Release this thread's reference to the address environment */
- ret = up_addrenv_detach(tcb->group, tcb);
+ ret = up_addrenv_detach(tcb);
+ if (ttype == TCB_FLAG_TTYPE_TASK)
Review Comment:
It is fixed by the reference counter in the second patch. I think the 1st patch on itself does not work properly, the reference counter is needed as well. If you think it is better to merge / squash the first 2 patches I can do that ?
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097070781
##########
binfmt/binfmt_execmodule.c:
##########
@@ -164,23 +164,23 @@ int exec_module(FAR const struct binary_s *binp,
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
- ret = up_addrenv_select(&binp->addrenv, &oldenv);
+ ret = addrenv_select((struct addrenv_s *)&binp->addrenv);
Review Comment:
Because binp is declared as const here, but the reference counter is needed in addrenv_select() so it must be made mutable
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095348435
##########
sched/sched/sched_releasetcb.c:
##########
@@ -154,7 +154,11 @@ int nxsched_release_tcb(FAR struct tcb_s *tcb, uint8_t ttype)
#ifdef CONFIG_ARCH_ADDRENV
/* Release this thread's reference to the address environment */
- ret = up_addrenv_detach(tcb->group, tcb);
+ ret = up_addrenv_detach(tcb);
+ if (ttype == TCB_FLAG_TTYPE_TASK)
Review Comment:
Both patch is very big, let's keep as it.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1090746638
##########
sched/addrenv/addrenv.c:
##########
@@ -0,0 +1,523 @@
+/****************************************************************************
+ * sched/addrenv/addrenv.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <nuttx/wqueue.h>
+
+#include "sched/sched.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_dsr
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+static void addrenv_dsr(FAR void *arg)
+{
+ FAR struct addrenv_s *addrenv = (FAR struct addrenv_s *)arg;
+
+ /* Destroy the address environment */
+
+ up_addrenv_destroy(&addrenv->pgdir);
+
+ /* Then finally release the memory */
+
+ kmm_free(addrenv);
+}
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* This variable holds the current address environment. These contents are
+ * _never_ NULL, besides when the system is started and there are only the
+ * initial kernel mappings available.
+ *
+ * This must only be accessed with interrupts disabled.
+ */
+
+static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Instantiate the group address environment for the current thread at the
+ * the head of the ready to run list.
+ *
+ * This function is called from platform-specific code after any context
+ * switch (i.e., after any change in the thread at the head of the
+ * ready-to-run list). This function will change the address environment
+ * if the new thread is part of a different task group.
+ *
+ * Input Parameters:
+ * tcb - The TCB of thread that needs an address environment. This should
+ * be the TCB at the head of the ready-to-run list, but that is not
+ * enough.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. A negated errno value is returned on
+ * any failure.
+ *
+ * Assumptions:
+ * This function should only be called within critical OS sections with
+ * interrupts disabled. Interrupts are disabled internally just to be
+ * certain, however.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb)
+{
+ FAR struct addrenv_s *curr;
+ FAR struct addrenv_s *next;
+ irqstate_t flags;
+ int cpu;
+ int ret;
+
+ /* NULL for the tcb means to use the TCB of the task at the head of the
+ * ready to run list.
+ */
+
+ if (!tcb)
+ {
+ tcb = this_task();
+ }
+
+ DEBUGASSERT(tcb);
+ next = tcb->mm_curr;
+
+ /* Does the group have an address environment? */
+
+ if (!next)
+ {
+ /* No... just return perhaps leaving a different address environment
+ * intact.
+ */
+
+ return OK;
+ }
+
+ flags = enter_critical_section();
+
+ cpu = this_cpu();
+ curr = g_addrenv[cpu];
+
+ /* Are we going to change address environments? */
+
+ if (curr != next)
+ {
+ /* Yes.. Is there a current address environment in place? */
+
+ if (curr)
+ {
+ /* We need to flush the D-Cache and Invalidate the I-Cache for
+ * the group whose environment is disappearing.
+ */
+
+ up_addrenv_coherent(&curr->pgdir);
+ }
+
+ /* While the address environment is instantiated, it cannot be freed */
+
+ addrenv_take(next);
+
+ /* Instantiate the new address environment (removing the old
+ * environment in the process). For the case of kernel threads,
+ * the old mappings will be removed and no new mappings will be
+ * instantiated.
+ */
+
+ ret = up_addrenv_select(&next->pgdir);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_select failed: %d\n", ret);
+ }
+
+ /* This is a safe spot to drop the current address environment */
+
+ if (curr)
+ {
+ addrenv_drop(curr, true);
+ }
+
+ /* Save the new, current address environment group */
+
+ g_addrenv[cpu] = next;
+ }
+
+ leave_critical_section(flags);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
+{
+ int ret = OK;
+
+ if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+ {
+ tcb->mm_own = NULL;
+ }
+ else
+ {
+ tcb->mm_own = (FAR struct addrenv_s *)
+ kmm_zalloc(sizeof(struct addrenv_s));
+ if (tcb->mm_own == NULL)
+ {
+ ret = -ENOMEM;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb)
+{
+ if (tcb->mm_own != NULL)
+ {
+ kmm_free(tcb->mm_own);
+ tcb->mm_own = NULL;
+ }
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly created group. Called by exec()
+ * right before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct arch_addrenv_s *addrenv)
+{
+ int ret;
+
+ /* Clone the address environment for us */
+
+ ret = up_addrenv_clone(addrenv, &tcb->mm_own->pgdir);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_clone failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Attach the address environment */
+
+ tcb->mm_curr = tcb->mm_own;
+ tcb->mm_own->refs = 1;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process
+ * tcb - The tcb of the child process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ ret = up_addrenv_attach(ptcb, tcb);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_attach failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Take a reference to the address environment */
+
+ addrenv_take(ptcb->mm_own);
+
+ /* Share the parent's address environment */
+
+ tcb->mm_own = ptcb->mm_own;
+ tcb->mm_curr = tcb->mm_own;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ /* Detach from the address environment */
+
+ ret = up_addrenv_detach(tcb);
+
+ /* Then drop the address environment */
+
+ addrenv_drop(tcb->mm_own, false);
+ tcb->mm_own = NULL;
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_select
+ *
+ * Description:
+ * Temporarily select a different address environment for the currently
+ * running process.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_select(FAR struct addrenv_s *addrenv)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_take(addrenv);
+ tcb->mm_curr = addrenv;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_restore
+ *
+ * Description:
+ * Switch back to the procces's own address environment.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_restore(void)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_give(tcb->mm_curr);
+ tcb->mm_curr = tcb->mm_own;
+ return addrenv_switch(tcb);
Review Comment:
This logic is still a bit faulty. The binfmt (or elf/nxflat) temporary addrenv is freed in addrenv_switch() -> addrenv_drop() and it obviously fails because it is not heap.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1098260898
##########
sched/addrenv/addrenv.c:
##########
@@ -0,0 +1,562 @@
+/****************************************************************************
+ * sched/addrenv/addrenv.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <nuttx/wqueue.h>
+
+#include "sched/sched.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* This variable holds the current address environment. These contents are
+ * _never_ NULL, besides when the system is started and there are only the
+ * initial kernel mappings available.
+ *
+ * This must only be accessed with interrupts disabled.
+ *
+ * REVISIT: Try to get rid of this, global bookkeeping for this is dangerous.
+ */
+
+static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_destroy
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+ *
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void addrenv_destroy(FAR void *arg)
+{
+ FAR struct addrenv_s *addrenv = (FAR struct addrenv_s *)arg;
+
+ /* Destroy the address environment */
+
+ up_addrenv_destroy(&addrenv->addrenv);
+
+ /* Then finally release the memory */
+
+ kmm_free(addrenv);
+}
+
+/****************************************************************************
+ * Name: addrenv_clear_current
+ *
+ * Description:
+ * Clear the current addrenv from g_addrenv, if it matches the input.
+ *
+ * Input Parameters:
+ * addrenv - Pointer to the addrenv to free.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+static void addrenv_clear_current(FAR const addrenv_t *addrenv)
+{
+ int i;
+
+ /* Mark no address environment */
+
+ for (i = 0; i < CONFIG_SMP_NCPUS; i++)
+ {
+ if (addrenv == g_addrenv[i])
+ {
+ g_addrenv[i] = NULL;
+ }
+ }
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Instantiate the group address environment for the current thread at the
+ * the head of the ready to run list.
+ *
+ * This function is called from platform-specific code after any context
+ * switch (i.e., after any change in the thread at the head of the
+ * ready-to-run list). This function will change the address environment
+ * if the new thread is part of a different task group.
+ *
+ * Input Parameters:
+ * tcb - The TCB of thread that needs an address environment. This should
+ * be the TCB at the head of the ready-to-run list, but that is not
+ * enough.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. A negated errno value is returned on
+ * any failure.
+ *
+ * Assumptions:
+ * This function should only be called within critical OS sections with
+ * interrupts disabled. Interrupts are disabled internally just to be
+ * certain, however.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb)
+{
+ FAR struct addrenv_s *curr;
+ FAR struct addrenv_s *next;
+ irqstate_t flags;
+ int cpu;
+ int ret;
+
+ /* NULL for the tcb means to use the TCB of the task at the head of the
+ * ready to run list.
+ */
+
+ if (!tcb)
+ {
+ tcb = this_task();
+ }
+
+ DEBUGASSERT(tcb);
+ next = tcb->addrenv_curr;
+
+ /* Does the group have an address environment? */
+
+ if (!next)
+ {
+ /* No... just return perhaps leaving a different address environment
+ * intact.
+ */
+
+ return OK;
+ }
+
+ flags = enter_critical_section();
+
+ cpu = this_cpu();
+ curr = g_addrenv[cpu];
+
+ /* Are we going to change address environments? */
+
+ if (curr != next)
+ {
+ /* Yes.. Is there a current address environment in place? */
+
+ if (curr)
+ {
+ /* We need to flush the D-Cache and Invalidate the I-Cache for
+ * the group whose environment is disappearing.
+ */
+
+ up_addrenv_coherent(&curr->addrenv);
+ }
+
+ /* While the address environment is instantiated, it cannot be freed */
+
+ addrenv_take(next);
+
+ /* Instantiate the new address environment (removing the old
+ * environment in the process). For the case of kernel threads,
+ * the old mappings will be removed and no new mappings will be
+ * instantiated.
+ */
+
+ ret = up_addrenv_select(&next->addrenv);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_select failed: %d\n", ret);
+ }
+
+ /* This is a safe spot to drop the current address environment */
+
+ if (curr)
+ {
+ addrenv_drop(curr, true);
+ }
+
+ /* Save the new, current address environment group */
+
+ g_addrenv[cpu] = next;
+ }
+
+ leave_critical_section(flags);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
+{
+ int ret = OK;
+
+ if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+ {
+ tcb->addrenv_own = NULL;
+ }
+ else
+ {
+ tcb->addrenv_own = (FAR struct addrenv_s *)
+ kmm_zalloc(sizeof(struct addrenv_s));
+ if (tcb->addrenv_own == NULL)
+ {
+ ret = -ENOMEM;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb)
+{
+ if (tcb->addrenv_own != NULL)
+ {
+ kmm_free(tcb->addrenv_own);
+ tcb->addrenv_own = NULL;
+ }
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly created group. Called by exec()
+ * right before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct addrenv_s *addrenv)
+{
+ int ret;
+
+ /* Clone the address environment for us */
+
+ ret = up_addrenv_clone(&addrenv->addrenv, &tcb->addrenv_own->addrenv);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_clone failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Attach the address environment */
+
+ tcb->addrenv_curr = tcb->addrenv_own;
+ tcb->addrenv_own->refs = 1;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process
+ * tcb - The tcb of the child process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ ret = up_addrenv_attach(ptcb, tcb);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_attach failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Take a reference to the address environment */
+
+ addrenv_take(ptcb->addrenv_own);
+
+ /* Share the parent's address environment */
+
+ tcb->addrenv_own = ptcb->addrenv_own;
+ tcb->addrenv_curr = tcb->addrenv_own;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ /* Detach from the address environment */
+
+ ret = up_addrenv_detach(tcb);
+
+ /* Then drop the address environment */
+
+ addrenv_drop(tcb->addrenv_own, false);
+ tcb->addrenv_own = NULL;
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_select
+ *
+ * Description:
+ * Temporarily select a different address environment for the currently
+ * running process.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_select(FAR struct addrenv_s *addrenv)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_take(addrenv);
+ tcb->addrenv_curr = addrenv;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_restore
+ *
+ * Description:
+ * Switch back to the procces's own address environment.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_restore(void)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_give(tcb->addrenv_curr);
+
+ if (tcb->addrenv_own == NULL)
+ {
+ /* Kernel thread, clear g_addrenv, as it is not valid any more */
+
+ addrenv_clear_current(tcb->addrenv_curr);
+ }
+
+ tcb->addrenv_curr = tcb->addrenv_own;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_take
+ *
+ * Description:
+ * Take a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+void addrenv_take(FAR struct addrenv_s *addrenv)
+{
+ irqstate_t flags = enter_critical_section();
+ addrenv->refs++;
+ leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: addrenv_give
+ *
+ * Description:
+ * Give back a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_give(FAR struct addrenv_s *addrenv)
+{
+ irqstate_t flags;
+ int refs;
+
+ flags = enter_critical_section();
+ refs = --addrenv->refs;
+ leave_critical_section(flags);
+
+ return refs;
+}
+
+/****************************************************************************
+ * Name: addrenv_drop
+ *
+ * Description:
+ * Drop an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ * deferred - yes: The address environment should be dropped by the worker
+ * no: The address environment can be dropped at once
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
Review Comment:
Done now
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095450990
##########
arch/arm/src/armv7-a/arm_addrenv.c:
##########
@@ -98,8 +98,6 @@
#include <assert.h>
#include <debug.h>
-#include <nuttx/addrenv.h>
Review Comment:
Done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097128261
##########
include/nuttx/addrenv.h:
##########
@@ -218,13 +220,24 @@
(CONFIG_ARCH_PGPOOL_VBASE + CONFIG_ARCH_PGPOOL_SIZE)
#endif
+/****************************************************************************
+ * Public Type Definitions
+ ****************************************************************************/
+
+struct tcb_s; /* Forward reference to TCB */
/****************************************************************************
* Public Types
****************************************************************************/
#ifndef __ASSEMBLY__
+struct addrenv_s
+{
+ struct arch_addrenv_s pgdir; /* The address environment page directory */
+};
+
+typedef struct addrenv_s addrenv_t;
Review Comment:
Done
##########
include/nuttx/addrenv.h:
##########
@@ -269,6 +282,83 @@ struct addrenv_reserve_s
* Public Function Prototypes
****************************************************************************/
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype);
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Switch to an address environment
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task to switch to, or NULL to use the task at the
+ * head of the ready-to-run list.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb);
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly process. Called by exec() right
+ * before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct arch_addrenv_s *addrenv);
Review Comment:
In the 3rd patch
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097128601
##########
binfmt/binfmt_execmodule.c:
##########
@@ -164,23 +164,23 @@ int exec_module(FAR const struct binary_s *binp,
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
- ret = up_addrenv_select(&binp->addrenv, &oldenv);
+ ret = addrenv_select((struct addrenv_s *)&binp->addrenv);
if (ret < 0)
{
- berr("ERROR: up_addrenv_select() failed: %d\n", ret);
+ berr("ERROR: addrenv_select() failed: %d\n", ret);
goto errout_with_envp;
}
- ret = up_addrenv_vheap(&binp->addrenv, &vheap);
+ ret = up_addrenv_vheap(&binp->addrenv.pgdir, &vheap);
Review Comment:
Done
##########
binfmt/libelf/libelf_addrenv.c:
##########
@@ -89,27 +90,32 @@ int elf_addrenv_alloc(FAR struct elf_loadinfo_s *loadinfo, size_t textsize,
/* Create an address environment for the new ELF task */
- ret = up_addrenv_create(textsize, datasize, heapsize, &loadinfo->addrenv);
+ ret = up_addrenv_create(textsize, datasize, heapsize,
+ &loadinfo->addrenv.pgdir);
Review Comment:
Done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097767509
##########
sched/addrenv/addrenv.c:
##########
@@ -0,0 +1,562 @@
+/****************************************************************************
+ * sched/addrenv/addrenv.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <nuttx/wqueue.h>
+
+#include "sched/sched.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/* This variable holds the current address environment. These contents are
+ * _never_ NULL, besides when the system is started and there are only the
+ * initial kernel mappings available.
+ *
+ * This must only be accessed with interrupts disabled.
+ *
+ * REVISIT: Try to get rid of this, global bookkeeping for this is dangerous.
+ */
+
+static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_destroy
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+ *
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void addrenv_destroy(FAR void *arg)
+{
+ FAR struct addrenv_s *addrenv = (FAR struct addrenv_s *)arg;
+
+ /* Destroy the address environment */
+
+ up_addrenv_destroy(&addrenv->addrenv);
+
+ /* Then finally release the memory */
+
+ kmm_free(addrenv);
+}
+
+/****************************************************************************
+ * Name: addrenv_clear_current
+ *
+ * Description:
+ * Clear the current addrenv from g_addrenv, if it matches the input.
+ *
+ * Input Parameters:
+ * addrenv - Pointer to the addrenv to free.
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+static void addrenv_clear_current(FAR const addrenv_t *addrenv)
+{
+ int i;
+
+ /* Mark no address environment */
+
+ for (i = 0; i < CONFIG_SMP_NCPUS; i++)
+ {
+ if (addrenv == g_addrenv[i])
+ {
+ g_addrenv[i] = NULL;
+ }
+ }
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Instantiate the group address environment for the current thread at the
+ * the head of the ready to run list.
+ *
+ * This function is called from platform-specific code after any context
+ * switch (i.e., after any change in the thread at the head of the
+ * ready-to-run list). This function will change the address environment
+ * if the new thread is part of a different task group.
+ *
+ * Input Parameters:
+ * tcb - The TCB of thread that needs an address environment. This should
+ * be the TCB at the head of the ready-to-run list, but that is not
+ * enough.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. A negated errno value is returned on
+ * any failure.
+ *
+ * Assumptions:
+ * This function should only be called within critical OS sections with
+ * interrupts disabled. Interrupts are disabled internally just to be
+ * certain, however.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb)
+{
+ FAR struct addrenv_s *curr;
+ FAR struct addrenv_s *next;
+ irqstate_t flags;
+ int cpu;
+ int ret;
+
+ /* NULL for the tcb means to use the TCB of the task at the head of the
+ * ready to run list.
+ */
+
+ if (!tcb)
+ {
+ tcb = this_task();
+ }
+
+ DEBUGASSERT(tcb);
+ next = tcb->addrenv_curr;
+
+ /* Does the group have an address environment? */
+
+ if (!next)
+ {
+ /* No... just return perhaps leaving a different address environment
+ * intact.
+ */
+
+ return OK;
+ }
+
+ flags = enter_critical_section();
+
+ cpu = this_cpu();
+ curr = g_addrenv[cpu];
+
+ /* Are we going to change address environments? */
+
+ if (curr != next)
+ {
+ /* Yes.. Is there a current address environment in place? */
+
+ if (curr)
+ {
+ /* We need to flush the D-Cache and Invalidate the I-Cache for
+ * the group whose environment is disappearing.
+ */
+
+ up_addrenv_coherent(&curr->addrenv);
+ }
+
+ /* While the address environment is instantiated, it cannot be freed */
+
+ addrenv_take(next);
+
+ /* Instantiate the new address environment (removing the old
+ * environment in the process). For the case of kernel threads,
+ * the old mappings will be removed and no new mappings will be
+ * instantiated.
+ */
+
+ ret = up_addrenv_select(&next->addrenv);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_select failed: %d\n", ret);
+ }
+
+ /* This is a safe spot to drop the current address environment */
+
+ if (curr)
+ {
+ addrenv_drop(curr, true);
+ }
+
+ /* Save the new, current address environment group */
+
+ g_addrenv[cpu] = next;
+ }
+
+ leave_critical_section(flags);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
+{
+ int ret = OK;
+
+ if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+ {
+ tcb->addrenv_own = NULL;
+ }
+ else
+ {
+ tcb->addrenv_own = (FAR struct addrenv_s *)
+ kmm_zalloc(sizeof(struct addrenv_s));
+ if (tcb->addrenv_own == NULL)
+ {
+ ret = -ENOMEM;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb)
+{
+ if (tcb->addrenv_own != NULL)
+ {
+ kmm_free(tcb->addrenv_own);
+ tcb->addrenv_own = NULL;
+ }
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly created group. Called by exec()
+ * right before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct addrenv_s *addrenv)
+{
+ int ret;
+
+ /* Clone the address environment for us */
+
+ ret = up_addrenv_clone(&addrenv->addrenv, &tcb->addrenv_own->addrenv);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_clone failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Attach the address environment */
+
+ tcb->addrenv_curr = tcb->addrenv_own;
+ tcb->addrenv_own->refs = 1;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process
+ * tcb - The tcb of the child process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ ret = up_addrenv_attach(ptcb, tcb);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_attach failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Take a reference to the address environment */
+
+ addrenv_take(ptcb->addrenv_own);
+
+ /* Share the parent's address environment */
+
+ tcb->addrenv_own = ptcb->addrenv_own;
+ tcb->addrenv_curr = tcb->addrenv_own;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ /* Detach from the address environment */
+
+ ret = up_addrenv_detach(tcb);
+
+ /* Then drop the address environment */
+
+ addrenv_drop(tcb->addrenv_own, false);
+ tcb->addrenv_own = NULL;
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_select
+ *
+ * Description:
+ * Temporarily select a different address environment for the currently
+ * running process.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_select(FAR struct addrenv_s *addrenv)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_take(addrenv);
+ tcb->addrenv_curr = addrenv;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_restore
+ *
+ * Description:
+ * Switch back to the procces's own address environment.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_restore(void)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_give(tcb->addrenv_curr);
+
+ if (tcb->addrenv_own == NULL)
+ {
+ /* Kernel thread, clear g_addrenv, as it is not valid any more */
+
+ addrenv_clear_current(tcb->addrenv_curr);
+ }
+
+ tcb->addrenv_curr = tcb->addrenv_own;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_take
+ *
+ * Description:
+ * Take a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+void addrenv_take(FAR struct addrenv_s *addrenv)
+{
+ irqstate_t flags = enter_critical_section();
+ addrenv->refs++;
+ leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: addrenv_give
+ *
+ * Description:
+ * Give back a reference to an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_give(FAR struct addrenv_s *addrenv)
+{
+ irqstate_t flags;
+ int refs;
+
+ flags = enter_critical_section();
+ refs = --addrenv->refs;
+ leave_critical_section(flags);
+
+ return refs;
+}
+
+/****************************************************************************
+ * Name: addrenv_drop
+ *
+ * Description:
+ * Drop an address environment.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ * deferred - yes: The address environment should be dropped by the worker
+ * no: The address environment can be dropped at once
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
Review Comment:
Just noticed that the function description is wrong for many of these. Copy&pasted "Returned value" field. I will patch it tomorrow.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095347745
##########
include/nuttx/sched.h:
##########
@@ -532,6 +526,12 @@ struct tcb_s
FAR struct task_group_s *group; /* Pointer to shared task group data */
+ /* Address Environment ****************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+ FAR struct addrenv_s *mm_own; /* Task (group) own memory mappings */
Review Comment:
could be, I think addrenv is better prefix than mm since many place use addrenv.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#issuecomment-1408675705
There is some cleanup still to be done for other platforms. It seems the call to "group_addrenv" exists on platforms that do not and never have supported address environments, so it is a bit tedious to fix them. Will push this fix tomorrow.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1090746638
##########
sched/addrenv/addrenv.c:
##########
@@ -0,0 +1,523 @@
+/****************************************************************************
+ * sched/addrenv/addrenv.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <nuttx/wqueue.h>
+
+#include "sched/sched.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_dsr
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+static void addrenv_dsr(FAR void *arg)
+{
+ FAR struct addrenv_s *addrenv = (FAR struct addrenv_s *)arg;
+
+ /* Destroy the address environment */
+
+ up_addrenv_destroy(&addrenv->pgdir);
+
+ /* Then finally release the memory */
+
+ kmm_free(addrenv);
+}
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* This variable holds the current address environment. These contents are
+ * _never_ NULL, besides when the system is started and there are only the
+ * initial kernel mappings available.
+ *
+ * This must only be accessed with interrupts disabled.
+ */
+
+static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Instantiate the group address environment for the current thread at the
+ * the head of the ready to run list.
+ *
+ * This function is called from platform-specific code after any context
+ * switch (i.e., after any change in the thread at the head of the
+ * ready-to-run list). This function will change the address environment
+ * if the new thread is part of a different task group.
+ *
+ * Input Parameters:
+ * tcb - The TCB of thread that needs an address environment. This should
+ * be the TCB at the head of the ready-to-run list, but that is not
+ * enough.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. A negated errno value is returned on
+ * any failure.
+ *
+ * Assumptions:
+ * This function should only be called within critical OS sections with
+ * interrupts disabled. Interrupts are disabled internally just to be
+ * certain, however.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb)
+{
+ FAR struct addrenv_s *curr;
+ FAR struct addrenv_s *next;
+ irqstate_t flags;
+ int cpu;
+ int ret;
+
+ /* NULL for the tcb means to use the TCB of the task at the head of the
+ * ready to run list.
+ */
+
+ if (!tcb)
+ {
+ tcb = this_task();
+ }
+
+ DEBUGASSERT(tcb);
+ next = tcb->mm_curr;
+
+ /* Does the group have an address environment? */
+
+ if (!next)
+ {
+ /* No... just return perhaps leaving a different address environment
+ * intact.
+ */
+
+ return OK;
+ }
+
+ flags = enter_critical_section();
+
+ cpu = this_cpu();
+ curr = g_addrenv[cpu];
+
+ /* Are we going to change address environments? */
+
+ if (curr != next)
+ {
+ /* Yes.. Is there a current address environment in place? */
+
+ if (curr)
+ {
+ /* We need to flush the D-Cache and Invalidate the I-Cache for
+ * the group whose environment is disappearing.
+ */
+
+ up_addrenv_coherent(&curr->pgdir);
+ }
+
+ /* While the address environment is instantiated, it cannot be freed */
+
+ addrenv_take(next);
+
+ /* Instantiate the new address environment (removing the old
+ * environment in the process). For the case of kernel threads,
+ * the old mappings will be removed and no new mappings will be
+ * instantiated.
+ */
+
+ ret = up_addrenv_select(&next->pgdir);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_select failed: %d\n", ret);
+ }
+
+ /* This is a safe spot to drop the current address environment */
+
+ if (curr)
+ {
+ addrenv_drop(curr, true);
+ }
+
+ /* Save the new, current address environment group */
+
+ g_addrenv[cpu] = next;
+ }
+
+ leave_critical_section(flags);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
+{
+ int ret = OK;
+
+ if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+ {
+ tcb->mm_own = NULL;
+ }
+ else
+ {
+ tcb->mm_own = (FAR struct addrenv_s *)
+ kmm_zalloc(sizeof(struct addrenv_s));
+ if (tcb->mm_own == NULL)
+ {
+ ret = -ENOMEM;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb)
+{
+ if (tcb->mm_own != NULL)
+ {
+ kmm_free(tcb->mm_own);
+ tcb->mm_own = NULL;
+ }
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly created group. Called by exec()
+ * right before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct arch_addrenv_s *addrenv)
+{
+ int ret;
+
+ /* Clone the address environment for us */
+
+ ret = up_addrenv_clone(addrenv, &tcb->mm_own->pgdir);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_clone failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Attach the address environment */
+
+ tcb->mm_curr = tcb->mm_own;
+ tcb->mm_own->refs = 1;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process
+ * tcb - The tcb of the child process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ ret = up_addrenv_attach(ptcb, tcb);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_attach failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Take a reference to the address environment */
+
+ addrenv_take(ptcb->mm_own);
+
+ /* Share the parent's address environment */
+
+ tcb->mm_own = ptcb->mm_own;
+ tcb->mm_curr = tcb->mm_own;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ /* Detach from the address environment */
+
+ ret = up_addrenv_detach(tcb);
+
+ /* Then drop the address environment */
+
+ addrenv_drop(tcb->mm_own, false);
+ tcb->mm_own = NULL;
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_select
+ *
+ * Description:
+ * Temporarily select a different address environment for the currently
+ * running process.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_select(FAR struct addrenv_s *addrenv)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_take(addrenv);
+ tcb->mm_curr = addrenv;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_restore
+ *
+ * Description:
+ * Switch back to the procces's own address environment.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_restore(void)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_give(tcb->mm_curr);
+ tcb->mm_curr = tcb->mm_own;
+ return addrenv_switch(tcb);
Review Comment:
This logic is still a bit faulty. The binfmt (or elf/nxflat) temporary addrenv is freed here and it obviously fails because it is not heap.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1098293357
##########
sched/addrenv/addrenv.c:
##########
@@ -79,13 +79,42 @@ static void addrenv_destroy(FAR void *arg)
/* Destroy the address environment */
- up_addrenv_destroy(&addrenv->pgdir);
+ up_addrenv_destroy(&addrenv->addrenv);
Review Comment:
Oops, done now
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1090588218
##########
arch/risc-v/src/common/riscv_doirq.c:
##########
@@ -98,7 +98,7 @@ uintptr_t *riscv_doirq(int irq, uintptr_t *regs)
* thread at the head of the ready-to-run list.
*/
- group_addrenv(NULL);
Review Comment:
This needs to be fixed to other targets too
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1091552332
##########
sched/addrenv/addrenv.c:
##########
@@ -0,0 +1,523 @@
+/****************************************************************************
+ * sched/addrenv/addrenv.c
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <assert.h>
+#include <debug.h>
+
+#include <nuttx/addrenv.h>
+#include <nuttx/irq.h>
+#include <nuttx/sched.h>
+#include <nuttx/wqueue.h>
+
+#include "sched/sched.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_dsr
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+static void addrenv_dsr(FAR void *arg)
+{
+ FAR struct addrenv_s *addrenv = (FAR struct addrenv_s *)arg;
+
+ /* Destroy the address environment */
+
+ up_addrenv_destroy(&addrenv->pgdir);
+
+ /* Then finally release the memory */
+
+ kmm_free(addrenv);
+}
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/* This variable holds the current address environment. These contents are
+ * _never_ NULL, besides when the system is started and there are only the
+ * initial kernel mappings available.
+ *
+ * This must only be accessed with interrupts disabled.
+ */
+
+static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_switch
+ *
+ * Description:
+ * Instantiate the group address environment for the current thread at the
+ * the head of the ready to run list.
+ *
+ * This function is called from platform-specific code after any context
+ * switch (i.e., after any change in the thread at the head of the
+ * ready-to-run list). This function will change the address environment
+ * if the new thread is part of a different task group.
+ *
+ * Input Parameters:
+ * tcb - The TCB of thread that needs an address environment. This should
+ * be the TCB at the head of the ready-to-run list, but that is not
+ * enough.
+ *
+ * Returned Value:
+ * Zero (OK) is returned on success. A negated errno value is returned on
+ * any failure.
+ *
+ * Assumptions:
+ * This function should only be called within critical OS sections with
+ * interrupts disabled. Interrupts are disabled internally just to be
+ * certain, however.
+ *
+ ****************************************************************************/
+
+int addrenv_switch(FAR struct tcb_s *tcb)
+{
+ FAR struct addrenv_s *curr;
+ FAR struct addrenv_s *next;
+ irqstate_t flags;
+ int cpu;
+ int ret;
+
+ /* NULL for the tcb means to use the TCB of the task at the head of the
+ * ready to run list.
+ */
+
+ if (!tcb)
+ {
+ tcb = this_task();
+ }
+
+ DEBUGASSERT(tcb);
+ next = tcb->mm_curr;
+
+ /* Does the group have an address environment? */
+
+ if (!next)
+ {
+ /* No... just return perhaps leaving a different address environment
+ * intact.
+ */
+
+ return OK;
+ }
+
+ flags = enter_critical_section();
+
+ cpu = this_cpu();
+ curr = g_addrenv[cpu];
+
+ /* Are we going to change address environments? */
+
+ if (curr != next)
+ {
+ /* Yes.. Is there a current address environment in place? */
+
+ if (curr)
+ {
+ /* We need to flush the D-Cache and Invalidate the I-Cache for
+ * the group whose environment is disappearing.
+ */
+
+ up_addrenv_coherent(&curr->pgdir);
+ }
+
+ /* While the address environment is instantiated, it cannot be freed */
+
+ addrenv_take(next);
+
+ /* Instantiate the new address environment (removing the old
+ * environment in the process). For the case of kernel threads,
+ * the old mappings will be removed and no new mappings will be
+ * instantiated.
+ */
+
+ ret = up_addrenv_select(&next->pgdir);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_select failed: %d\n", ret);
+ }
+
+ /* This is a safe spot to drop the current address environment */
+
+ if (curr)
+ {
+ addrenv_drop(curr, true);
+ }
+
+ /* Save the new, current address environment group */
+
+ g_addrenv[cpu] = next;
+ }
+
+ leave_critical_section(flags);
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_allocate
+ *
+ * Description:
+ * Allocate an address environment for a new process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly created task.
+ * ttype - The type of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_allocate(FAR struct tcb_s *tcb, uint8_t ttype)
+{
+ int ret = OK;
+
+ if ((ttype & TCB_FLAG_TTYPE_MASK) == TCB_FLAG_TTYPE_KERNEL)
+ {
+ tcb->mm_own = NULL;
+ }
+ else
+ {
+ tcb->mm_own = (FAR struct addrenv_s *)
+ kmm_zalloc(sizeof(struct addrenv_s));
+ if (tcb->mm_own == NULL)
+ {
+ ret = -ENOMEM;
+ }
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_free
+ *
+ * Description:
+ * Free an address environment for a process.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the task.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_free(FAR struct tcb_s *tcb)
+{
+ if (tcb->mm_own != NULL)
+ {
+ kmm_free(tcb->mm_own);
+ tcb->mm_own = NULL;
+ }
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_attach
+ *
+ * Description:
+ * Attach address environment to a newly created group. Called by exec()
+ * right before injecting the new process into the system.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the newly loaded task.
+ * addrenv - The address environment that is attached.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_attach(FAR struct tcb_s *tcb,
+ FAR const struct arch_addrenv_s *addrenv)
+{
+ int ret;
+
+ /* Clone the address environment for us */
+
+ ret = up_addrenv_clone(addrenv, &tcb->mm_own->pgdir);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_clone failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Attach the address environment */
+
+ tcb->mm_curr = tcb->mm_own;
+ tcb->mm_own->refs = 1;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_join
+ *
+ * Description:
+ * Join the parent process's address environment.
+ *
+ * Input Parameters:
+ * ptcb - The tcb of the parent process
+ * tcb - The tcb of the child process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_join(FAR struct tcb_s *ptcb, FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ ret = up_addrenv_attach(ptcb, tcb);
+ if (ret < 0)
+ {
+ berr("ERROR: up_addrenv_attach failed: %d\n", ret);
+ return ret;
+ }
+
+ /* Take a reference to the address environment */
+
+ addrenv_take(ptcb->mm_own);
+
+ /* Share the parent's address environment */
+
+ tcb->mm_own = ptcb->mm_own;
+ tcb->mm_curr = tcb->mm_own;
+
+ return OK;
+}
+
+/****************************************************************************
+ * Name: addrenv_leave
+ *
+ * Description:
+ * Leave a process's address environment.
+ *
+ * Input Parameters:
+ * tcb - The tcb of the process
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_leave(FAR struct tcb_s *tcb)
+{
+ int ret;
+
+ /* Detach from the address environment */
+
+ ret = up_addrenv_detach(tcb);
+
+ /* Then drop the address environment */
+
+ addrenv_drop(tcb->mm_own, false);
+ tcb->mm_own = NULL;
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: addrenv_select
+ *
+ * Description:
+ * Temporarily select a different address environment for the currently
+ * running process.
+ *
+ * Input Parameters:
+ * addrenv - The address environment.
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_select(FAR struct addrenv_s *addrenv)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_take(addrenv);
+ tcb->mm_curr = addrenv;
+ return addrenv_switch(tcb);
+}
+
+/****************************************************************************
+ * Name: addrenv_restore
+ *
+ * Description:
+ * Switch back to the procces's own address environment.
+ *
+ * Input Parameters:
+ * None
+ *
+ * Returned Value:
+ * This is a NuttX internal function so it follows the convention that
+ * 0 (OK) is returned on success and a negated errno is returned on
+ * failure.
+ *
+ ****************************************************************************/
+
+int addrenv_restore(void)
+{
+ FAR struct tcb_s *tcb = this_task();
+ addrenv_give(tcb->mm_curr);
+ tcb->mm_curr = tcb->mm_own;
+ return addrenv_switch(tcb);
Review Comment:
Fixed by grabbing a reference in nxflat_addrenv_alloc / elf_addrenv_alloc.
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1095453141
##########
include/nuttx/sched.h:
##########
@@ -532,6 +526,12 @@ struct tcb_s
FAR struct task_group_s *group; /* Pointer to shared task group data */
+ /* Address Environment ****************************************************/
+
+#ifdef CONFIG_ARCH_ADDRENV
+ FAR struct addrenv_s *mm_own; /* Task (group) own memory mappings */
Review Comment:
Done
##########
sched/addrenv/addrenv.c:
##########
@@ -52,6 +53,39 @@
static FAR struct addrenv_s *g_addrenv[CONFIG_SMP_NCPUS];
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: addrenv_dsr
+ *
+ * Description:
+ * Deferred service routine for destroying an address environment. This is
+ * so that the heavy lifting is not done when the context is switching, or
+ * from ISR.
+ *
+ * Input Parameters:
+ * arg - Contains pointer to the address environment that is freed.
+ *
+ * Returned Value:
+ * None.
+ *
+ ****************************************************************************/
+
+static void addrenv_dsr(FAR void *arg)
Review Comment:
Done
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] pussuw commented on a diff in pull request #8355: arch/addrenv: Refactor address environment handling, by moving tg_addrenv out of the group structure
Posted by "pussuw (via GitHub)" <gi...@apache.org>.
pussuw commented on code in PR #8355:
URL: https://github.com/apache/nuttx/pull/8355#discussion_r1097144860
##########
binfmt/binfmt_execmodule.c:
##########
@@ -164,23 +164,23 @@ int exec_module(FAR const struct binary_s *binp,
#if defined(CONFIG_ARCH_ADDRENV) && defined(CONFIG_BUILD_KERNEL)
/* Instantiate the address environment containing the user heap */
- ret = up_addrenv_select(&binp->addrenv, &oldenv);
+ ret = addrenv_select((struct addrenv_s *)&binp->addrenv);
Review Comment:
Sure, I can do that
--
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: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org