Re: [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs

From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>
Cc: xen-devel@lists.xenproject.org, Oleksandr_Tyshchenko@epam.com,
	andrii_anisov@epam.com, andre.przywara@arm.com
Subject: Re: [Xen-devel] [PATCH 06/17] xen/arm64: head: Introduce distinct paths for the boot CPU and secondary CPUs
Date: Wed, 26 Jun 2019 10:14:26 +0100	[thread overview]
Message-ID: <255dc067-e45f-419a-5e0f-1f64954eda49@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1906251727410.5851@sstabellini-ThinkPad-T480s>

Hi Stefano,

On 26/06/2019 02:00, Stefano Stabellini wrote:
> On Mon, 10 Jun 2019, Julien Grall wrote:
>> The boot code is currently quite difficult to go through because of the
>> lack of documentation and a number of indirection to avoid executing
>> some path in either the boot CPU or secondary CPUs.
>>
>> In an attempt to make the boot code easier to follow, each parts of the
>> boot are now in separate functions. Furthermore, the paths for the boot
>> CPU and secondary CPUs are now distincted and for now will call each

I notice a few typo in my commit message:

s/distincted/distinct/

>> functions.
>>
>> Follow-ups will remove unecessary calls and do further improvement

s/unecessary/unnecessary/

>> +launch:
>>           PRINT("- Ready -\r\n")
>>   
>>           /* The boot CPU should go straight into C now */
>> @@ -594,7 +635,6 @@ paging:
>>           dsb   sy                     /* Ensure completion of TLB flush */
>>           isb
>>   
>> -launch:
> 
> Just below PRINT("- Ready -\r\n"), there is still a:
> 
>    cbz   x22, launch
> 
> moving the launch label up it looks like it will cause an infinite loop?

Urgh. this line is dropped in a later patch, so the issue only would happen 
during bisection.

I will update the code to avoid the infinite loop here.

> 
> Everything else looks good, and I like the reorg of the code.

Thank you! I will rework the arm32 code the same way then :).

> 
> 
>>           ldr   x0, =init_data
>>           add   x0, x0, #INITINFO_stack /* Find the boot-time stack */
>>           ldr   x0, [x0]
>> @@ -609,6 +649,7 @@ launch:
>>           b     start_xen              /* and disappear into the land of C */
>>   1:
>>           b     start_secondary        /* (to the appropriate entry point) */
>> +ENDPROC(launch)
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel