[Xen-devel,2/2] xen/arm: lpae: Fix comments coding style

Message ID 20170615203057.755-3-julien.grall@arm.com
State Superseded
Headers show
Series
  • xen/arm: Move LPAE definition in a separate header.
Related show

Commit Message

Julien Grall June 15, 2017, 8:30 p.m.
Also adding one missing full stop.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/lpae.h | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

Comments

Andrew Cooper June 15, 2017, 10:19 p.m. | #1
On 15/06/2017 21:30, Julien Grall wrote:
> Also adding one missing full stop.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/include/asm-arm/lpae.h | 45 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
> index 1e6a68926e..6244240ca0 100644
> --- a/xen/include/asm-arm/lpae.h
> +++ b/xen/include/asm-arm/lpae.h
> @@ -3,10 +3,12 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -/* WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
> +/*
> + * WARNING!  Unlike the Intel pagetable code, where l1 is the lowest

As your changing the comment, it would be more correct to say "Unlike
the x86 pagetable code".  L1 through L4 is only the Xen naming scheme;
neither Intel nor AMD use it in their manuals.

~Andrew
Julien Grall June 16, 2017, 7:43 a.m. | #2
Hi Andrew,

On 15/06/2017 23:19, Andrew Cooper wrote:
> On 15/06/2017 21:30, Julien Grall wrote:
>> Also adding one missing full stop.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>  xen/include/asm-arm/lpae.h | 45 ++++++++++++++++++++++++++++++---------------
>>  1 file changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
>> index 1e6a68926e..6244240ca0 100644
>> --- a/xen/include/asm-arm/lpae.h
>> +++ b/xen/include/asm-arm/lpae.h
>> @@ -3,10 +3,12 @@
>>
>>  #ifndef __ASSEMBLY__
>>
>> -/* WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
>> +/*
>> + * WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
>
> As your changing the comment, it would be more correct to say "Unlike
> the x86 pagetable code".  L1 through L4 is only the Xen naming scheme;
> neither Intel nor AMD use it in their manuals.

I can do that.

Cheers,
Stefano Stabellini June 16, 2017, 8:51 p.m. | #3
On Thu, 15 Jun 2017, Julien Grall wrote:
> Also adding one missing full stop.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>

>  xen/include/asm-arm/lpae.h | 45 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 30 insertions(+), 15 deletions(-)
> 
> diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
> index 1e6a68926e..6244240ca0 100644
> --- a/xen/include/asm-arm/lpae.h
> +++ b/xen/include/asm-arm/lpae.h
> @@ -3,10 +3,12 @@
>  
>  #ifndef __ASSEMBLY__
>  
> -/* WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
> +/*
> + * WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
>   * level and l4 is the root of the trie, the ARM pagetables follow ARM's
>   * documentation: the levels are called first, second &c in the order
> - * that the MMU walks them (i.e. "first" is the root of the trie). */
> + * that the MMU walks them (i.e. "first" is the root of the trie).
> + */
>  
>  /******************************************************************************
>   * ARMv7-A LPAE pagetables: 3-level trie, mapping 40-bit input to
> @@ -17,15 +19,18 @@
>   * different place from those in leaf nodes seems to be to allow linear
>   * pagetable tricks.  If we're not doing that then the set of permission
>   * bits that's not in use in a given node type can be used as
> - * extra software-defined bits. */
> + * extra software-defined bits.
> + */
>  
>  typedef struct __packed {
>      /* These are used in all kinds of entry. */
>      unsigned long valid:1;      /* Valid mapping */
>      unsigned long table:1;      /* == 1 in 4k map entries too */
>  
> -    /* These ten bits are only used in Block entries and are ignored
> -     * in Table entries. */
> +    /*
> +     * These ten bits are only used in Block entries and are ignored
> +     * in Table entries.
> +     */
>      unsigned long ai:3;         /* Attribute Index */
>      unsigned long ns:1;         /* Not-Secure */
>      unsigned long user:1;       /* User-visible */
> @@ -38,30 +43,38 @@ typedef struct __packed {
>      unsigned long long base:36; /* Base address of block or next table */
>      unsigned long sbz:4;        /* Must be zero */
>  
> -    /* These seven bits are only used in Block entries and are ignored
> -     * in Table entries. */
> +    /*
> +     * These seven bits are only used in Block entries and are ignored
> +     * in Table entries.
> +     */
>      unsigned long contig:1;     /* In a block of 16 contiguous entries */
>      unsigned long pxn:1;        /* Privileged-XN */
>      unsigned long xn:1;         /* eXecute-Never */
>      unsigned long avail:4;      /* Ignored by hardware */
>  
> -    /* These 5 bits are only used in Table entries and are ignored in
> -     * Block entries */
> +    /*
> +     * These 5 bits are only used in Table entries and are ignored in
> +     * Block entries.
> +     */
>      unsigned long pxnt:1;       /* Privileged-XN */
>      unsigned long xnt:1;        /* eXecute-Never */
>      unsigned long apt:2;        /* Access Permissions */
>      unsigned long nst:1;        /* Not-Secure */
>  } lpae_pt_t;
>  
> -/* The p2m tables have almost the same layout, but some of the permission
> - * and cache-control bits are laid out differently (or missing) */
> +/*
> + * The p2m tables have almost the same layout, but some of the permission
> + * and cache-control bits are laid out differently (or missing).
> + */
>  typedef struct __packed {
>      /* These are used in all kinds of entry. */
>      unsigned long valid:1;      /* Valid mapping */
>      unsigned long table:1;      /* == 1 in 4k map entries too */
>  
> -    /* These ten bits are only used in Block entries and are ignored
> -     * in Table entries. */
> +    /*
> +     * These ten bits are only used in Block entries and are ignored
> +     * in Table entries.
> +     */
>      unsigned long mattr:4;      /* Memory Attributes */
>      unsigned long read:1;       /* Read access */
>      unsigned long write:1;      /* Write access */
> @@ -73,8 +86,10 @@ typedef struct __packed {
>      unsigned long long base:36; /* Base address of block or next table */
>      unsigned long sbz3:4;
>  
> -    /* These seven bits are only used in Block entries and are ignored
> -     * in Table entries. */
> +    /*
> +     * These seven bits are only used in Block entries and are ignored
> +     * in Table entries.
> +     */
>      unsigned long contig:1;     /* In a block of 16 contiguous entries */
>      unsigned long sbz2:1;
>      unsigned long xn:1;         /* eXecute-Never */
> -- 
> 2.11.0
>

Patch hide | download patch | download mbox

diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index 1e6a68926e..6244240ca0 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -3,10 +3,12 @@ 
 
 #ifndef __ASSEMBLY__
 
-/* WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
+/*
+ * WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
  * level and l4 is the root of the trie, the ARM pagetables follow ARM's
  * documentation: the levels are called first, second &c in the order
- * that the MMU walks them (i.e. "first" is the root of the trie). */
+ * that the MMU walks them (i.e. "first" is the root of the trie).
+ */
 
 /******************************************************************************
  * ARMv7-A LPAE pagetables: 3-level trie, mapping 40-bit input to
@@ -17,15 +19,18 @@ 
  * different place from those in leaf nodes seems to be to allow linear
  * pagetable tricks.  If we're not doing that then the set of permission
  * bits that's not in use in a given node type can be used as
- * extra software-defined bits. */
+ * extra software-defined bits.
+ */
 
 typedef struct __packed {
     /* These are used in all kinds of entry. */
     unsigned long valid:1;      /* Valid mapping */
     unsigned long table:1;      /* == 1 in 4k map entries too */
 
-    /* These ten bits are only used in Block entries and are ignored
-     * in Table entries. */
+    /*
+     * These ten bits are only used in Block entries and are ignored
+     * in Table entries.
+     */
     unsigned long ai:3;         /* Attribute Index */
     unsigned long ns:1;         /* Not-Secure */
     unsigned long user:1;       /* User-visible */
@@ -38,30 +43,38 @@  typedef struct __packed {
     unsigned long long base:36; /* Base address of block or next table */
     unsigned long sbz:4;        /* Must be zero */
 
-    /* These seven bits are only used in Block entries and are ignored
-     * in Table entries. */
+    /*
+     * These seven bits are only used in Block entries and are ignored
+     * in Table entries.
+     */
     unsigned long contig:1;     /* In a block of 16 contiguous entries */
     unsigned long pxn:1;        /* Privileged-XN */
     unsigned long xn:1;         /* eXecute-Never */
     unsigned long avail:4;      /* Ignored by hardware */
 
-    /* These 5 bits are only used in Table entries and are ignored in
-     * Block entries */
+    /*
+     * These 5 bits are only used in Table entries and are ignored in
+     * Block entries.
+     */
     unsigned long pxnt:1;       /* Privileged-XN */
     unsigned long xnt:1;        /* eXecute-Never */
     unsigned long apt:2;        /* Access Permissions */
     unsigned long nst:1;        /* Not-Secure */
 } lpae_pt_t;
 
-/* The p2m tables have almost the same layout, but some of the permission
- * and cache-control bits are laid out differently (or missing) */
+/*
+ * The p2m tables have almost the same layout, but some of the permission
+ * and cache-control bits are laid out differently (or missing).
+ */
 typedef struct __packed {
     /* These are used in all kinds of entry. */
     unsigned long valid:1;      /* Valid mapping */
     unsigned long table:1;      /* == 1 in 4k map entries too */
 
-    /* These ten bits are only used in Block entries and are ignored
-     * in Table entries. */
+    /*
+     * These ten bits are only used in Block entries and are ignored
+     * in Table entries.
+     */
     unsigned long mattr:4;      /* Memory Attributes */
     unsigned long read:1;       /* Read access */
     unsigned long write:1;      /* Write access */
@@ -73,8 +86,10 @@  typedef struct __packed {
     unsigned long long base:36; /* Base address of block or next table */
     unsigned long sbz3:4;
 
-    /* These seven bits are only used in Block entries and are ignored
-     * in Table entries. */
+    /*
+     * These seven bits are only used in Block entries and are ignored
+     * in Table entries.
+     */
     unsigned long contig:1;     /* In a block of 16 contiguous entries */
     unsigned long sbz2:1;
     unsigned long xn:1;         /* eXecute-Never */