Re: [Xenomai-core] [BUG] target width of shifts on 64 bits

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1.1: Type: text/plain, Size: 2197 bytes --]

Philippe Gerum wrote:
> On Wed, 2007-04-18 at 20:13 +0200, Jan Kiszka wrote:
>> Hi Philippe,
>>
>> here is an explanation of the scalable scheduler issue I face on x86_64
>> under different gcc compilers:
>>
>>         unsigned long x = 0;
>>         int n = 32;
>>
>>         x |= 1 << n;
>>
>> The last instruction translates to:
>>
>> 	mov    0xfffffffffffffffc(%rbp),%ecx
>> 	mov    $0x1,%eax
>> 	shl    %cl,%eax
>> 	cltq
>> 	or     %rax,0xfffffffffffffff0(%rbp)
>>
> 
> Blast. Good spot.
> 
>> That means we only shift with 32-bit precision although the target type
>> is 64 bit. We find such code for setting the queue usage bits in
>> addmlq(), but probably elsewhere too. This variant lets gcc generate the
>> desired code:
>>
>> 	x |= (unsigned long)1 << n;
>>
>> Compiler issue, x86_64-specific oddity, or generic 64-bit problem we may
>> have across the ipipe and Xenomai code (ppc64, ia64?)?
>>
> 
> A brief look at the I-pipe code base shows that most shift expressions
> have righthand sides limited to small values (at least always < 32), and
> when they don't, the lefthand side is properly cast to long long values,
> so this should be ok.
> 
> BUT, it's a general 64bit port issue for Xenomai, which is not specific
> to the multi-level queue implementation. We have the same issue going on
> with at least:
> 
> - the posix registry
> - the message pipe support from the nucleus
> - the vrtx id generator
> 
> Gentlemen, it's time for bug hunting.
> 
>> After patching nucleus/queue.h appropriately, my oopses disappear, but
>> RT threads still do not run (no CSW to the threads latency creates).
>> Jan
>>
>>
>> PS: If you are interested, I could post a modified qemu patch that
>> enables gdb kernel debugging under x86_64.
>>
> 
> Yes please. I would have a look to the remaining issue I have here
> exclusively over qemu, which seems unrelated to the multi-level queue
> issue though.
> 

Attached. A post to qemu-devel is also on the way. I wonder way the
original patch by Jason Wessel, posted last September, or a variant of
it still didn't make it into a qemu release or at least its CVS. Anyway.

Jan

[-- Attachment #1.2: x86_64-gdb.patch --]
[-- Type: text/plain, Size: 2562 bytes --]

Index: qemu-0.9.0/gdbstub.c
===================================================================
--- qemu-0.9.0.orig/gdbstub.c
+++ qemu-0.9.0/gdbstub.c
@@ -220,9 +220,78 @@ static int put_packet(GDBState *s, char 
     }
     return 0;
 }
+#if defined(TARGET_X86_64)
+/* Defines from GDB register struct numbers */
+#define _RAX	0
+#define _RBX	1
+#define _RCX	2
+#define _RDX	3
+#define _RSI	4
+#define _RDI	5
+#define _RBP	6
+#define _RSP	7
+#define _R8	8
+#define _R15	15
+#define _PC	16
+#define _PS	17
+#define _CS	18
+#define _SS	19
+#define _DS	20
+#define _ES	21
+#define _FS	22
+#define _GS	23
+#define _NREGS	24
 
-#if defined(TARGET_I386)
+static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
+{
+    uint64_t *registers = (uint64_t *)mem_buf;
+    int i;
+
+    registers[_RAX] = env->regs[R_EAX];
+    registers[_RBX] = env->regs[R_EBX];
+    registers[_RCX] = env->regs[R_ECX];
+    registers[_RDX] = env->regs[R_EDX];
+    registers[_RSI] = env->regs[R_ESI];
+    registers[_RDI] = env->regs[R_EDI];
+    registers[_RBP] = env->regs[R_EBP];
+    registers[_RSP] = env->regs[R_ESP];
+    for (i = 8; i < 16; i++)
+        registers[i] = env->regs[i];
+    registers[_PC] = env->eip;
+    registers[_PS] = env->eflags;
+    registers[_CS] = env->segs[R_CS].selector;
+    registers[_SS] = env->segs[R_SS].selector;
+    registers[_DS] = env->segs[R_DS].selector;
+    registers[_ES] = env->segs[R_ES].selector;
+    registers[_FS] = env->segs[R_FS].selector;
+    registers[_GS] = env->segs[R_GS].selector;
+
+    for(i = 0; i < _NREGS; i++)
+        tswapl(registers[i]);
+
+    return _NREGS * 8;
+}
+
+static void cpu_gdb_write_registers(CPUState *env, uint8_t *mem_buf, int size)
+{
+    uint32_t *registers = (uint32_t *)mem_buf;
+    int i;
+
+    env->regs[R_EAX] = tswapl(registers[_RAX]);
+    env->regs[R_EBX] = tswapl(registers[_RBX]);
+    env->regs[R_ECX] = tswapl(registers[_RCX]);
+    env->regs[R_EDX] = tswapl(registers[_RDX]);
+    env->regs[R_ESI] = tswapl(registers[_RSI]);
+    env->regs[R_EDI] = tswapl(registers[_RDI]);
+    env->regs[R_EBP] = tswapl(registers[_RBP]);
+    env->regs[R_ESP] = tswapl(registers[_RSP]);
+    for (i = 8; i < 16; i++)
+        env->regs[i] = tswapl(registers[i]);
+    env->eip = tswapl(registers[_PC]);
+    env->eflags = tswapl(registers[_PS]);
+}
 
+#elif defined(TARGET_I386)
 static int cpu_gdb_read_registers(CPUState *env, uint8_t *mem_buf)
 {
     uint32_t *registers = (uint32_t *)mem_buf;

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]