From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sergei Organov <osv@javad.ru>
To: ecos-discuss@sourceware.cygnus.com
Subject: [ECOS] PowerPC HAL questions.
Date: Fri, 12 May 2000 09:21:00 -0000
Message-id: <87k8gz68p0.fsf@osv.javad.ru>
X-SW-Source: 2000-05/msg00105.html

Hello,

While developing FP support for the PowerPC HAL I've got a few questions. Some
of them are actually suggestions for possible improvements.

Q.1. All the writing to the MSR in the assembly code are surrounded by the
'sync' instructions. AFAIK it's because of bug in some PPC chips. However, the
exported macros 'HAL_DISABLE_INTERRUPTS', 'HAL_ENABLE_INTERRUPTS', and
'HAL_RESTORE_INTERRUPTS' don't surround 'mtmsr' instruction by 'sync'
instructions. Is it a bug?

Q.2. Is it worth to make surrounding of 'mtmsr' instruction by 'sync'
instructions configurable? I mean providing ability for processor variants to
change this. It could be more important after addition of "on demand" FPU
context switch support because it requires additional tweaking of the FP bit
in MSR.

Q.3. What's the reason to restore only MSR EE bit in context switches as
opposed to restoring of entire MSR?

Q.4. What's the reason to enable interrupts in the
'hal_interrupt_stack_call_pending_DSRs' routine? Note that when
CYGIMP_HAL_COMMON_INTERRUPTS_USE_INTERRUPT_STACK isn't defined, this routine
isn't invoked, and interrupts thus stay as-is before call to the
'Cyg_Interrupt::call_pending_DSRs_inner'.

Q.5. The following macro

	.macro hal_cpu_int_enable
        mfmsr   r0
        ori     r3,r3,0x8000
        rlwimi  r0,r3,0,16,16
        sync
        mtmsr   r0
        sync
	.endm

could be rewritten as

	.macro hal_cpu_int_enable
        mfmsr   r0
        ori     r0,0x8000
        sync
        mtmsr   r0
        sync
	.endm

that is 1 instruction less. Is there something wrong about the latter
implementation?

Similar question about

	.macro hal_cpu_int_disable
        mfmsr   r0
        li      r3,0
        rlwimi  r0,r3,0,16,16
        sync
        mtmsr   r0
        sync
	.endm

and

	.macro hal_cpu_int_disable
        mfmsr   r0
        rlwinm  r0,r0,0,18,17
        sync
        mtmsr   r0
        sync
	.endm

Q.6. The common level-0 code of VSRs uses SPRG1-3 to save work registers, then
the SPRG1-3 are moved back to GPRs and are stored on the stack by the default
VSRs. Isn't it better to store work registers directly on the stack and use
SPRGs for some global variables such as address of 'hal_interrupt_handlers',
address of 'hal_interrupt_data', etc.? It will also allow to save SRR0 and
SRR1, and then set MSR RI as soon as possible. This could be critical for some
applications (e.g., MPC509 goes to the check-stop state if another exception
occurs when RI bit isn't set, processor hangs, and even internal watchdog
doesn't work. The exception could occur due to spikes on the bus while testing 
by applying high voltage to the ground of the device).

Related issue. In the startup code:

        # set up stack
        lwi     sp,__interrupt_stack
        mtspr   SPRG0,sp        # save in sprg0 for later use

SPRG0 isn't then used in the interrupts: the __interrupt_stack is loaded by
'lwi' (that is actually 2 instructions) instead:

#ifdef CYGIMP_HAL_COMMON_INTERRUPTS_USE_INTERRUPT_STACK
        lwi     r3,__interrupt_stack            # stack top


BR,
Sergei Organov.