Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions include/nuttx/fdpic.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,24 +105,23 @@ static inline FAR void *fdpic_callback(FAR void *fn)
* carries no module base. Elsewhere fdpic_callback() is enough.
*
* Input Parameters:
* arg - The one word argument.
* entry - The code address to enter, already resolved from the descriptor.
* got - The module data base to install.
* arg - The one word argument.
* desc - The entry point to enter and the data base to install.
*
****************************************************************************/

static inline void fdpic_invoke(uintptr_t arg, uintptr_t entry,
uintptr_t got)
static inline void fdpic_invoke(uintptr_t arg,
FAR const struct fdpic_desc_s *desc)
{
up_fdpic_invoke(arg, entry, got);
up_fdpic_invoke(arg, desc->entry, desc->got);
}

#else

# define fdpic_base() (0)
# define fdpic_callback(fn) (fn)
# define fdpic_invoke(arg, entry, got) \
((void)(got), (((CODE void (*)(uintptr_t))(uintptr_t)(entry))(arg)))
# define fdpic_invoke(arg, desc) \
(((CODE void (*)(uintptr_t))(uintptr_t)(desc)->entry)(arg))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we cast desc to function pointer directly for no fdpic case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can do it, for no benefit.


#endif /* CONFIG_FDPIC */

Expand Down
10 changes: 10 additions & 0 deletions include/nuttx/signal.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@

#include <nuttx/wqueue.h>
#include <nuttx/sched.h>
#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

/****************************************************************************
* Pre-processor Definitions
Expand Down Expand Up @@ -67,7 +70,14 @@ struct sigwork_s
{
struct work_s work; /* Work queue structure */
union sigval value; /* Data passed with notification */
#ifdef CONFIG_FDPIC
struct fdpic_desc_s desc; /* Notification function, and the data base
* of a module callback or zero. The base is
* captured at registration and installed
* around the call on the worker thread. */
#else
sigev_notify_function_t func; /* Notification function */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why need define desc at line 74? I suppose that func should point to fdpic_desc_s in fdpic case.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the firmware and the built-in applications are not FDPIC, so their sigev_notify_function is plain. Only way to tell them apart is the registering context.

#endif
};

#ifdef __cplusplus
Expand Down
14 changes: 14 additions & 0 deletions libs/libc/dirent/lib_scandir.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
#include <errno.h>
#include <stdlib.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

#include "libc.h"

/* The scandir() function is not appropriate for use within the kernel in its
Expand Down Expand Up @@ -91,6 +95,15 @@ int scandir(FAR const char *path, FAR struct dirent ***namelist,
* the original errno value to be able to restore it in case of success.
*/

#ifdef CONFIG_FDPIC
/* Resolve the filter descriptor. compar is not resolved here: qsort()
* does it, and resolving twice reads a code address as a descriptor.
*/

filter = (CODE int (*)(FAR const struct dirent *))
fdpic_callback((FAR void *)filter);
#endif

errsv = get_errno();

dirp = opendir(path);
Expand Down Expand Up @@ -191,6 +204,7 @@ int scandir(FAR const char *path, FAR struct dirent ***namelist,
if (compar)
{
typedef int (*compar_fn_t)(FAR const void *, FAR const void *);

qsort(list, cnt, sizeof(*list), (compar_fn_t)compar);
}

Expand Down
14 changes: 14 additions & 0 deletions libs/libc/pthread/pthread_create.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@

#include <nuttx/pthread.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

/****************************************************************************
* Private Functions
****************************************************************************/
Expand Down Expand Up @@ -88,6 +92,16 @@ static void pthread_startup(pthread_startroutine_t entry,
int pthread_create(FAR pthread_t *thread, FAR const pthread_attr_t *attr,
pthread_startroutine_t pthread_entry, pthread_addr_t arg)
{
#ifdef CONFIG_FDPIC
/* Resolve the descriptor once, in the public entry point. The new
* thread inherits the creator's D-Space, so it needs only the code
* address.
*/

pthread_entry = (pthread_startroutine_t)
fdpic_callback((FAR void *)pthread_entry);
#endif

return nx_pthread_create(pthread_startup, thread, attr, pthread_entry,
arg);
}
13 changes: 13 additions & 0 deletions libs/libc/pthread/pthread_once.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@
#include <nuttx/mutex.h>
#include <nuttx/debug.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

/****************************************************************************
* Public Functions
****************************************************************************/
Expand Down Expand Up @@ -73,6 +77,15 @@ int pthread_once(FAR pthread_once_t *once_control,
return EINVAL;
}

#ifdef CONFIG_FDPIC
/* Resolve the descriptor here. The value is a local copy, so a later
* call through the same once_control resolves afresh.
*/

init_routine = (CODE void (*)(void))
fdpic_callback((FAR void *)init_routine);
#endif

if (!once_control->done)
{
pthread_mutex_lock(&once_control->mutex);
Expand Down
4 changes: 4 additions & 0 deletions libs/libc/signal/sig_signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ _sa_handler_t signal(int signo, _sa_handler_t func)

DEBUGASSERT(func != SIG_ERR && func != SIG_HOLD);

/* Not resolved here. nxsig_action() resolves the handler, which covers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the comment?

* a module calling sigaction() directly as well.
*/

/* Initialize the sigaction structure */

act.sa_handler = func;
Expand Down
11 changes: 11 additions & 0 deletions libs/libc/stdlib/lib_bsearch.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
****************************************************************************/

#include <stdlib.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif
#include <assert.h>

/****************************************************************************
Expand Down Expand Up @@ -114,6 +118,13 @@ FAR void *bsearch(FAR const void *key, FAR const void *base, size_t nel,
DEBUGASSERT(base != NULL || nel == 0);
DEBUGASSERT(compar != NULL);

#ifdef CONFIG_FDPIC
/* See qsort(): an FDPIC caller passes a descriptor, not a code address */

compar = (CODE int (*)(FAR const void *, FAR const void *))
fdpic_callback((FAR void *)compar);
#endif

for (lim = nel, lower = (const char *)base; lim != 0; lim >>= 1)
{
middle = lower + (lim >> 1) * width;
Expand Down
35 changes: 32 additions & 3 deletions libs/libc/stdlib/lib_qsort.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@
#include <sys/param.h>
#include <stdlib.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
Expand Down Expand Up @@ -156,8 +160,9 @@ static inline FAR char *med3(FAR char *a, FAR char *b, FAR char *c,
*
****************************************************************************/

void qsort(FAR void *base, size_t nel, size_t width,
CODE int(*compar)(FAR const void *, FAR const void *))
static void qsort_internal(FAR void *base, size_t nel, size_t width,
CODE int(*compar)(FAR const void *,
FAR const void *))
{
FAR char *pa;
FAR char *pb;
Expand Down Expand Up @@ -277,7 +282,7 @@ void qsort(FAR void *base, size_t nel, size_t width,

if ((r = pb - pa) > width)
{
qsort(base, r / width, width, compar);
qsort_internal(base, r / width, width, compar);
}

if ((r = pd - pc) > width)
Expand All @@ -289,3 +294,27 @@ void qsort(FAR void *base, size_t nel, size_t width,
goto loop;
}
}

/****************************************************************************
* Name: qsort
*
* Description:
* Public entry point. Resolves the comparison function once, then hands
* an ordinary pointer to the implementation, which recurses.
*
****************************************************************************/

void qsort(FAR void *base, size_t nel, size_t width,
CODE int(*compar)(FAR const void *, FAR const void *))
{
#ifdef CONFIG_FDPIC
/* An FDPIC module passes the address of a function descriptor, not a
* code address.
*/

compar = (CODE int (*)(FAR const void *, FAR const void *))
fdpic_callback((FAR void *)compar);
#endif

qsort_internal(base, nel, width, compar);
}
27 changes: 27 additions & 0 deletions sched/mqueue/mq_notify.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
#include <nuttx/irq.h>
#include <nuttx/sched.h>

#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD)
# include <nuttx/fdpic.h>
#endif

#include "sched/sched.h"
#include "mqueue/mqueue.h"

Expand Down Expand Up @@ -156,6 +160,29 @@ int mq_notify(mqd_t mqdes, FAR const struct sigevent *notification)
sizeof(struct sigevent));

msgq->ntpid = rtcb->pid;

#if defined(CONFIG_FDPIC) && defined(CONFIG_SIG_EVTHREAD)
/* Resolve the callback here, where this still runs in the
* module's context. It fires later on a worker that carries no
* data base, so the base travels with it. The descriptor holds
* the base of the module the callback belongs to, which is not
* always the caller's. The function shares a union with the
* thread ID, so only a SIGEV_THREAD event has one to resolve.
*/

msgq->ntwork.desc.got = 0;

if ((notification->sigev_notify & SIGEV_THREAD) != 0 &&
fdpic_base() != 0)
{
FAR struct fdpic_desc_s *desc =
(FAR void *)notification->sigev_notify_function;

msgq->ntevent.sigev_notify_function =
(sigev_notify_function_t)desc->entry;
msgq->ntwork.desc.got = desc->got;
}
#endif
}
}

Expand Down
17 changes: 17 additions & 0 deletions sched/signal/sig_action.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
#include <nuttx/signal.h>
#include <nuttx/spinlock.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

#include "sched/sched.h"
#include "group/group.h"
#include "signal/signal.h"
Expand Down Expand Up @@ -326,6 +330,19 @@ int nxsig_action(int signo, FAR const struct sigaction *act,

handler = act->sa_handler;

#ifdef CONFIG_FDPIC
/* Resolve the handler here, the innermost common code, so it happens
* exactly once. SIG_ERR, SIG_IGN, SIG_DFL and SIG_HOLD are small
* integers rather than addresses, so exclude them by hand.
*/

if (handler != SIG_ERR && handler != SIG_IGN && handler != SIG_DFL &&
handler != SIG_HOLD)
{
handler = (_sa_handler_t)fdpic_callback((FAR void *)handler);
}
#endif

#ifdef CONFIG_SIG_DEFAULT
/* If the caller is setting the handler to SIG_DFL, then we need to
* replace this with the correct, internal default signal action handler.
Expand Down
24 changes: 24 additions & 0 deletions sched/signal/sig_notification.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@

#include <nuttx/signal.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

#include "sched/sched.h"
#include "signal/signal.h"

Expand Down Expand Up @@ -70,7 +74,23 @@ static void nxsig_notification_worker(FAR void *arg)

/* Perform the callback */

#ifdef CONFIG_FDPIC
/* The worker does not carry the module's data base. Install the base
* captured at registration around the call. A zero base means the
* callback is not a module's.
*/

if (work->desc.got != 0)
{
fdpic_invoke((uintptr_t)work->value.sival_ptr, &work->desc);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not call fdpic_invoke(work->value, xxx->sigev_notify_function) directly? and remove the change in mq_notify.c and desc field in sigwork_s.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get it. nxsig_notification() does not run in the registering context. Drivers, timer_settime() and mq_sndinternal() may not run in the module context.

}
else
{
((sigev_notify_function_t)work->desc.entry)(work->value);
}
#else
work->func(work->value);
#endif
}

#endif /* CONFIG_SIG_EVTHREAD */
Expand Down Expand Up @@ -155,7 +175,11 @@ int nxsig_notification(pid_t pid, FAR struct sigevent *event,
/* Initialize the work information */

work->value = event->sigev_value;
#ifdef CONFIG_FDPIC
work->desc.entry = (uintptr_t)event->sigev_notify_function;
#else
work->func = event->sigev_notify_function;
#endif

/* Then queue the work */

Expand Down
18 changes: 16 additions & 2 deletions sched/task/task_create.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@
#include <nuttx/kthread.h>
#include <nuttx/fs/fs.h>

#ifdef CONFIG_FDPIC
# include <nuttx/fdpic.h>
#endif

#include "sched/sched.h"
#include "group/group.h"
#include "task/task.h"
Expand Down Expand Up @@ -202,8 +206,18 @@ int task_create_with_stack(FAR const char *name, int priority,
FAR void *stack_addr, int stack_size,
main_t entry, FAR char * const argv[])
{
int ret = nxtask_create(name, priority, stack_addr,
stack_size, entry, argv, NULL);
int ret;

#ifdef CONFIG_FDPIC
/* Resolve here, once: this covers task_create() too, which is a plain
* forwarder. The new task inherits the creator's D-Space.
*/

entry = (main_t)fdpic_callback((FAR void *)entry);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why not change the invocation point by fdpic_invoke instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the new task already gets r9 from the inherited dspace (nxtask_dup_dspace() -> up_initial_state() -> REG_PIC) and keeps it for its life, so only the entry needs unwrapping; fdpic_invoke() is for a callback on a shared worker, which has no base.

#endif

ret = nxtask_create(name, priority, stack_addr,
stack_size, entry, argv, NULL);
if (ret < 0)
{
set_errno(-ret);
Expand Down
Loading
Loading