> -----Original Message----- > From: Philippe Gerum [mailto:[email protected]] > Sent: Monday, October 04, 2010 5:12 AM > To: Steve Deiters > Cc: Xenomai Core ([email protected]); adeos-main > Subject: Re: [Adeos-main] enable_kernel_fp broken with IPIPE on PowerPC > > On Fri, 2010-10-01 at 19:51 +0000, Steve Deiters wrote: > > I'm getting a thread crash where an unaligned floating point access > occurs. I tracked the cause down to enable_kernel_fp within the > fix_alignment routine. The enable_kernel_fp routine is as follows: > > > > void enable_kernel_fp(void) > > { > > unsigned long flags; > > > > WARN_ON(preemptible()); > > > > local_irq_save_hw_cond(flags); > > > > #ifdef CONFIG_SMP > > if (current->thread.regs && (current->thread.regs->msr & > MSR_FP)) > > giveup_fpu(current); > > else > > giveup_fpu(NULL); /* just enables FP for kernel */ > > #else > > giveup_fpu(last_task_used_math); > > #endif /* CONFIG_SMP */ > > local_irq_restore_hw_cond(flags); > > } > > > > > > > > The local_irq_save_hw_cond saves the old MSR value in flags. When this > value is restored with local_irq_restore_hw_cond it loses the MSR[FP] bit > that was set in giveup_fpu. If the MSR[FP] was not previously set before > it saved the flags, I get a FPU exception a bit later in the alignment > handling. > > > > > > As a quick fix I changed the line to restore to > > > > local_irq_restore_hw_cond(flags|MSR_FP); > > > > > > I'm not sure this is a correct fix. I'm don't know where else there > might be code that is modifying the MSR in a similar fashion. It seems > any such case would be broken. > > > > I'm using the ipipe version 2.10-03 patch that was bundled with Xenomai > 2.5.4 on Linux 2.6.33.5. I noticed that this is still the same though in > the 2.11-00 ipipe patch. > > > > Actually, giveup_fpu already handles the interrupt state properly, so > the protection code in enable_kernel_fp is buggy and useless as well. > I did not see any other spot where calling assembly code which may touch > the MSR would conflict with interrupt protection in the caller. Could > you try this patch instead? > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index e4eaca4..3743b27 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -98,12 +98,8 @@ void flush_fp_to_thread(struct task_struct *tsk) > > void enable_kernel_fp(void) > { > - unsigned long flags; > - > WARN_ON(preemptible()); > > - local_irq_save_hw_cond(flags); > - > #ifdef CONFIG_SMP > if (current->thread.regs && (current->thread.regs->msr & MSR_FP)) > giveup_fpu(current); > @@ -112,7 +108,6 @@ void enable_kernel_fp(void) > #else > giveup_fpu(last_task_used_math); > #endif /* CONFIG_SMP */ > - local_irq_restore_hw_cond(flags); > } > EXPORT_SYMBOL(enable_kernel_fp); >
This also works for me. I will use this for now. Thanks. _______________________________________________ Adeos-main mailing list [email protected] https://mail.gna.org/listinfo/adeos-main
