[Xenomai-core] [PATCH] Fix ppc fpu support

[Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

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

Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but 
not yet set last_task_used_math to NULL. As a result the tasks MSR_FP 
will get set, although it should be cleared. If the task happens to hit 
one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU 
state might be saved to the task. The attached patch should fix this. I 
couldn't try it on most recent Xenomai trunk, because latency wouldn't 
build anymore. However, I see no reason it shouldn't work. All thee 
having trouble with X and Xenomai, give this a shot.

-- Heikki Lindholm

[-- Attachment #2: xenomai-060106-ppc-hwfpu.patch --]
[-- Type: text/plain, Size: 1551 bytes --]

diff -Nru xenomai/include/asm-powerpc/system.h xenomai-devel/include/asm-powerpc/system.h
--- xenomai/include/asm-powerpc/system.h	2006-01-06 15:55:19.000000000 +0200
+++ xenomai-devel/include/asm-powerpc/system.h	2006-01-06 16:44:53.000000000 +0200
@@ -55,6 +55,7 @@
     rthal_fpenv_t fpuenv  __attribute__ ((aligned (16)));
     rthal_fpenv_t *fpup;	/* Pointer to the FPU backup area */
     struct task_struct *user_fpu_owner;
+    unsigned long user_fpu_owner_prev_msr;
     /* Pointer the the FPU owner in userspace:
        - NULL for RT K threads,
        - last_task_used_math for Linux US threads (only current or NULL when MP)
@@ -368,7 +369,10 @@
         rthal_save_fpu(tcb->fpup);
 
         if(tcb->user_fpu_owner && tcb->user_fpu_owner->thread.regs)
+            {
+            tcb->user_fpu_owner_prev_msr = tcb->user_fpu_owner->thread.regs->msr;
             tcb->user_fpu_owner->thread.regs->msr &= ~MSR_FP;
+            }
         }   
 
 #endif /* CONFIG_XENO_HW_FPU */
@@ -383,7 +387,13 @@
         {
         rthal_restore_fpu(tcb->fpup);
 
-        if(tcb->user_fpu_owner && tcb->user_fpu_owner->thread.regs)
+	/* Note: Only enable FP in MSR, if it was enabled when we saved the
+	 * fpu state. We might have preempted Linux when it had disabled FP
+	 * for the thread, but not yet set last_task_used_math to NULL 
+	 */
+        if(tcb->user_fpu_owner && 
+			tcb->user_fpu_owner->thread.regs &&
+			((tcb->user_fpu_owner_prev_msr & MSR_FP) != 0))
             tcb->user_fpu_owner->thread.regs->msr |= MSR_FP;
         }   
 

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Jan Kiszka]

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

Heikki Lindholm wrote:
> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but
> not yet set last_task_used_math to NULL. As a result the tasks MSR_FP
> will get set, although it should be cleared. If the task happens to hit
> one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU
> state might be saved to the task. The attached patch should fix this. I
> couldn't try it on most recent Xenomai trunk, because latency wouldn't
> build anymore. However, I see no reason it shouldn't work. All thee

Is it still broken with latest revision 376? Philippe had merged the fix
for my mess, and it worked at least for 2.6 on my box again.

> having trouble with X and Xenomai, give this a shot.
> 
> -- Heikki Lindholm
> 

Jan

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

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

Jan Kiszka kirjoitti:
> Heikki Lindholm wrote:
> 
>>Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but
>>not yet set last_task_used_math to NULL. As a result the tasks MSR_FP
>>will get set, although it should be cleared. If the task happens to hit
>>one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU
>>state might be saved to the task. The attached patch should fix this. I
>>couldn't try it on most recent Xenomai trunk, because latency wouldn't
>>build anymore. However, I see no reason it shouldn't work. All thee
> 
> 
> Is it still broken with latest revision 376? Philippe had merged the fix
> for my mess, and it worked at least for 2.6 on my box again.

I'd say this has been unchanged since the beginning (0.9?).

-- hl

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Jan Kiszka]

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

Heikki Lindholm wrote:
> Jan Kiszka kirjoitti:
> 
>> Heikki Lindholm wrote:
>>
>>> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but
>>> not yet set last_task_used_math to NULL. As a result the tasks MSR_FP
>>> will get set, although it should be cleared. If the task happens to hit
>>> one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU
>>> state might be saved to the task. The attached patch should fix this. I
>>> couldn't try it on most recent Xenomai trunk, because latency wouldn't
>>> build anymore. However, I see no reason it shouldn't work. All thee
>>
>>
>>
>> Is it still broken with latest revision 376? Philippe had merged the fix
>> for my mess, and it worked at least for 2.6 on my box again.
> 
> 
> I'd say this has been unchanged since the beginning (0.9?).
> 

Then, what latency are we talking about? My last modifications went to
src/testsuite/latency. Did you mean ksrc/skins/native/demos/latency.c?

Jan

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

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Jan Kiszka]

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

Jan Kiszka wrote:
> Heikki Lindholm wrote:
> 
>>Jan Kiszka kirjoitti:
>>
>>
>>>Heikki Lindholm wrote:
>>>
>>>
>>>>Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but
>>>>not yet set last_task_used_math to NULL. As a result the tasks MSR_FP
>>>>will get set, although it should be cleared. If the task happens to hit
>>>>one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU
>>>>state might be saved to the task. The attached patch should fix this. I
>>>>couldn't try it on most recent Xenomai trunk, because latency wouldn't
>>>>build anymore. However, I see no reason it shouldn't work. All thee
>>>
>>>
>>>
>>>Is it still broken with latest revision 376? Philippe had merged the fix
>>>for my mess, and it worked at least for 2.6 on my box again.
>>
>>
>>I'd say this has been unchanged since the beginning (0.9?).
>>
> 
> 
> Then, what latency are we talking about? My last modifications went to
> src/testsuite/latency. Did you mean ksrc/skins/native/demos/latency.c?
> 

BTW, the latter compiles fine for me as well (in my case by providing
the full path to xeno-config via XENO_CONFIG to "make").

Jan

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

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

Jan Kiszka kirjoitti:
> Heikki Lindholm wrote:
> 
>>Jan Kiszka kirjoitti:
>>
>>
>>>Heikki Lindholm wrote:
>>>
>>>
>>>>Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but
>>>>not yet set last_task_used_math to NULL. As a result the tasks MSR_FP
>>>>will get set, although it should be cleared. If the task happens to hit
>>>>one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU
>>>>state might be saved to the task. The attached patch should fix this. I
>>>>couldn't try it on most recent Xenomai trunk, because latency wouldn't
>>>>build anymore. However, I see no reason it shouldn't work. All thee
>>>
>>>
>>>
>>>Is it still broken with latest revision 376? Philippe had merged the fix
>>>for my mess, and it worked at least for 2.6 on my box again.
>>
>>
>>I'd say this has been unchanged since the beginning (0.9?).
>>
> 
> 
> Then, what latency are we talking about? My last modifications went to
> src/testsuite/latency. Did you mean ksrc/skins/native/demos/latency.c?

Sorry, I misinterpreted in a hurry. I was (still) talking about the fpu 
;) It might be that the latency test didn't compile, because I didn't 
even enable the rtdm measuring device in kernel config or something(?). 
I'll try again.

-- Heikki Lindholm

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

Heikki Lindholm kirjoitti:
> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but 
> not yet set last_task_used_math to NULL. As a result the tasks MSR_FP 
> will get set, although it should be cleared. If the task happens to hit 
> one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU 
> state might be saved to the task. The attached patch should fix this. I 
> couldn't try it on most recent Xenomai trunk, because latency wouldn't 
> build anymore. However, I see no reason it shouldn't work. All thee 
> having trouble with X and Xenomai, give this a shot.

Additionally, there's also a Linux bug, which might cause corruption in 
2.6.14 PREEMPT:

http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=9f232a125bf86b0dae09f8ea4a0553535cf6b658

For solid performance that should also be applied on top of the I-pipe 
patch.

-- Heikki Lindholm

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

Jan Kiszka kirjoitti:
> Heikki Lindholm wrote:
> 
>>Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but
>>not yet set last_task_used_math to NULL. As a result the tasks MSR_FP
>>will get set, although it should be cleared. If the task happens to hit
>>one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU
>>state might be saved to the task. The attached patch should fix this. I
>>couldn't try it on most recent Xenomai trunk, because latency wouldn't
>>build anymore. However, I see no reason it shouldn't work. All thee
> 
> 
> Is it still broken with latest revision 376? Philippe had merged the fix
> for my mess, and it worked at least for 2.6 on my box again.

Just tried. At least svn r380 compiled and worked fine for me.

-- Heikki Lindholm

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Philippe Gerum]

Heikki Lindholm wrote:
> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but 
> not yet set last_task_used_math to NULL. As a result the tasks MSR_FP 
> will get set, although it should be cleared. If the task happens to hit 
> one of the codepaths that save FPU state if MSR_FP is set, the wrong FPU 
> state might be saved to the task. The attached patch should fix this. I 
> couldn't try it on most recent Xenomai trunk, because latency wouldn't 
> build anymore. However, I see no reason it shouldn't work. All thee 
> having trouble with X and Xenomai, give this a shot.
> 

Merged in r383, thanks.

> -- Heikki Lindholm
> 
> 
> ------------------------------------------------------------------------
> 
> diff -Nru xenomai/include/asm-powerpc/system.h xenomai-devel/include/asm-powerpc/system.h
> --- xenomai/include/asm-powerpc/system.h	2006-01-06 15:55:19.000000000 +0200
> +++ xenomai-devel/include/asm-powerpc/system.h	2006-01-06 16:44:53.000000000 +0200
> @@ -55,6 +55,7 @@
>      rthal_fpenv_t fpuenv  __attribute__ ((aligned (16)));
>      rthal_fpenv_t *fpup;	/* Pointer to the FPU backup area */
>      struct task_struct *user_fpu_owner;
> +    unsigned long user_fpu_owner_prev_msr;
>      /* Pointer the the FPU owner in userspace:
>         - NULL for RT K threads,
>         - last_task_used_math for Linux US threads (only current or NULL when MP)
> @@ -368,7 +369,10 @@
>          rthal_save_fpu(tcb->fpup);
>  
>          if(tcb->user_fpu_owner && tcb->user_fpu_owner->thread.regs)
> +            {
> +            tcb->user_fpu_owner_prev_msr = tcb->user_fpu_owner->thread.regs->msr;
>              tcb->user_fpu_owner->thread.regs->msr &= ~MSR_FP;
> +            }
>          }   
>  
>  #endif /* CONFIG_XENO_HW_FPU */
> @@ -383,7 +387,13 @@
>          {
>          rthal_restore_fpu(tcb->fpup);
>  
> -        if(tcb->user_fpu_owner && tcb->user_fpu_owner->thread.regs)
> +	/* Note: Only enable FP in MSR, if it was enabled when we saved the
> +	 * fpu state. We might have preempted Linux when it had disabled FP
> +	 * for the thread, but not yet set last_task_used_math to NULL 
> +	 */
> +        if(tcb->user_fpu_owner && 
> +			tcb->user_fpu_owner->thread.regs &&
> +			((tcb->user_fpu_owner_prev_msr & MSR_FP) != 0))
>              tcb->user_fpu_owner->thread.regs->msr |= MSR_FP;
>          }   
>  
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core


-- 

Philippe.

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

Philippe Gerum kirjoitti:
> Heikki Lindholm wrote:
> 
>> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, but 
>> not yet set last_task_used_math to NULL. As a result the tasks MSR_FP 
>> will get set, although it should be cleared. If the task happens to 
>> hit one of the codepaths that save FPU state if MSR_FP is set, the 
>> wrong FPU state might be saved to the task. The attached patch should 
>> fix this. I couldn't try it on most recent Xenomai trunk, because 
>> latency wouldn't build anymore. However, I see no reason it shouldn't 
>> work. All thee having trouble with X and Xenomai, give this a shot.
>>
> 
> Merged in r383, thanks.

On a related note, how do you think patches to Linux kernel that don't 
fit into I-pipe should be handled? Put into patches directory 
separately? There are now at least two instances for 2.6.14/ppc: the fpu 
bug that I already posted on this thread and the ppc64 UP building bug 
that effectively makes impossible to build I-pipe 2.6.14 for UP on ppc64.

-- Heikki Lindholm

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Philippe Gerum]

Heikki Lindholm wrote:
> Philippe Gerum kirjoitti:
> 
>> Heikki Lindholm wrote:
>>
>>> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, 
>>> but not yet set last_task_used_math to NULL. As a result the tasks 
>>> MSR_FP will get set, although it should be cleared. If the task 
>>> happens to hit one of the codepaths that save FPU state if MSR_FP is 
>>> set, the wrong FPU state might be saved to the task. The attached 
>>> patch should fix this. I couldn't try it on most recent Xenomai 
>>> trunk, because latency wouldn't build anymore. However, I see no 
>>> reason it shouldn't work. All thee having trouble with X and Xenomai, 
>>> give this a shot.
>>>
>>
>> Merged in r383, thanks.
> 
> 
> On a related note, how do you think patches to Linux kernel that don't 
> fit into I-pipe should be handled? Put into patches directory 
> separately? There are now at least two instances for 2.6.14/ppc: the fpu 
> bug that I already posted on this thread and the ppc64 UP building bug 
> that effectively makes impossible to build I-pipe 2.6.14 for UP on ppc64.
>

I'm going to put those patches on GNA, so that people can access to them 
easily. I'm going to add a fix for x86 too, which is needed at least on 
one of my boxen since 2.6.13 to make the PCI setup correct again.

Is this patch still the right one to fix the UP build on ppc64?
http://patchwork.ozlabs.org/linuxppc64/patchcontent?id=3178

> -- Heikki Lindholm
> 


-- 

Philippe.

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Heikki Lindholm]

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

Philippe Gerum kirjoitti:
> Heikki Lindholm wrote:
> 
>> Philippe Gerum kirjoitti:
>>
>>> Heikki Lindholm wrote:
>>>
>>>> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, 
>>>> but not yet set last_task_used_math to NULL. As a result the tasks 
>>>> MSR_FP will get set, although it should be cleared. If the task 
>>>> happens to hit one of the codepaths that save FPU state if MSR_FP is 
>>>> set, the wrong FPU state might be saved to the task. The attached 
>>>> patch should fix this. I couldn't try it on most recent Xenomai 
>>>> trunk, because latency wouldn't build anymore. However, I see no 
>>>> reason it shouldn't work. All thee having trouble with X and 
>>>> Xenomai, give this a shot.
>>>>
>>>
>>> Merged in r383, thanks.
>>
>>
>>
>> On a related note, how do you think patches to Linux kernel that don't 
>> fit into I-pipe should be handled? Put into patches directory 
>> separately? There are now at least two instances for 2.6.14/ppc: the 
>> fpu bug that I already posted on this thread and the ppc64 UP building 
>> bug that effectively makes impossible to build I-pipe 2.6.14 for UP on 
>> ppc64.
>>
> 
> I'm going to put those patches on GNA, so that people can access to them 
> easily. I'm going to add a fix for x86 too, which is needed at least on 
> one of my boxen since 2.6.13 to make the PCI setup correct again.
> 
> Is this patch still the right one to fix the UP build on ppc64?
> http://patchwork.ozlabs.org/linuxppc64/patchcontent?id=3178

Yep. That makes it build. But catenate it with the following and it'll 
boot, too ;)

-- Heikki Lindholm

[-- Attachment #2: 2.6.14-ppc64-UP-bootcpu.patch --]
[-- Type: text/plain, Size: 559 bytes --]

diff -Nru linux-2.6.14/arch/ppc64/kernel/prom.c linux-2.6.14-dev/arch/ppc64/kernel/prom.c
--- linux-2.6.14/arch/ppc64/kernel/prom.c	2005-10-28 03:02:08.000000000 +0300
+++ linux-2.6.14-dev/arch/ppc64/kernel/prom.c	2005-11-06 14:14:07.000000000 +0200
@@ -1019,6 +1019,7 @@
 		 */
 		boot_cpuid_phys = initial_boot_params->boot_cpuid_phys;
 		boot_cpuid = 0;
+		set_hard_smp_processor_id(boot_cpuid, boot_cpuid_phys);
 	} else {
 		/* Check if it's the boot-cpu, set it's hw index in paca now */
 		if (get_flat_dt_prop(node, "linux,boot-cpu", NULL) != NULL) {

Re: [Xenomai-core] [PATCH] Fix ppc fpu support

[Philippe Gerum]

Heikki Lindholm wrote:
> Philippe Gerum kirjoitti:
> 
>> Heikki Lindholm wrote:
>>
>>> Philippe Gerum kirjoitti:
>>>
>>>> Heikki Lindholm wrote:
>>>>
>>>>> Xenomai might preempt linux when linux has cleared a tasks MSR_FP, 
>>>>> but not yet set last_task_used_math to NULL. As a result the tasks 
>>>>> MSR_FP will get set, although it should be cleared. If the task 
>>>>> happens to hit one of the codepaths that save FPU state if MSR_FP 
>>>>> is set, the wrong FPU state might be saved to the task. The 
>>>>> attached patch should fix this. I couldn't try it on most recent 
>>>>> Xenomai trunk, because latency wouldn't build anymore. However, I 
>>>>> see no reason it shouldn't work. All thee having trouble with X and 
>>>>> Xenomai, give this a shot.
>>>>>
>>>>
>>>> Merged in r383, thanks.
>>>
>>>
>>>
>>>
>>> On a related note, how do you think patches to Linux kernel that 
>>> don't fit into I-pipe should be handled? Put into patches directory 
>>> separately? There are now at least two instances for 2.6.14/ppc: the 
>>> fpu bug that I already posted on this thread and the ppc64 UP 
>>> building bug that effectively makes impossible to build I-pipe 2.6.14 
>>> for UP on ppc64.
>>>
>>
>> I'm going to put those patches on GNA, so that people can access to 
>> them easily. I'm going to add a fix for x86 too, which is needed at 
>> least on one of my boxen since 2.6.13 to make the PCI setup correct 
>> again.
>>
>> Is this patch still the right one to fix the UP build on ppc64?
>> http://patchwork.ozlabs.org/linuxppc64/patchcontent?id=3178
> 
> 
> Yep. That makes it build. But catenate it with the following and it'll 
> boot, too ;)
>

Ah! Great release. Mm, do we need another one in order to use it on a 
console shell, or should we only telnet to the G5 and look at the syslog 
for bash output? :o>

> -- Heikki Lindholm
> 
> 
> ------------------------------------------------------------------------
> 
> diff -Nru linux-2.6.14/arch/ppc64/kernel/prom.c linux-2.6.14-dev/arch/ppc64/kernel/prom.c
> --- linux-2.6.14/arch/ppc64/kernel/prom.c	2005-10-28 03:02:08.000000000 +0300
> +++ linux-2.6.14-dev/arch/ppc64/kernel/prom.c	2005-11-06 14:14:07.000000000 +0200
> @@ -1019,6 +1019,7 @@
>  		 */
>  		boot_cpuid_phys = initial_boot_params->boot_cpuid_phys;
>  		boot_cpuid = 0;
> +		set_hard_smp_processor_id(boot_cpuid, boot_cpuid_phys);
>  	} else {
>  		/* Check if it's the boot-cpu, set it's hw index in paca now */
>  		if (get_flat_dt_prop(node, "linux,boot-cpu", NULL) != NULL) {


-- 

Philippe.