[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.