Re: [Minios-devel] [PATCH v3 26/43] arm64: implement the run_idle_thread

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 16/04/18 07:32, Huang Shijie wrote:
> The patch implements the run_idle_thread() for the idle thread
This patch is a really good example about why I keep suggesting to scrap 
arch/arm and write from start.
The commit message does not match the patch. You fix the code without 
explaining why. Why do you need to fix the code?
I am not going to fight against scrapping the code, I had enough with 
that. However, you should at least try to make that series easier to 
review. This likely means rewording most of your commit message to make 
clear the rationale of the change.
> Signed-off-by: Huang Shijie <shijie.huang@xxxxxxx>
> ---
>   arch/arm/sched.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/sched.c b/arch/arm/sched.c
> index a209513..e63ddd4 100644
> --- a/arch/arm/sched.c
> +++ b/arch/arm/sched.c
> @@ -39,8 +39,8 @@ struct thread* arch_create_thread(char *name, void 
> (*function)(void *),
>   
>   void run_idle_thread(void)
>   {
> -    __asm__ __volatile__ ("mov sp, %0; bx %1"::
> -            "r"(idle_thread->sp + 4 * CALLEE_SAVED_REGISTERS),
> +    __asm__ __volatile__ ("mov sp, %0; br %1"::
> +            "r"(idle_thread->sp + sizeof(unsigned long) * 
> CALLEE_SAVED_REGISTERS),
>               "r"(idle_thread->ip));
I was expecting to see no assembly code in the common directory with the 
new split.
>       /* Never arrive here! */
>   }
Cheers,

--
Julien Grall

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