[Xenomai-core] [PATCH 0/2] xnvdso mechanism and unit test

[Xenomai-core] [PATCH 0/2] xnvdso mechanism and unit test

[Wolfgang Mauerer]

Hi,

the following two mails integrate Gilles' comments into
the xnvdso patch and add a unit test. Notice that this submission
does not consider efficient access of shared data from
kernel space. Since this problem is independent from the
xnvdso mechanism, it will be addressed when I submit the NTP
time correction sharing code.

Thanks, Wolfgang

[Xenomai-core] [PATCH 1/2] Add support for sharing kernel/userland data between Xenomai and Linux

[Wolfgang Mauerer]

A new structure (struct xnshared) is introduced. It allows us to share
generic data between user and kernel of Linux and Xenomai; a bitmap
of feature flags located at the beginning of the structure identifies
which data are shared. The structure is allocated in the global semaphore
heap, and xnsysinfo.xnshared_offset identifies the offset of the
structure within the heap.

Currently, no shared features are yet supported - the patch only
introduces the necessary ABI changes. When the need arises
to share data between said entities, the structure must
be accordingly extended, and a new feature bit must be added
to the flags.

Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@domain.hid>
Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---
 include/asm-generic/syscall.h |    1 +
 include/nucleus/Makefile.am   |    3 +-
 include/nucleus/xnvdso.h      |   61 +++++++++++++++++++++++++++++++++++++++++
 ksrc/nucleus/module.c         |    7 +++++
 ksrc/nucleus/shadow.c         |   22 +++++++++++++++
 5 files changed, 93 insertions(+), 1 deletions(-)
 create mode 100644 include/nucleus/xnvdso.h

diff --git a/include/asm-generic/syscall.h b/include/asm-generic/syscall.h
index 483b99f..8f1ddc6 100644
--- a/include/asm-generic/syscall.h
+++ b/include/asm-generic/syscall.h
@@ -53,6 +53,7 @@
 typedef struct xnsysinfo {
 	unsigned long long cpufreq;	/* CPU frequency */
 	unsigned long tickval;		/* Tick duration (ns) */
+	unsigned long xnvdso_off;  	/* Offset of xnvdso in the sem heap */
 } xnsysinfo_t;
 
 #define SIGSHADOW  SIGWINCH
diff --git a/include/nucleus/Makefile.am b/include/nucleus/Makefile.am
index 4be05f8..26d3fa2 100644
--- a/include/nucleus/Makefile.am
+++ b/include/nucleus/Makefile.am
@@ -34,4 +34,5 @@ includesub_HEADERS = \
 	trace.h \
 	types.h \
 	version.h \
-	xenomai.h
+	xenomai.h \
+	xnvdso.h
diff --git a/include/nucleus/xnvdso.h b/include/nucleus/xnvdso.h
new file mode 100644
index 0000000..fbff8fa
--- /dev/null
+++ b/include/nucleus/xnvdso.h
@@ -0,0 +1,61 @@
+#ifndef _XENO_NUCLEUS_XNVDSO_H
+#define _XENO_NUCLEUS_XNVDSO_H
+
+/*!\file xnvdso.h
+ * \brief Definitions for global semaphore heap shared objects
+ * \author Wolfgang Mauerer
+ *
+ * Copyright (C) 2009 Wolfgang Mauerer <wolfgang.mauerer@domain.hid>.
+ *
+ * Xenomai is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2 of the License,
+ * or (at your option) any later version.
+ *
+ * Xenomai is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Xenomai; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA
+ * 02111-1307, USA.
+ */
+
+#include <nucleus/types.h>
+
+/*
+ * Data shared between Xenomai kernel/userland and the Linux kernel/userland
+ * on the global semaphore heap. The features element indicates which data are
+ * shared. Notice that struct xnvdso may only grow, but never shrink.
+ */
+struct xnvdso {
+	unsigned long long features;
+
+	/* Embed domain specific structures that describe the
+	 * shared data here */
+};
+
+/*
+ * For each shared feature, add a flag below. For now, the set is still
+ * empty.
+ */
+/*
+#define XNVDSO_FEAT_A	0x0000000000000001
+#define XNVDSO_FEAT_B	0x0000000000000002
+#define XNVDSO_FEAT_C	0x0000000000000004
+#define XNVDSO_FEATURES	(XNVDSO_FEAT_A | XNVDSO_FEAT_B | XVDSO_FEAT_C)
+*/
+
+#define XNVDSO_FEATURES 0x0000000000000000
+
+extern struct xnvdso *xnvdso;
+
+static inline int xnvdso_test_feature(unsigned long long feature)
+{
+	return testbits(xnvdso->features, feature);
+}
+
+extern void xnheap_init_vdso(void);
+#endif /* _XENO_NUCLEUS_XNVDSO_H */
diff --git a/ksrc/nucleus/module.c b/ksrc/nucleus/module.c
index 5404182..0a17661 100644
--- a/ksrc/nucleus/module.c
+++ b/ksrc/nucleus/module.c
@@ -35,6 +35,11 @@
 #endif /* CONFIG_XENO_OPT_PIPE */
 #include <nucleus/select.h>
 #include <asm/xenomai/bits/init.h>
+#ifdef CONFIG_XENO_OPT_PERVASIVE
+#include <nucleus/xnvdso.h>
+#else
+static inline void xnheap_init_vdso(void) { }
+#endif /* CONFIG_XENO_OPT_PERVASIVE */
 
 MODULE_DESCRIPTION("Xenomai nucleus");
 MODULE_AUTHOR("rpm@xenomai.org");
@@ -106,6 +111,8 @@ int __init __xeno_sys_init(void)
 		goto cleanup_arch;
 
 	xnheap_set_label(&__xnsys_global_ppd.sem_heap, "global sem heap");
+
+	xnheap_init_vdso();
 #endif
 	
 #ifdef __KERNEL__
diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c
index d0cb416..bff7dc5 100644
--- a/ksrc/nucleus/shadow.c
+++ b/ksrc/nucleus/shadow.c
@@ -50,6 +50,7 @@
 #include <nucleus/trace.h>
 #include <nucleus/stat.h>
 #include <nucleus/sys_ppd.h>
+#include <nucleus/xnvdso.h>
 #include <asm/xenomai/features.h>
 #include <asm/xenomai/syscall.h>
 #include <asm/xenomai/bits/shadow.h>
@@ -693,6 +694,24 @@ void xnshadow_clear_sig(xnthread_t *thread, unsigned muxid)
 }
 EXPORT_SYMBOL_GPL(xnshadow_clear_sig);
 
+struct xnvdso *xnvdso;
+EXPORT_SYMBOL_GPL(xnvdso);
+
+/*
+ * We re-use the global semaphore heap to provide a multi-purpose shared
+ * memory area between Xenomai and Linux - for both kernel and userland
+ */
+void __init xnheap_init_vdso(void)
+{
+	xnvdso = (struct xnvdso *)xnheap_alloc(&__xnsys_global_ppd.sem_heap,
+						  sizeof(*xnvdso));
+
+	if (!xnvdso)
+		xnpod_fatal("Xenomai: cannot allocate memory for xnvdso!\n");
+
+	xnvdso->features = XNVDSO_FEATURES;
+}
+
 static inline void handle_rt_signals(xnthread_t *thread,
 				     struct pt_regs *regs,
 				     int sysflags)
@@ -1746,6 +1765,9 @@ static int xnshadow_sys_info(struct pt_regs *regs)
 
 	info.cpufreq = xnarch_get_cpu_freq();
 
+	info.xnvdso_off =
+	  xnheap_mapped_offset(&xnsys_ppd_get(1)->sem_heap, xnvdso);
+
 	return __xn_safe_copy_to_user((void __user *)infarg, &info, sizeof(info));
 }
 
-- 
1.6.4

[Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism

[Wolfgang Mauerer]

This testcase checks if the value in xnvdso->features matches the
value of XNVDSO_FEATURES, that is, if the information is correctly
transferred from kernel to userland.

Notice that the approach will fail once configurations are supported
that know of multiple features and implement only some of them.
In this case, the testcase needs to be extended accordingly
to check that only the expected features are present.

Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@domain.hid>
Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---
 src/testsuite/unit/Makefile.am |   15 +++++++++++++-
 src/testsuite/unit/runinfo.in  |    1 +
 src/testsuite/unit/xnvdso.c    |   43 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 1 deletions(-)
 create mode 100644 src/testsuite/unit/xnvdso.c

diff --git a/src/testsuite/unit/Makefile.am b/src/testsuite/unit/Makefile.am
index 24d077a..c77cc54 100644
--- a/src/testsuite/unit/Makefile.am
+++ b/src/testsuite/unit/Makefile.am
@@ -2,7 +2,8 @@ testdir = $(exec_prefix)/share/xenomai/testsuite/unit
 
 CCLD = $(top_srcdir)/scripts/wrap-link.sh $(CC)
 
-bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native
+bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native \
+               xnvdso
 
 arith_SOURCES = arith.c arith-noinline.c arith-noinline.h
 
@@ -53,6 +54,18 @@ mutex_torture_native_LDADD = \
 	../../skins/native/libnative.la \
 	-lpthread -lm
 
+xnvdso_SOURCES = xnvdso.c
+
+xnvdso_CPPFLAGS = \
+	@XENO_USER_CFLAGS@ \
+	-I$(top_srcdir)/include
+
+xnvdso_LDFLAGS = @XENO_USER_LDFLAGS@
+
+xnvdso_LDADD = \
+	../../skins/native/libnative.la \
+	-lpthread -lm
+
 install-data-local:
 	$(mkinstalldirs) $(DESTDIR)$(testdir)
 	@sed -e's,@exec_prefix\@,$(exec_prefix),g' $(srcdir)/runinfo.in > $(DESTDIR)$(testdir)/.runinfo
diff --git a/src/testsuite/unit/runinfo.in b/src/testsuite/unit/runinfo.in
index f4cd208..a22afc0 100644
--- a/src/testsuite/unit/runinfo.in
+++ b/src/testsuite/unit/runinfo.in
@@ -2,3 +2,4 @@ arith:native:!@exec_prefix@domain.hid
 wakeup-time:native:!@exec_prefix@domain.hid
 mutex-torture-posix:posix:!@exec_prefix@domain.hid
 mutex-torture-native:native:!@exec_prefix@domain.hid
+xnvdso:native:!@exec_prefix@domain.hid
diff --git a/src/testsuite/unit/xnvdso.c b/src/testsuite/unit/xnvdso.c
new file mode 100644
index 0000000..6d7d8e1
--- /dev/null
+++ b/src/testsuite/unit/xnvdso.c
@@ -0,0 +1,43 @@
+/*
+ * VDSO feature set testcase
+ * by Wolfgang Mauerer <wolfgang.mauerer@domain.hid>
+ */
+
+#include <stdio.h>
+#include <asm/xenomai/syscall.h>
+#include <nucleus/xnvdso.h>
+
+extern unsigned long xeno_sem_heap[2];
+
+int main(int argc, char **argv)
+{
+	int err;
+	xnsysinfo_t sysinfo;
+	struct xnvdso *xnvdso;
+
+	if (!xeno_sem_heap[1]) {
+		fprintf(stderr, "Could not determine position of the "
+			"global semaphore heap\n");
+		return 1;
+	}
+
+	/* The muxid is irrelevant for this test as long as it's valid */
+	err = XENOMAI_SYSCALL2(__xn_sys_info, 1, &sysinfo);
+	if (err < 0) {
+		fprintf(stderr, "sys_sys_info failed: %d\n", err);
+		return 1;
+	}
+
+	printf("Address of the global semaphore heap: 0x%lx\n",
+	       xeno_sem_heap[1]);
+	printf("Offset of xnvdso: %lu\n", sysinfo.xnvdso_off);
+
+	xnvdso = (struct xnvdso *)(xeno_sem_heap[1] + sysinfo.xnvdso_off);
+	printf("Contents of the features flag: %llu\n", xnvdso->features);
+
+	if (xnvdso->features == XNVDSO_FEATURES)
+		return 0;
+
+	fprintf(stderr, "error: xnvdso->features != XNVDSO_FEATURES\n");
+	return 1;
+}
-- 
1.6.4

Re: [Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism

[Gilles Chanteperdrix]

Wolfgang Mauerer wrote:
> This testcase checks if the value in xnvdso->features matches the
> value of XNVDSO_FEATURES, that is, if the information is correctly
> transferred from kernel to userland.
> 
> Notice that the approach will fail once configurations are supported
> that know of multiple features and implement only some of them.
> In this case, the testcase needs to be extended accordingly
> to check that only the expected features are present.

Please allow to pass the value we want to check on the test command line.

-- 
					    Gilles.

Re: [Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism

[Wolfgang Mauerer]

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]

Gilles Chanteperdrix wrote:
> Wolfgang Mauerer wrote:
>> This testcase checks if the value in xnvdso->features matches the
>> value of XNVDSO_FEATURES, that is, if the information is correctly
>> transferred from kernel to userland.
>>
>> Notice that the approach will fail once configurations are supported
>> that know of multiple features and implement only some of them.
>> In this case, the testcase needs to be extended accordingly
>> to check that only the expected features are present.
> 
> Please allow to pass the value we want to check on the test command line.

here you go. I suppose keeping XNVDSO_FEATURES as default value
unless something different is specified is okay?

Cheers,

Wolfgang

[-- Attachment #2: testcase.diff --]
[-- Type: text/x-patch, Size: 3701 bytes --]

commit 0f31dc5c50f7034dddbac67c22e53823f1be72d3
Author: Wolfgang Mauerer <wolfgang.mauerer@domain.hid>
Date:   Tue Dec 22 23:11:31 2009 +0100

    Testcase for the xnvdso mechanism
    
    This testcase checks if the value in xnvdso->features matches the
    feature set specified on the command line. When no explicit
    feature test set is given, XNVDSO_FEATURES (i.e., the set
    of all features known to the current Xenomai revision) is used.
    
    Notice that the default approach will naturally fail once
    configurations are supported that know of multiple features and
    implement only some of them.
    
    Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@domain.hid>
    Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>

diff --git a/src/testsuite/unit/Makefile.am b/src/testsuite/unit/Makefile.am
index 24d077a..c77cc54 100644
--- a/src/testsuite/unit/Makefile.am
+++ b/src/testsuite/unit/Makefile.am
@@ -2,7 +2,8 @@ testdir = $(exec_prefix)/share/xenomai/testsuite/unit
 
 CCLD = $(top_srcdir)/scripts/wrap-link.sh $(CC)
 
-bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native
+bin_PROGRAMS = arith wakeup-time mutex-torture-posix mutex-torture-native \
+               xnvdso
 
 arith_SOURCES = arith.c arith-noinline.c arith-noinline.h
 
@@ -53,6 +54,18 @@ mutex_torture_native_LDADD = \
 	../../skins/native/libnative.la \
 	-lpthread -lm
 
+xnvdso_SOURCES = xnvdso.c
+
+xnvdso_CPPFLAGS = \
+	@XENO_USER_CFLAGS@ \
+	-I$(top_srcdir)/include
+
+xnvdso_LDFLAGS = @XENO_USER_LDFLAGS@
+
+xnvdso_LDADD = \
+	../../skins/native/libnative.la \
+	-lpthread -lm
+
 install-data-local:
 	$(mkinstalldirs) $(DESTDIR)$(testdir)
 	@sed -e's,@exec_prefix\@,$(exec_prefix),g' $(srcdir)/runinfo.in > $(DESTDIR)$(testdir)/.runinfo
diff --git a/src/testsuite/unit/runinfo.in b/src/testsuite/unit/runinfo.in
index f4cd208..a22afc0 100644
--- a/src/testsuite/unit/runinfo.in
+++ b/src/testsuite/unit/runinfo.in
@@ -2,3 +2,4 @@ arith:native:!@exec_prefix@domain.hid
 wakeup-time:native:!@exec_prefix@domain.hid
 mutex-torture-posix:posix:!@exec_prefix@domain.hid
 mutex-torture-native:native:!@exec_prefix@domain.hid
+xnvdso:native:!@exec_prefix@domain.hid
diff --git a/src/testsuite/unit/xnvdso.c b/src/testsuite/unit/xnvdso.c
new file mode 100644
index 0000000..69305c2
--- /dev/null
+++ b/src/testsuite/unit/xnvdso.c
@@ -0,0 +1,52 @@
+/*
+ * VDSO feature set testcase
+ * by Wolfgang Mauerer <wolfgang.mauerer@domain.hid>
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <asm/xenomai/syscall.h>
+#include <nucleus/xnvdso.h>
+
+extern unsigned long xeno_sem_heap[2];
+
+int main(int argc, char **argv)
+{
+	int err;
+	xnsysinfo_t sysinfo;
+	struct xnvdso *xnvdso;
+	unsigned long long test_features;
+
+	if (argc != 2) {
+		printf("No specific feature(s) given, using XNVDSO_FEATURE\n");
+		test_features = XNVDSO_FEATURES;
+	} else {
+		test_features = strtoull(argv[1], NULL, 0);
+	}
+
+	if (!xeno_sem_heap[1]) {
+		fprintf(stderr, "Could not determine position of the "
+			"global semaphore heap\n");
+		return 1;
+	}
+
+	/* The muxid is irrelevant for this test as long as it's valid */
+	err = XENOMAI_SYSCALL2(__xn_sys_info, 1, &sysinfo);
+	if (err < 0) {
+		fprintf(stderr, "sys_sys_info failed: %d\n", err);
+		return 1;
+	}
+
+	printf("Address of the global semaphore heap: 0x%lx\n",
+	       xeno_sem_heap[1]);
+	printf("Offset of xnvdso: %lu\n", sysinfo.xnvdso_off);
+
+	xnvdso = (struct xnvdso *)(xeno_sem_heap[1] + sysinfo.xnvdso_off);
+	printf("Contents of the features flag: %llu\n", xnvdso->features);
+
+	if (xnvdso->features == test_features)
+		return 0;
+
+	fprintf(stderr, "error: xnvdso->features != %llu\n", test_features);
+	return 1;
+}

Re: [Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism

[Gilles Chanteperdrix]

Wolfgang Mauerer wrote:
> Gilles Chanteperdrix wrote:
>> Wolfgang Mauerer wrote:
>>> This testcase checks if the value in xnvdso->features matches the
>>> value of XNVDSO_FEATURES, that is, if the information is correctly
>>> transferred from kernel to userland.
>>>
>>> Notice that the approach will fail once configurations are supported
>>> that know of multiple features and implement only some of them.
>>> In this case, the testcase needs to be extended accordingly
>>> to check that only the expected features are present.
>> Please allow to pass the value we want to check on the test command line.
> 
> here you go. I suppose keeping XNVDSO_FEATURES as default value
> unless something different is specified is okay?

Yes, ok. I am fine with the two patches.

-- 
					    Gilles.

Re: [Xenomai-core] [PATCH 2/2] Testcase for the xnvdso mechanism

[Gilles Chanteperdrix]

Gilles Chanteperdrix wrote:
> Wolfgang Mauerer wrote:
>> Gilles Chanteperdrix wrote:
>>> Wolfgang Mauerer wrote:
>>>> This testcase checks if the value in xnvdso->features matches the
>>>> value of XNVDSO_FEATURES, that is, if the information is correctly
>>>> transferred from kernel to userland.
>>>>
>>>> Notice that the approach will fail once configurations are supported
>>>> that know of multiple features and implement only some of them.
>>>> In this case, the testcase needs to be extended accordingly
>>>> to check that only the expected features are present.
>>> Please allow to pass the value we want to check on the test command line.
>> here you go. I suppose keeping XNVDSO_FEATURES as default value
>> unless something different is specified is okay?
> 
> Yes, ok. I am fine with the two patches.

Ok. I merged them, but renamed a bit the symbols to get things more in
line with what we do usually.

xnvdso is the struct
nkvdso is the global variable
the xnsysinfo_t member is now simply vdso.

-- 
					    Gilles.