ppc multlibs and BSP removal was Re: powerpc altivec support

Fri Feb 11 14:10:22 UTC 2005

Sergei... no insult intended to Ralf but I am sure you know the PowerPC 
details better than he does at this level of detail.  Could you submit a
patch for the  issues you comment on in this email?  If it is for a BSP
you can't test, I am happy if you will make a good faith attempt with a 
desk check and it compiles.

This method of making changes across targets and BSPs has worked for 
RTEMS in the past and generally is safe enough.

Thanks.

--joel

Sergei Organov wrote:
> Ralf Corsepius <ralf.corsepius at rtems.org> writes:
> 
>>On Thu, 2005-02-10 at 21:16 +0300, Sergei Organov wrote:
>>
>>
>>>Where exactly do you see more suspicious spots?
>>
>>rtems-cvs/trunk.
>>
>>
>>For example c/src/lib/libcpu/powerpc/ppc403/vectors/vectors.S
>>contains this near line 47:
>>
>>        /* Where this file will be loaded */
>>        .set    file_base, PPC_VECTOR_FILE_BASE
>>
>>        /* Offset to store reg 0 */
>>
>>        .set    IP_LINK, 0
>>/* PPC_ABI_EABI */
>>        .set    IP_0, (IP_LINK + 8)
> 
> [...]
> 
>>Note the hard-coded "8" below the PPC_ABI_EABI comment (I added several
>>such comments when removing POWEROPEN at places which had formerly been
>>"guarded" by PPC_ABI_POWEROPEN defines).
> 
> 
> Well, this "8" is not EABI-specific, i.e., it's the same for EABI and
> SYSV. It's offset of 'gpr0' field in CPU_Interrupt_frame structure
> defined in cpu.h.
> 
> 
> [...]
> 
> 
>>c/src/lib/libcpu/powerpc/ppc403/vectors/vectors.S:52:/* PPC_ABI_EABI */
>>c/src/lib/libcpu/powerpc/ppc403/vectors/vectors.S-53-	.set	IP_0, (IP_LINK + 8)
> 
> 
> It should include "ppc_offs.h" instead of defining it itself.
> 
> 
>>c/src/lib/libbsp/powerpc/ppcn_60x/vectors/vectors.S:53:/* PPC_ABI_EABI */
>>c/src/lib/libbsp/powerpc/ppcn_60x/vectors/vectors.S-54-    .set    IP_0, (IP_LINK + 8)
> 
> 
> Ditto.
> 
> 
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/rtems/score/ppc_offs.h:109:/* PPC_ABI_EABI */
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/rtems/score/ppc_offs.h-110-	.set	IP_0, (IP_LINK + 8)
> 
> 
> Ditto.
> 
> 
> [...]
> 
> 
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/rtems/score/ppc_offs.h:148:/* PPC_ABI_EABI */
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/rtems/score/ppc_offs.h-149-	.set	Default_r2, 16
> 
> 
> Not a problem, -- offset of Default_r2 field in the _CPU_IRQ_info
> structure in cpu.h. The field exists both for EABI and SYSV, though for
> SYSV it's not necessary but won't hurt.
> 
> 
>>--
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-336- *  _CPU_Install_interrupt_stack
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-337- */
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-338-
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-339-void _CPU_Install_interrupt_stack( void )
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-340-{
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c:341:/* PPC_ABI_EABI */
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-342-  _CPU_IRQ_info.Stack = _CPU_Interrupt_stack_high - 8;
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/cpu.c-343-}
> 
> 
> Well, in my version this is fixed:
> 
> void _CPU_Install_interrupt_stack( void )
> {
>   _CPU_IRQ_info.Stack = (void*)(((unsigned32)_CPU_Interrupt_stack_high
>     & ~(PPC_STACK_ALIGNMENT-1)) - CPU_MINIMUM_STACK_FRAME_SIZE);
>   *(unsigned32*)(_CPU_IRQ_info.Stack) = 0;
> }
> 
> Please notice that it also fixes possible alignment problem and writes 0
> to the first interrupt frame so that stack unwinding is capable to
> detect the first frame.
> 
> [...]
> 
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S-128-  /*
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S-129-   *  (*_ISR_Vector_table[ vector ])( vector );
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S-130-   */
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S:131:#if (PPC_ABI == PPC_ABI_SVR4 || PPC_ABI == PPC_ABI_EABI)
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S-132-	mtlr	r4
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S-133-	lwz	r2, Default_r2(r11)
> 
> 
> Loading r2 is useless for SYSV but won't hurt.
> 
> 
>>c/src/lib/libbsp/powerpc/support/old_exception_processing/irq_stub.S-134-	lwz	r13, Default_r13(r11)
> 
> 
> Well, new exception processing loads neither r2 nor r13 in a hope that
> all the code in the executable strictly adhere to the ABI (either SYSV
> or EABI). I believe this is unsafe assumption. I.e., some assembly code
> could easily temporarily change r13 or r2 and if interrupt happens
> while one of these registers is in inconsistent state, the new exception
> processing application will break as C code in interrupt handlers does
> assume these registers contain the correct values. Such a bug, if hit,
> will be very difficult to debug. I did debug similar problem on TMS320
> before, so I do know, don't wish anybody the same trouble.
> 
> 
> 

-- 
Joel Sherrill, Ph.D.             Director of Research & Development
joel at OARcorp.com                 On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
    Support Available             (256) 722-9985