[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: ppc multlibs and BSP removal was Re: powerpc altivec support




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@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@OARcorp.com                 On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
   Support Available             (256) 722-9985