[PATCH] ISR not causing an DSR in some rare conditions

[Sergei Organov <osv at javad dot com>]

Jay Foster <jay@systech.com> writes:
> Revised patch attached.

The patch looks OK to me, but I don't see it applied to the public CVS
:(

-- Sergei.

>
>
> -----Original Message-----
> From: Sergei Organov [mailto:osv@javad.com]
> Sent: Monday, January 23, 2006 2:12 AM
> To: ecos-patches@sources.redhat.com
> Subject: Re: [ECOS] Re: ISR not causing an DSR in some rare conditions
>
>
> Jay Foster <jay@systech.com> writes:
>
>> Attached is a patch for the first problem described below.  This is
>> essentially Sergei Organov's suggested change put into patch form.
>
> The original suggestion had:
>
>        sub     sp,sp,#(ARMREG_SIZE - armreg_lr - 4) // skip svc_sp, svc_lr,
> vector, cpsr, and pc
>        stmfd   sp!,{ip,lr}
>
> and your patch has:
>
>        sub     sp,sp,#(ARMREG_SIZE - armreg_pc) // skip svc_sp, svc_lr,
> vector, cpsr, and pc
>        stmfd   sp!,{ip,lr}
>
> While they are exactly the same from the point of view of assembler, for
> humans the first variant emphasized that lr will be put at armreg_lr
> offset in the context structure. The fact that 'armreg_pc' happens to be
> equal to 'armreg_lr - 4' does not mean one should use it instead.
>
> Could you please preserve the former variant in your patch.
>
> -- Sergei.
>
>
> Index: ecos/packages/hal/arm/arch/current/ChangeLog
> ===================================================================
> RCS file: /cvs/ecos/ecos/packages/hal/arm/arch/current/ChangeLog,v
> retrieving revision 1.104
> diff -u -5 -p -r1.104 ChangeLog
> --- ecos/packages/hal/arm/arch/current/ChangeLog	21 Apr 2005 18:17:54 -0000	1.104
> +++ ecos/packages/hal/arm/arch/current/ChangeLog	23 Jan 2006 19:31:35 -0000
> @@ -1,5 +1,10 @@
> +2006-01-18  Jay Foster    <jay@systech.com>
> +
> +	* src/context.S (hal_thread_switch_context): Close race condition 
> +	that could cause corruption of the sp or lr registers.
> +
>  2005-04-21  Ian Campbell  <icampbell@arcom.com>
>  
>  	* src/redboot_linux_exec.c: Added -t option which takes the
>  	physical address to copy to. Very useful for booting non-Linux
>  	operating systems.
> Index: ecos/packages/hal/arm/arch/current/src/context.S
> ===================================================================
> RCS file: /cvs/ecos/ecos/packages/hal/arm/arch/current/src/context.S,v
> retrieving revision 1.9
> diff -u -5 -p -r1.9 context.S
> --- ecos/packages/hal/arm/arch/current/src/context.S	23 May 2002 23:01:42 -0000	1.9
> +++ ecos/packages/hal/arm/arch/current/src/context.S	23 Jan 2006 19:31:36 -0000
> @@ -90,13 +90,13 @@ _name_: 
>  
>  // Note: this is a little wasteful since r0..r3 don't need to be saved.
>  // They are saved here though so that the information can match the HAL_SavedRegisters
>          
>  FUNC_START_ARM(hal_thread_switch_context, r2)
> -        sub     ip,sp,#20               // skip svc_sp, svc_lr, vector, cpsr, and pc
> -        stmfd   ip!,{sp,lr}
> -        mov     sp,ip
> +        mov     ip,sp
> +        sub     sp,sp,#(ARMREG_SIZE - armreg_lr - 4) // skip svc_sp, svc_lr, vector, cpsr, and pc
> +        stmfd   sp!,{ip,lr}
>          stmfd   sp!,{r0-r10,fp,ip}
>          mrs     r2,cpsr
>          str     r2,[sp,#armreg_cpsr]
>          str     sp,[r1]                 // return new stack pointer
>  #ifdef __thumb__