diff mbox series

[Xen-devel,09/17] xen/arm64: head: Improve coding style and document cpu_init()

Message ID 20190610193215.23704-10-julien.grall@arm.com
State Superseded
Headers show
Series xen/arm64: Rework head.S to make it more compliant with the Arm Arm | expand

Commit Message

Julien Grall June 10, 2019, 7:32 p.m. UTC
Adjust the coding style used in the comments within cpu_init(). Take the
opportunity to alter the early print to match the function name.

Lastly, document the behavior and the main registers usage within the
function.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/head.S | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Stefano Stabellini June 26, 2019, 1:01 a.m. UTC | #1
On Mon, 10 Jun 2019, Julien Grall wrote:
> Adjust the coding style used in the comments within cpu_init(). Take the
> opportunity to alter the early print to match the function name.
> 
> Lastly, document the behavior and the main registers usage within the
> function.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/arm64/head.S | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index 6aa3148192..ee0024173e 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -396,19 +396,26 @@ skip_bss:
>          ret
>  ENDPROC(zero_bss)
>  
> +/*
> + * Initialize the processor for turning the MMU on.
> + *
> + * Clobbers x0 - x4

Shouldn't it be x0 - x3?

The rest looks fine.


> + */
>  cpu_init:
> -        PRINT("- Setting up control registers -\r\n")
> +        PRINT("- Initialize CPU -\r\n")
>  
>          /* Set up memory attribute type tables */
>          ldr   x0, =MAIRVAL
>          msr   mair_el2, x0
>  
> -        /* Set up TCR_EL2:
> +        /*
> +         * Set up TCR_EL2:
>           * PS -- Based on ID_AA64MMFR0_EL1.PARange
>           * Top byte is used
>           * PT walks use Inner-Shareable accesses,
>           * PT walks are write-back, write-allocate in both cache levels,
> -         * 48-bit virtual address space goes through this table. */
> +         * 48-bit virtual address space goes through this table.
> +         */
>          ldr   x0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(64-48))
>          /* ID_AA64MMFR0_EL1[3:0] (PARange) corresponds to TCR_EL2[18:16] (PS) */
>          mrs   x1, ID_AA64MMFR0_EL1
> @@ -427,9 +434,11 @@ cpu_init:
>          ldr   x0, =(HSCTLR_BASE)
>          msr   SCTLR_EL2, x0
>  
> -        /* Ensure that any exceptions encountered at EL2
> +        /*
> +         * Ensure that any exceptions encountered at EL2
>           * are handled using the EL2 stack pointer, rather
> -         * than SP_EL0. */
> +         * than SP_EL0.
> +         */
>          msr spsel, #1
>          ret
>  ENDPROC(cpu_init)
> -- 
> 2.11.0
>
Julien Grall June 26, 2019, 10:34 a.m. UTC | #2
Hi Stefano,

On 26/06/2019 02:01, Stefano Stabellini wrote:
> On Mon, 10 Jun 2019, Julien Grall wrote:
>> Adjust the coding style used in the comments within cpu_init(). Take the
>> opportunity to alter the early print to match the function name.
>>
>> Lastly, document the behavior and the main registers usage within the
>> function.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>   xen/arch/arm/arm64/head.S | 19 ++++++++++++++-----
>>   1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>> index 6aa3148192..ee0024173e 100644
>> --- a/xen/arch/arm/arm64/head.S
>> +++ b/xen/arch/arm/arm64/head.S
>> @@ -396,19 +396,26 @@ skip_bss:
>>           ret
>>   ENDPROC(zero_bss)
>>   
>> +/*
>> + * Initialize the processor for turning the MMU on.
>> + *
>> + * Clobbers x0 - x4
> 
> Shouldn't it be x0 - x3?

Yes it should be. I will update the comment.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index 6aa3148192..ee0024173e 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -396,19 +396,26 @@  skip_bss:
         ret
 ENDPROC(zero_bss)
 
+/*
+ * Initialize the processor for turning the MMU on.
+ *
+ * Clobbers x0 - x4
+ */
 cpu_init:
-        PRINT("- Setting up control registers -\r\n")
+        PRINT("- Initialize CPU -\r\n")
 
         /* Set up memory attribute type tables */
         ldr   x0, =MAIRVAL
         msr   mair_el2, x0
 
-        /* Set up TCR_EL2:
+        /*
+         * Set up TCR_EL2:
          * PS -- Based on ID_AA64MMFR0_EL1.PARange
          * Top byte is used
          * PT walks use Inner-Shareable accesses,
          * PT walks are write-back, write-allocate in both cache levels,
-         * 48-bit virtual address space goes through this table. */
+         * 48-bit virtual address space goes through this table.
+         */
         ldr   x0, =(TCR_RES1|TCR_SH0_IS|TCR_ORGN0_WBWA|TCR_IRGN0_WBWA|TCR_T0SZ(64-48))
         /* ID_AA64MMFR0_EL1[3:0] (PARange) corresponds to TCR_EL2[18:16] (PS) */
         mrs   x1, ID_AA64MMFR0_EL1
@@ -427,9 +434,11 @@  cpu_init:
         ldr   x0, =(HSCTLR_BASE)
         msr   SCTLR_EL2, x0
 
-        /* Ensure that any exceptions encountered at EL2
+        /*
+         * Ensure that any exceptions encountered at EL2
          * are handled using the EL2 stack pointer, rather
-         * than SP_EL0. */
+         * than SP_EL0.
+         */
         msr spsel, #1
         ret
 ENDPROC(cpu_init)