Re: [Qemu-devel] [PATCH]: sh4 delay slot code update

[Magnus Damm]

Hi Paul,

Thanks for your comments.

On Nov 28, 2007 9:49 PM, Paul Mundt <address@hidden> wrote:
> On Wed, Nov 28, 2007 at 06:54:20PM +0900, Magnus Damm wrote:
> > +#define DELAY_SLOT_TRUE        (1 << 2)
> > +#define DELAY_SLOT_CLEARME     (1 << 3)
> > +/* The dynamic value of the DELAY_SLOT_TRUE flag determines whether the 
> > jump
> > + * after the delay slot should be taken or not. It is calculated from SR_T.
> > + *
> > + * It is unclear if it is permitted to modify the SR_T flag in a delay 
> > slot.
> > + * The use of DELAY_SLOT_TRUE flag makes us accept such SR_T modification.
> > + */
>
> Nesting a 'tst' in a delay slot is certainly valid, and GCC correctly
> treats it as a slottable instruction. If you're in doubt as to whether an
> opcode can be placed in a delay slot or not, the machine descriptor is a
> good way of sorting things out. The only restrictions I know of things
> that cause changes to PC, most of the system instructions (like trapa and
> ldtlb), and so on. There are of course cases where an instruction itself
> is slottable which may perform illegal behaviour via PC modification or
> so on, and we do have an exception for trapping that sort of abuse.

I was mainly wondering if I really needed to save the state of SR_T,
but I assumed so. So the code should be correct. And yes, I'm sure
there are quite a few slottable instructions with interesting side
effects, but that's a separate issue.

> You can see an example in arch/sh/kernel/entry-common.S:
>
>         syscall_exit_work:
>                 ! r0: current_thread_info->flags
>                 ! r8: current_thread_info
>                 tst     #_TIF_SYSCALL_TRACE | _TIF_SINGLESTEP | 
> _TIF_SYSCALL_AUDIT, r0
>                 bt/s    work_pending
>                  tst    #_TIF_NEED_RESCHED, r0
>
>         ....
>         work_pending:
>                 ! r0: current_thread_info->flags
>                 ! r8: current_thread_info
>                 ! t:  result of "tst    #_TIF_NEED_RESCHED, r0"
>                 bf/s    work_resched
>                  tst    #(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK), r0
>
>         ....
>
> This sort of access is not a particularly rare workload. Presumably you'd hit
> this under system emulation at the very least.

Yeah, that's a pretty good example that shows that I need to save the
SR_T state before executing the delay slot instruction. Thanks for
pointing out that code.

/ magnus