[Xen-devel,MM-PART3,v2,01/12] xen/arm: lpae: Add a macro to generate offsets from an address

There are few places requiring to generate offsets from an address.
Rather than open-coding everywhere, we can introduce a macro to do the
job for us.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

---
    Changes in v2:
        - Add Andrii's reviewed-by
---
 xen/arch/arm/p2m.c         | 23 +++--------------------
 xen/include/asm-arm/lpae.h |  9 +++++++++
 2 files changed, 12 insertions(+), 20 deletions(-)

On Tue, 14 May 2019, Julien Grall wrote:
> There are few places requiring to generate offsets from an address.
> Rather than open-coding everywhere, we can introduce a macro to do the
> job for us.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>
> 
> ---
>     Changes in v2:
>         - Add Andrii's reviewed-by
> ---
>  xen/arch/arm/p2m.c         | 23 +++--------------------
>  xen/include/asm-arm/lpae.h |  9 +++++++++
>  2 files changed, 12 insertions(+), 20 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 92c2413f20..06fa342a8f 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -368,12 +368,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
>      p2m_type_t _t;
>  
>      /* Convenience aliases */

I don't know if you want to keep this comment, we could get rid of it.
Either way:

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


> -    const unsigned int offsets[4] = {
> -        zeroeth_table_offset(addr),
> -        first_table_offset(addr),
> -        second_table_offset(addr),
> -        third_table_offset(addr)
> -    };
> +    DECLARE_OFFSETS(offsets, addr);
>  
>      ASSERT(p2m_is_locked(p2m));
>      BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
> @@ -888,7 +883,6 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
>                             p2m_type_t t,
>                             p2m_access_t a)
>  {
> -    paddr_t addr = gfn_to_gaddr(sgfn);
>      unsigned int level = 0;
>      unsigned int target = 3 - (page_order / LPAE_SHIFT);
>      lpae_t *entry, *table, orig_pte;
> @@ -897,12 +891,7 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
>      bool removing_mapping = mfn_eq(smfn, INVALID_MFN);
>  
>      /* Convenience aliases */
> -    const unsigned int offsets[4] = {
> -        zeroeth_table_offset(addr),
> -        first_table_offset(addr),
> -        second_table_offset(addr),
> -        third_table_offset(addr)
> -    };
> +    DECLARE_OFFSETS(offsets, gfn_to_gaddr(sgfn));
>  
>      ASSERT(p2m_is_write_locked(p2m));
>  
> @@ -1199,15 +1188,9 @@ bool p2m_resolve_translation_fault(struct domain *d, gfn_t gfn)
>      unsigned int level = 0;
>      bool resolved = false;
>      lpae_t entry, *table;
> -    paddr_t addr = gfn_to_gaddr(gfn);
>  
>      /* Convenience aliases */
> -    const unsigned int offsets[4] = {
> -        zeroeth_table_offset(addr),
> -        first_table_offset(addr),
> -        second_table_offset(addr),
> -        third_table_offset(addr)
> -    };
> +    DECLARE_OFFSETS(offsets, gfn_to_gaddr(gfn));
>  
>      p2m_write_lock(p2m);
>  
> diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
> index 545b0c8f24..c22780f8f3 100644
> --- a/xen/include/asm-arm/lpae.h
> +++ b/xen/include/asm-arm/lpae.h
> @@ -218,6 +218,15 @@ TABLE_OFFSET_HELPERS(64);
>  #undef TABLE_OFFSET
>  #undef TABLE_OFFSET_HELPERS
>  
> +/* Generate an array @var containing the offset for each level from @addr */
> +#define DECLARE_OFFSETS(var, addr)          \
> +    const unsigned int var[4] = {           \
> +        zeroeth_table_offset(addr),         \
> +        first_table_offset(addr),           \
> +        second_table_offset(addr),          \
> +        third_table_offset(addr)            \
> +    }
> +
>  #endif /* __ASSEMBLY__ */
>  
>  /*
> -- 
> 2.11.0
>

Hi Stefano,

On 11/06/2019 19:21, Stefano Stabellini wrote:
> On Tue, 14 May 2019, Julien Grall wrote:

>> There are few places requiring to generate offsets from an address.

>> Rather than open-coding everywhere, we can introduce a macro to do the

>> job for us.

>>

>> Signed-off-by: Julien Grall <julien.grall@arm.com>

>> Reviewed-by: Andrii Anisov <andrii_anisov@epam.com>

>>

>> ---

>>      Changes in v2:

>>          - Add Andrii's reviewed-by

>> ---

>>   xen/arch/arm/p2m.c         | 23 +++--------------------

>>   xen/include/asm-arm/lpae.h |  9 +++++++++

>>   2 files changed, 12 insertions(+), 20 deletions(-)

>>

>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c

>> index 92c2413f20..06fa342a8f 100644

>> --- a/xen/arch/arm/p2m.c

>> +++ b/xen/arch/arm/p2m.c

>> @@ -368,12 +368,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,

>>       p2m_type_t _t;

>>   

>>       /* Convenience aliases */

> 

> I don't know if you want to keep this comment, we could get rid of it.


It is probably useless now. I will drop it along with the newline on top.

> Either way:

> 

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


Thank you!

Cheers,

-- 
Julien Grall