ppc multlibs and BSP removal was Re: powerpc altivec support

[Sergei Organov]

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.



-- 
Sergei.