Re: [Xen-devel] [PATCH ARM v6 09/14] mini-os: arm: scheduling

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-07-28 at 12:20 +0100, Thomas Leonard wrote:
> On 28 July 2014 11:53, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Wed, 2014-07-16 at 12:07 +0100, Thomas Leonard wrote:
> >> Based on an initial patch by Karim Raslan.
> >>
> >> Signed-off-by: Karim Allah Ahmed <karim.allah.ahmed@xxxxxxxxx>
> >> Signed-off-by: Thomas Leonard <talex5@xxxxxxxxx>
> >> ---
> >>  extras/mini-os/arch/arm/arm32.S | 22 ++++++++++++++++++++++
> >>  extras/mini-os/arch/arm/sched.c | 38 
> >> ++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 60 insertions(+)
> >>  create mode 100644 extras/mini-os/arch/arm/sched.c
> >>
> >> diff --git a/extras/mini-os/arch/arm/arm32.S 
> >> b/extras/mini-os/arch/arm/arm32.S
> >> index 88efe24..56429b1 100644
> >> --- a/extras/mini-os/arch/arm/arm32.S
> >> +++ b/extras/mini-os/arch/arm/arm32.S
> >> @@ -204,8 +204,30 @@ irq_handler:
> >>  IRQ_handler:
> >>       .long   0x0
> >>
> >> +
> >> +.globl __arch_switch_threads
> >> +@ => r0 = &prev->sp
> >> +@    r1 = &next->sp
> >> +@ <= returns to next thread's saved return address
> >> +__arch_switch_threads:
> >> +     stmia   r0, {sp, lr}    @ Store current sp and ip to prev's struct 
> >> thread
> >> +     str     fp, [sp, #-4]   @ Store fp on the old stack
> >> +
> >> +     ldmia   r1, {sp, lr}    @ Load new sp, ip from next's struct thread
> >> +     ldr     fp, [sp, #-4]   @ Restore fp from the stack
> >> +
> >> +     mov     pc, lr
> >
> > I'm a bit confused by the lack of saving of the GPRs here. Shouldn't at
> > least the caller saved registers (r4..r11) be saved/restored here?
> 
> If they were caller saved then sched.c would save them itself before
> calling switch_threads. But the AAPCS says:
> 
> "A subroutine must preserve the contents of the registers r4-r8, r10,
> r11 and SP (and r9 in PCS variants that designate r9 as v6)."
> 
> So I think these are callee-saved and we should save them here.

I had a typo in my register/AAPCS cheatsheet but not in my head, so I
was kind of right for the wrong reason ;-). r4..r11 are indeed *callee*
saved.

Normally on ctxt switch you would save and restore everything to prevent
information leaks but since mini-os processes^Wthreads are mutually
trusting it doesn't seem entirely necessary here (but it does save
remembering which registers are which ;-)).

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel