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

2017-07-17 Thread Julien Grall 

Hi Sergej,

On 06/07/17 12:50, Sergej Proskurin wrote:

The ARMv8 architecture supports pages with different (4K, 16K, and 64K) sizes.


NIT: ARMv8 supports both AArch32 and AArch64. However, only AArch64 
supports different page granularities. To avoid confusion, please 
s/ARMv8 architecture/AArch64/



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 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
v3: Eliminate redundant macro definitions by introducing generic macros.

v4: Replace existing macros with ones that generate static inline
helpers as to ease the readability of the code.

Move the introduced code into lpae.h

v5: Remove PAGE_SHIFT_* defines from lpae.h as we import them now from
the header xen/lib.h.

Remove *_guest_table_offset macros as to reduce the number of
exported macros which are only used once. Instead, use the
associated functionality directly within the
GUEST_TABLE_OFFSET_HELPERS.

Add comment in GUEST_TABLE_OFFSET_HELPERS stating that a page table
with 64K page size granularity does not have a zeroeth lookup level.

Add #undefs for GUEST_TABLE_OFFSET and GUEST_TABLE_OFFSET_HELPERS.

Remove CONFIG_ARM_64 #defines.

v6: Rename *_guest_table_offset_* helpers to *_table_offset_* as they
are sufficiently generic to be applied not only to the guest's page
table walks.

Change the type of the parameter and return value of the
*_table_offset_* helpers from vaddr_t to paddr_t to enable applying
these helpers also for other purposes such as computation of IPA
offsets in second stage translation tables.
---
 xen/include/asm-arm/lpae.h | 62 ++
 1 file changed, 62 insertions(+)

diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index a62b118630..f0b3d21aa7 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -3,6 +3,8 @@

 #ifndef __ASSEMBLY__

+#include 
+
 /*
  * WARNING!  Unlike the x86 pagetable code, where l1 is the lowest level and
  * l4 is the root of the trie, the ARM pagetables follow ARM's documentation:
@@ -151,6 +153,66 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned 
int level)
 return (level < 3) && lpae_mapping(pte);
 }

+/*
+ * The ARMv8 architecture supports pages with different sizes (4K, 16K, and


Same here.


+ * 64K). To enable page table walks for various configurations, the following
+ * helpers enable walking the translation table with varying page size
+ * granularities.
+ */
+
+#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 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)  (offs & lpae_entry_mask(gran))
+#define GUEST_TABLE_OFFSET_HELPERS(gran)   
 \


You renamed *_guest_table_offset to *_table_offset but not GUEST_TABLE_* 
one.


Please drop GUEST from both macros.

With that fixed:

Reviewed-by: Julien Grall 

Cheers,


+static inline paddr_t third_table_offset_##gran##K(paddr_t va) 
 \
+{  
 \
+return GUEST_TABLE_OFFSET((va >> third_shift(gran##K)), gran##K);  
 \
+}  
 \
+   
 \
+static inline paddr_t second_table_offset_##gran##K(paddr_t va)
 \
+{  
 \
+return GUEST_TABLE_OFFSET((va >> second_shift(gran##K)), gran##K); 
 \
+}  
 \
+   
 \
+static inline paddr_t first_table_offset_##gran##K(paddr_t va) 
 \
+{  
 \
+return GUEST_TABLE_OFFSET((va >>

[Xen-devel] [PATCH v6 03/14] arm/mem_access: Add defines supporting PTs with varying page sizes

2017-07-06 Thread Sergej Proskurin 
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 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
v3: Eliminate redundant macro definitions by introducing generic macros.

v4: Replace existing macros with ones that generate static inline
helpers as to ease the readability of the code.

Move the introduced code into lpae.h

v5: Remove PAGE_SHIFT_* defines from lpae.h as we import them now from
the header xen/lib.h.

Remove *_guest_table_offset macros as to reduce the number of
exported macros which are only used once. Instead, use the
associated functionality directly within the
GUEST_TABLE_OFFSET_HELPERS.

Add comment in GUEST_TABLE_OFFSET_HELPERS stating that a page table
with 64K page size granularity does not have a zeroeth lookup level.

Add #undefs for GUEST_TABLE_OFFSET and GUEST_TABLE_OFFSET_HELPERS.

Remove CONFIG_ARM_64 #defines.

v6: Rename *_guest_table_offset_* helpers to *_table_offset_* as they
are sufficiently generic to be applied not only to the guest's page
table walks.

Change the type of the parameter and return value of the
*_table_offset_* helpers from vaddr_t to paddr_t to enable applying
these helpers also for other purposes such as computation of IPA
offsets in second stage translation tables.
---
 xen/include/asm-arm/lpae.h | 62 ++
 1 file changed, 62 insertions(+)

diff --git a/xen/include/asm-arm/lpae.h b/xen/include/asm-arm/lpae.h
index a62b118630..f0b3d21aa7 100644
--- a/xen/include/asm-arm/lpae.h
+++ b/xen/include/asm-arm/lpae.h
@@ -3,6 +3,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include 
+
 /*
  * WARNING!  Unlike the x86 pagetable code, where l1 is the lowest level and
  * l4 is the root of the trie, the ARM pagetables follow ARM's documentation:
@@ -151,6 +153,66 @@ static inline bool lpae_is_superpage(lpae_t pte, unsigned 
int level)
 return (level < 3) && lpae_mapping(pte);
 }
 
+/*
+ * The ARMv8 architecture supports pages with different sizes (4K, 16K, and
+ * 64K). To enable page table walks for various configurations, the following
+ * helpers enable walking the translation table with varying page size
+ * granularities.
+ */
+
+#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 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)  (offs & lpae_entry_mask(gran))
+#define GUEST_TABLE_OFFSET_HELPERS(gran)   
 \
+static inline paddr_t third_table_offset_##gran##K(paddr_t va) 
 \
+{  
 \
+return GUEST_TABLE_OFFSET((va >> third_shift(gran##K)), gran##K);  
 \
+}  
 \
+   
 \
+static inline paddr_t second_table_offset_##gran##K(paddr_t va)
 \
+{  
 \
+return GUEST_TABLE_OFFSET((va >> second_shift(gran##K)), gran##K); 
 \
+}  
 \
+   
 \
+static inline paddr_t first_table_offset_##gran##K(paddr_t va) 
 \
+{  
 \
+return GUEST_TABLE_OFFSET((va >> first_shift(gran##K)), gran##K);  
 \
+}  
 \
+   
 \
+static inline paddr_t zeroeth_table_offset_##gran##K(paddr_t va)   
 \
+{  
 \
+/* Note that there is no zeroeth lookup level with a