Re: [Xen-devel] [RFC PATCH v3 03/10] arm/mem_access: Add defines supporting PTs with varying page sizes

[Julien Grall <julien.grall@xxxxxxx>]

Hi Sergej,

On 06/15/2017 12:05 PM, Sergej Proskurin wrote:
> The ARMv8 architecture supports pages with different (4K, 16K, and 64K) sizes.
> To enable guest page table walks for various configurations, this commit
> extends the defines and helpers of the current implementation.
>
> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> ---
> v3: Eliminate redundant macro definitions by introducing generic macros.
> ---
>   xen/include/asm-arm/page.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
>
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 497b4c86ad..e2e4b597a5 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -500,6 +500,51 @@ static inline int gva_to_ipa(vaddr_t va, paddr_t *paddr, 
> unsigned int flags)
>   
>   #define PAGE_ALIGN(x) (((x) + PAGE_SIZE - 1) & PAGE_MASK)
>   
> +#define LPAE_SHIFT_4K           (9)
> +#define LPAE_SHIFT_16K          (11)
> +#define LPAE_SHIFT_64K          (13)
> +
> +#define lpae_entries(gran)      (_AC(1,U) << LPAE_SHIFT_##gran)
> +#define lpae_entry_mask(gran)   (lpae_entries(gran) - 1)
> +
> +#define PAGE_SHIFT_4K           (12)
> +#define PAGE_SHIFT_16K          (14)
> +#define PAGE_SHIFT_64K          (16)
> +
> +#define third_shift(gran)       (PAGE_SHIFT_##gran)
> +#define third_size(gran)        ((paddr_t)1 << third_shift(gran))
> +
> +#define second_shift(gran)      (third_shift(gran) + LPAE_SHIFT_##gran)
> +#define second_size(gran)       ((paddr_t)1 << second_shift(gran))
> +
> +#define first_shift(gran)       (second_shift(gran) + LPAE_SHIFT_##gran)
> +#define first_size(gran)        ((paddr_t)1 << first_shift(gran))
> +
> +/* Note that there is no zeroeth lookup level with a 64K granule size. */
> +#define zeroeth_shift(gran)     (first_shift(gran) + LPAE_SHIFT_##gran)
> +#define zeroeth_size(gran)      ((paddr_t)1 << zeroeth_shift(gran))
> +
> +#define GUEST_TABLE_OFFSET(offs, gran)          ((paddr_t)(offs) & 
> lpae_entry_mask(gran))
> +#define third_guest_table_offset(va, gran)      GUEST_TABLE_OFFSET((va >> 
> third_shift(gran)), gran)
> +#define second_guest_table_offset(va, gran)     GUEST_TABLE_OFFSET((va >> 
> second_shift(gran)), gran)
> +#define first_guest_table_offset(va, gran)      GUEST_TABLE_OFFSET((va >> 
> first_shift(gran)), gran)
> +#define zeroeth_guest_table_offset(va, gran)    GUEST_TABLE_OFFSET((va >> 
> zeroeth_shift(gran)), gran)
> +
> +#define third_guest_table_offset_4k(va)         third_guest_table_offset(va, 
> 4K)
> +#define third_guest_table_offset_16k(va)        third_guest_table_offset(va, 
> 16K)
> +#define third_guest_table_offset_64k(va)        third_guest_table_offset(va, 
> 64K)
> +
> +#define second_guest_table_offset_4k(va)        second_guest_table_offset(va, 
> 4K)
> +#define second_guest_table_offset_16k(va)       second_guest_table_offset(va, 
> 16K)
> +#define second_guest_table_offset_64k(va)       second_guest_table_offset(va, 
> 64K)
> +
> +#define first_guest_table_offset_4k(va)         first_guest_table_offset(va, 
> 4K)
> +#define first_guest_table_offset_16k(va)        first_guest_table_offset(va, 
> 16K)
> +#define first_guest_table_offset_64k(va)        first_guest_table_offset(va, 
> 64K)
> +
> +#define zeroeth_guest_table_offset_4k(va)       zeroeth_guest_table_offset(va, 
> 4K)
> +#define zeroeth_guest_table_offset_16k(va)      zeroeth_guest_table_offset(va, 
> 16K)
So this is really confusing to group by level rather than granularity.

Also, I still think you can make this more generic by introducing macros 
that will generate helpers (see VGIC_REG_HELPERS).
This would require to use static inline function, but at least it would 
avoid to duplicate some much code.
> +
>   #endif /* __ARM_PAGE_H__ */
>   
>   /*
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel