Re: [Minios-devel] [PATCH 25/40] arm64: set the stack for the arm_start_thread

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Shijie,

On 07/11/17 08:25, Huang Shijie wrote:
> On Mon, Nov 06, 2017 at 02:39:30PM +0000, Julien Grall wrote:
> > Hi Shijie,
> >
> > On 03/11/17 03:12, Huang Shijie wrote:
> > > When a thread (which is picked by the scheduler) runs at the first time,
> > > it needs a stack to store the data.
> > >
> > > This patch set the stack for the arm_start_thread.
> > >
> > > Change-Id: I4cfa18b199cb920e9ebe19bbbe2cf508264b0ba8
> > > Jira: ENTOS-247
> > > Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> > > ---
> > >    arch/arm/sched.c | 6 ++++++
> > >    1 file changed, 6 insertions(+)
> > >
> > > diff --git a/arch/arm/sched.c b/arch/arm/sched.c
> > > index 448fd3e..f691d7d 100644
> > > --- a/arch/arm/sched.c
> > > +++ b/arch/arm/sched.c
> > > @@ -32,6 +32,12 @@ struct thread* arch_create_thread(char *name, void 
> > > (*function)(void *),
> > >         * for the initial switch. */
> > >        thread->sp = (unsigned long) sp - sizeof(unsigned long) * 
> > > CALLEE_SAVED_REGISTERS;
> > > +    /*
> > > +     * Please refer to the stack layout in the comment for 
> > > __arch_switch_threads.
> > > +     * We need to set the sp register for arm_start_thread when a thread runs
> > > +     * at the first time.
> > > +     */
> > > +    *(--sp) = thread->sp;
> > But this is common code between Arm64 and Arm32. How come you don't update
> > arm32?
> >
> > Why do you need to save sp on Arm64 and not Arm32?
> The arm32 also need to save sp.. this is bug for arm32..
How is it a bug given you said you were unable to test Arm32 guest? At 
the moment, it looks like a bit random...
In any case, if it is a real bug. Then the commit message/title should 
be reflecting that with proper justification.
Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/cgi-bin/mailman/listinfo/minios-devel