diff mbox series

[v3,01/19] xen/arm: lpae: Rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS

Message ID 20220221102218.33785-2-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series xen/arm: mm: Remove open-coding mappings | expand

Commit Message

Julien Grall Feb. 21, 2022, 10:22 a.m. UTC 
From: Julien Grall <jgrall@amazon.com>

Commit 05031fa87357 "xen/arm: guest_walk: Only generate necessary
offsets/masks" introduced LPAE_ENTRIES_MASK_GS. In a follow-up patch,
we will use it for to define LPAE_ENTRY_MASK.

This will lead to inconsistent naming. As LPAE_ENTRY_MASK is used in
many places, it is better to rename LPAE_ENTRIES_MASK_GS and avoid
some churn.

So rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---
    Changes in v2:
        - New patch
---
 xen/arch/arm/include/asm/lpae.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michal Orzel Feb. 22, 2022, 1:30 p.m. UTC | #1 
Hi Julien,

On 21.02.2022 11:22, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> Commit 05031fa87357 "xen/arm: guest_walk: Only generate necessary
> offsets/masks" introduced LPAE_ENTRIES_MASK_GS. In a follow-up patch,
> we will use it for to define LPAE_ENTRY_MASK.
> 
for is unneddded. Should be "we will use it to define..."

> This will lead to inconsistent naming. As LPAE_ENTRY_MASK is used in
> many places, it is better to rename LPAE_ENTRIES_MASK_GS and avoid
> some churn.
> 
> So rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> ---
>     Changes in v2:
>         - New patch
> ---
>  xen/arch/arm/include/asm/lpae.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
> index e94de2e7d8e8..4fb9a40a4ca9 100644
> --- a/xen/arch/arm/include/asm/lpae.h
> +++ b/xen/arch/arm/include/asm/lpae.h
> @@ -180,7 +180,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
>   */
>  #define LPAE_SHIFT_GS(gs)         ((gs) - 3)
>  #define LPAE_ENTRIES_GS(gs)       (_AC(1, U) << LPAE_SHIFT_GS(gs))
> -#define LPAE_ENTRIES_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
> +#define LPAE_ENTRY_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)

Incorrect indentation of (LPAE_ENTRIES_GS(gs) - 1)
> 
>  #define LEVEL_ORDER_GS(gs, lvl)   ((3 - (lvl)) * LPAE_SHIFT_GS(gs))
>  #define LEVEL_SHIFT_GS(gs, lvl)   (LEVEL_ORDER_GS(gs, lvl) + (gs))
> @@ -188,7 +188,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
>  
>  /* Offset in the table at level 'lvl' */
>  #define LPAE_TABLE_INDEX_GS(gs, lvl, addr)   \
> -    (((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRIES_MASK_GS(gs))
> +    (((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRY_MASK_GS(gs))
>  
>  /* Generate an array @var containing the offset for each level from @addr */
>  #define DECLARE_OFFSETS(var, addr)          \
> 

Cheers,
Michal
Bertrand Marquis Feb. 22, 2022, 3:21 p.m. UTC | #2 
Hi Julien,

> On 21 Feb 2022, at 10:22, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> Commit 05031fa87357 "xen/arm: guest_walk: Only generate necessary
> offsets/masks" introduced LPAE_ENTRIES_MASK_GS. In a follow-up patch,
> we will use it for to define LPAE_ENTRY_MASK.
> 
> This will lead to inconsistent naming. As LPAE_ENTRY_MASK is used in
> many places, it is better to rename LPAE_ENTRIES_MASK_GS and avoid
> some churn.
> 
> So rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

With the 2 small fixes suggested by Michal, you can add my
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> 
> ---
>    Changes in v2:
>        - New patch
> ---
> xen/arch/arm/include/asm/lpae.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
> index e94de2e7d8e8..4fb9a40a4ca9 100644
> --- a/xen/arch/arm/include/asm/lpae.h
> +++ b/xen/arch/arm/include/asm/lpae.h
> @@ -180,7 +180,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
>  */
> #define LPAE_SHIFT_GS(gs)         ((gs) - 3)
> #define LPAE_ENTRIES_GS(gs)       (_AC(1, U) << LPAE_SHIFT_GS(gs))
> -#define LPAE_ENTRIES_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
> +#define LPAE_ENTRY_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
> 
> #define LEVEL_ORDER_GS(gs, lvl)   ((3 - (lvl)) * LPAE_SHIFT_GS(gs))
> #define LEVEL_SHIFT_GS(gs, lvl)   (LEVEL_ORDER_GS(gs, lvl) + (gs))
> @@ -188,7 +188,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
> 
> /* Offset in the table at level 'lvl' */
> #define LPAE_TABLE_INDEX_GS(gs, lvl, addr)   \
> -    (((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRIES_MASK_GS(gs))
> +    (((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRY_MASK_GS(gs))
> 
> /* Generate an array @var containing the offset for each level from @addr */
> #define DECLARE_OFFSETS(var, addr)          \
> -- 
> 2.32.0
>
Julien Grall Feb. 24, 2022, 10:19 p.m. UTC | #3 
On 22/02/2022 13:30, Michal Orzel wrote:
> Hi Julien,

Hi Michal,

> On 21.02.2022 11:22, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> Commit 05031fa87357 "xen/arm: guest_walk: Only generate necessary
>> offsets/masks" introduced LPAE_ENTRIES_MASK_GS. In a follow-up patch,
>> we will use it for to define LPAE_ENTRY_MASK.
>>
> for is unneddded. Should be "we will use it to define..."

I fixed this one and ...

> 
>> This will lead to inconsistent naming. As LPAE_ENTRY_MASK is used in
>> many places, it is better to rename LPAE_ENTRIES_MASK_GS and avoid
>> some churn.
>>
>> So rename LPAE_ENTRIES_MASK_GS to LPAE_ENTRY_MASK_GS.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>
>> ---
>>      Changes in v2:
>>          - New patch
>> ---
>>   xen/arch/arm/include/asm/lpae.h | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
>> index e94de2e7d8e8..4fb9a40a4ca9 100644
>> --- a/xen/arch/arm/include/asm/lpae.h
>> +++ b/xen/arch/arm/include/asm/lpae.h
>> @@ -180,7 +180,7 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
>>    */
>>   #define LPAE_SHIFT_GS(gs)         ((gs) - 3)
>>   #define LPAE_ENTRIES_GS(gs)       (_AC(1, U) << LPAE_SHIFT_GS(gs))
>> -#define LPAE_ENTRIES_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
>> +#define LPAE_ENTRY_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
> 
> Incorrect indentation of (LPAE_ENTRIES_GS(gs) - 1)

... this one in my tree. I will commit it with Bertrand's reviewed-by.

Thank you for the review!

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/lpae.h b/xen/arch/arm/include/asm/lpae.h
index e94de2e7d8e8..4fb9a40a4ca9 100644
--- a/xen/arch/arm/include/asm/lpae.h
+++ b/xen/arch/arm/include/asm/lpae.h
@@ -180,7 +180,7 @@  static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
  */
 #define LPAE_SHIFT_GS(gs)         ((gs) - 3)
 #define LPAE_ENTRIES_GS(gs)       (_AC(1, U) << LPAE_SHIFT_GS(gs))
-#define LPAE_ENTRIES_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
+#define LPAE_ENTRY_MASK_GS(gs)  (LPAE_ENTRIES_GS(gs) - 1)
 
 #define LEVEL_ORDER_GS(gs, lvl)   ((3 - (lvl)) * LPAE_SHIFT_GS(gs))
 #define LEVEL_SHIFT_GS(gs, lvl)   (LEVEL_ORDER_GS(gs, lvl) + (gs))
@@ -188,7 +188,7 @@  static inline bool lpae_is_superpage(lpae_t pte, unsigned int level)
 
 /* Offset in the table at level 'lvl' */
 #define LPAE_TABLE_INDEX_GS(gs, lvl, addr)   \
-    (((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRIES_MASK_GS(gs))
+    (((addr) >> LEVEL_SHIFT_GS(gs, lvl)) & LPAE_ENTRY_MASK_GS(gs))
 
 /* Generate an array @var containing the offset for each level from @addr */
 #define DECLARE_OFFSETS(var, addr)          \