Solved. As you pointed out, Xenomai inverts the returned value from request_region. So, that was a bug in my application.

However, turns out that instead of request_region, I have to use request_mem_region. This is because the I/O region only goes up to 2^32, but the mem region goes up to 2^64.

So, attached is a patch to add two new syscalls: rt_misc_get_mem_region and rt_misc_put_mem_region.

   Steve


Philippe Gerum wrote:
Steven A. Falco wrote:
Your patch makes sense.

I have some results, but I'm not sure I understand what they mean.  I've
attached the test program that I am using.  Here is what it outputs:

bash-3.00# ./o2
Trying to free nonexistent resource <0000000000000000-00000000c0000001>
get leds: -16 Device or resource busy
put leds: 0 Success

Trying to free nonexistent resource <0000000000000000-ffffffffffffffff>
get low_mem: -16 Device or resource busy
put low_mem: 0 Success

Actually, we should define rt_misc_put_io_region() as a void routine,
since the kernel does not return any status from release_region(). At
this chance, I've fixed this, since returning 0 albeit a failure message
is dumped to the kernel log upon error is disturbing.

I am a little unclear on request_resource() - the return code is
backwards of what I would have expected.  Looking at examples in the
kernel, it appears that request_resource() returns EBUSY when things go
well, and it returns 0 when things go badly.  Like I said, that seems
backwards, but I guess it makes sense - EBUSY apparently means that the
resource is _now_ busy?


request_resource() should return a valid resource descriptor address
upon success, which Xenomai converts to zero. Conversely, -EBUSY is
returned if request_region() sends us back a NULL value, since this is
how check_region() behaves.

Anyway, following the kernel examples, my program considers a non-zero
return as success.  At that point I release the region.  If instead, I
get a zero return, then I treat that as a failure, and don't release the
region.

The part I don't understand is why I get the "Trying to free nonexistent
resource" messages.  Since I am getting an EBUSY, I thought that meant I
owned the resource, and that I should release it...


I suspect request_region() did actually fail.

Also, the addresses printed above are a bit strange.  For example, I
would have thought that instead of
"<0000000000000000-00000000c0000001>", it would print
"<00000001c0000000-00000001c0000013>".  Perhaps that is a clue - maybe
the start and length are not being passed correctly.

One more question.  It appears that if my program crashes, that the
region will never be released.  So, the normal behavior of an exiting
process freeing all its resources doesn't seem to be guaranteed.


As a matter of fact, we track all common objects like sema4s, mutexes,
queues, and so on, but not I/O regions. Consider this as an illustration
of our absolute laziness, since we do have the proper infrastructure to
handle I/O regions in the auto-cleanup process too.

    Steve


Philippe Gerum wrote:
Philippe Gerum wrote:
Steven A. Falco wrote:
The rt_misc_get_io_region() has the "start" argument as an unsigned long. On the PPC440, we have a 36-bit address space, where the I/O registers are generally above the 4GB area. For example, the UART is at address 0x1ef600300.

The Linux request_region call has "start" typed as a resource_size_t, which is a u64 on the PPC440 (i.e. CONFIG_RESOURCES_64BIT is set even though this is a 23-bit processor).

Is this something that should be handled by xeno-config? It could append a CFLAG indicating the size of a resource.
Or use a 64bit long unconditionally, to keep the same kernel-based
implementation, since there is no performance issue for this call. In
any case, we need to fix the API before 2.4 final is out -- which will
also affect the ABI, but it already changed during the 2.4 development
phase anyway.

Does this patch work for you?



Index: include/native/misc.h
===================================================================
--- include/native/misc.h	(revision 3101)
+++ include/native/misc.h	(working copy)
@@ -39,6 +39,13 @@
 int rt_misc_put_io_region(unsigned long start,
 			  unsigned long len);
 
+int rt_misc_get_mem_region(uint64_t start,
+			  unsigned long len,
+			  const char *label);
+
+int rt_misc_put_mem_region(uint64_t start,
+			  unsigned long len);
+
 #ifdef __cplusplus
 }
 #endif
Index: include/native/syscall.h
===================================================================
--- include/native/syscall.h	(revision 3101)
+++ include/native/syscall.h	(working copy)
@@ -119,6 +119,8 @@
 #define __native_timer_tsc2ns       93
 #define __native_queue_write        94
 #define __native_queue_read         95
+#define __native_misc_get_mem_region 96
+#define __native_misc_put_mem_region 97
 
 struct rt_arg_bulk {
 
Index: src/skins/native/misc.c
===================================================================
--- src/skins/native/misc.c	(revision 3101)
+++ src/skins/native/misc.c	(working copy)
@@ -35,3 +35,17 @@
 	return XENOMAI_SKINCALL2(__native_muxid,
 				 __native_misc_put_io_region, start, len);
 }
+
+int rt_misc_get_mem_region(uint64_t start,
+			  unsigned long len, const char *label)
+{
+	return XENOMAI_SKINCALL3(__native_muxid,
+				 __native_misc_get_mem_region, &start, len,
+				 label);
+}
+
+int rt_misc_put_mem_region(uint64_t start, unsigned long len)
+{
+	return XENOMAI_SKINCALL2(__native_muxid,
+				 __native_misc_put_mem_region, &start, len);
+}
Index: ksrc/skins/native/syscall.c
===================================================================
--- ksrc/skins/native/syscall.c	(revision 3101)
+++ ksrc/skins/native/syscall.c	(working copy)
@@ -3692,6 +3692,64 @@
 	return 0;
 }
 
+/*
+ * int __rt_misc_get_mem_region(uint64_t *start,
+ *                             unsigned long len,
+ *                             const char *label)
+ */
+
+static int __rt_misc_get_mem_region(struct task_struct *curr,
+				   struct pt_regs *regs)
+{
+	unsigned long len;
+	uint64_t start;
+	char label[64];
+
+	if (!__xn_access_ok
+	    (curr, VERIFY_READ, __xn_reg_arg1(regs), sizeof(start)))
+		return -EFAULT;
+
+	if (!__xn_access_ok
+	    (curr, VERIFY_READ, __xn_reg_arg3(regs), sizeof(label)))
+		return -EFAULT;
+
+	__xn_copy_from_user(curr, &start, (void __user *)__xn_reg_arg1(regs),
+			    sizeof(start));
+
+	__xn_strncpy_from_user(curr, label,
+			       (const char __user *)__xn_reg_arg3(regs),
+			       sizeof(label) - 1);
+	label[sizeof(label) - 1] = '\0';
+
+	len = __xn_reg_arg2(regs);
+
+	return request_mem_region(start, len, label) ? 0 : -EBUSY;
+}
+
+/*
+ * int __rt_misc_put_mem_region(uint64_t *start,
+ *                             unsigned long len)
+ */
+
+static int __rt_misc_put_mem_region(struct task_struct *curr,
+				   struct pt_regs *regs)
+{
+	unsigned long len;
+	uint64_t start;
+
+	if (!__xn_access_ok
+	    (curr, VERIFY_READ, __xn_reg_arg1(regs), sizeof(start)))
+		return -EFAULT;
+
+	__xn_copy_from_user(curr, &start, (void __user *)__xn_reg_arg1(regs),
+			    sizeof(start));
+
+	len = __xn_reg_arg2(regs);
+	release_mem_region(start, len);
+
+	return 0;
+}
+
 static __attribute__ ((unused))
 int __rt_call_not_available(struct task_struct *curr, struct pt_regs *regs)
 {
@@ -3852,6 +3910,10 @@
 	    {&__rt_misc_put_io_region, __xn_exec_lostage},
 	[__native_timer_ns2tsc] = {&__rt_timer_ns2tsc, __xn_exec_any},
 	[__native_timer_tsc2ns] = {&__rt_timer_tsc2ns, __xn_exec_any},
+	[__native_misc_get_mem_region] =
+	    {&__rt_misc_get_mem_region, __xn_exec_lostage},
+	[__native_misc_put_mem_region] =
+	    {&__rt_misc_put_mem_region, __xn_exec_lostage},
 };
 
 extern xntbase_t *__native_tbase;
_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to