From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Garnett To: ecos-discuss@sources.redhat.com Subject: Re: [ECOS] PowerPC HAL questions. Date: Wed, 05 Sep 2001 00:10:00 -0000 Message-ID: References: <87k8gz68p0.fsf@osv.javad.ru> X-SW-Source: 2001-09/msg00083.html Message-ID: <20010905001000.uu6j8vovRFCWSmgMTxKqh35D0FtPvwVWxVnZqgaHl94@z> Sergei Organov writes: > 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? Probably. Several people have worked on this code, so it is mostly down to individual style. Since at present the lack of these syncs does not cause any real problems there is little impetus to change it. > > 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. This might be a good thing to do. > > Q.3. What's the reason to restore only MSR EE bit in context switches as > opposed to restoring of entire MSR? Because we only want the interrupt enable state to be per-thread. There are other things in the MSR that can get changed on a global basis and which we want to stay unchanged across context switches. > > 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'. > Enabing interrupts while running DSRs is the main reason for doing this. Since some DSR can run for a long time, they would affect interrupt latency. If we enabled interrupts during DSR processing while on a thread stack, we have the possibility of taking nested interrupts, which would either overflow the stack, or require the stack to be extra large. So, to avoid this we transfer to the interrupt stack for DSR processing. A useful side-effect of this is that we can keep the thread stacks small. This is also the reason for preserving the EE bit across context switches - interrupts are disabled in threads that already have an interrupt context stacked and interrupts are not re-enabled until we have trasferred to another thread that is capable of taking an interrupt. Since we cannot do any of this if there is no interrupt stack, then all of this is conditional on CYGIMP_HAL_COMMON_INTERRUPTS_USE_INTERRUPT_STACK. We really do not expect anyone to disable this, the advantages of having a separate interrupt stack are far greater than the alternatives. However, if it is disabled, then the system will still work, although with reduced interrupt latency. > 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? > Not as far as I can see. I would probably have written the second variant too. This is probably just an example of code being cut-and-pasted. > 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). > There are several reasons for the code being as it is. The first is to make the exception decode code in the exception_vector macro as fast as possible by not touching memory more than the once to get the VSR table entry. We cannot assume that the SP is necessarily valid at this point. Although the default VSRs make this assumption, VSRs for handling things like TLB miss should not. If we stashed some data on the stack in this macro, it is very possible that the default VSRs would have to pop it again to set up their own saved states. We also want to provide the option of fast interrupt and exception handling to applications by allowing them to install their own VSRs. We do this by entering the VSR with the minimum of change to the machine state that we can achieve. > 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 > Just an intended optimization that never got finished. -- Nick Garnett Cygnus Solutions, a Red Hat Company Cambridge, UK