Re: [Xen-devel] [for-4.8][PATCH v2 11/23] xen/arm: p2m: Introduce p2m_get_root_pointer and use it in __p2m_lookup

[Stefano Stabellini]

On Thu, 15 Sep 2016, Julien Grall wrote:
> Mapping the root table is always done the same way. To avoid duplicating
> the code in a later patch, move the code in a separate helper.
> 
> Signed-off-by: Julien Grall 

Reviewed-by: Stefano Stabellini 


> ---
> Changes in v2:
> - Use level_orders rather than level_shifts - PAGE_SHIFT
> - Move the definition of level_orders in this patch
> * use uint8_t rather than unsigned
> * define *_ORDER in term of *_SHIFT
> ---
>  xen/arch/arm/p2m.c | 55 
> +++---
>  xen/include/asm-arm/page.h |  4 
>  2 files changed, 41 insertions(+), 18 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 413780b..b2a29ad 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -36,6 +36,8 @@ static const paddr_t level_masks[] =
>  { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
>  static const uint8_t level_shifts[] =
>  { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
> +static const uint8_t level_orders[] =
> +{ ZEROETH_ORDER, FIRST_ORDER, SECOND_ORDER, THIRD_ORDER };
>  
>  static bool_t p2m_valid(lpae_t pte)
>  {
> @@ -204,6 +206,37 @@ static void p2m_flush_tlb_sync(struct p2m_domain *p2m)
>  }
>  
>  /*
> + * Find and map the root page table. The caller is responsible for
> + * unmapping the table.
> + *
> + * The function will return NULL if the offset of the root table is
> + * invalid.
> + */
> +static lpae_t *p2m_get_root_pointer(struct p2m_domain *p2m,
> +gfn_t gfn)
> +{
> +unsigned int root_table;
> +
> +if ( P2M_ROOT_PAGES == 1 )
> +return __map_domain_page(p2m->root);
> +
> +/*
> + * Concatenated root-level tables. The table number will be the
> + * offset at the previous level. It is not possible to
> + * concatenate a level-0 root.
> + */
> +ASSERT(P2M_ROOT_LEVEL > 0);
> +
> +root_table = gfn_x(gfn) >> (level_orders[P2M_ROOT_LEVEL - 1]);
> +root_table &= LPAE_ENTRY_MASK;
> +
> +if ( root_table >= P2M_ROOT_PAGES )
> +return NULL;
> +
> +return __map_domain_page(p2m->root + root_table);
> +}
> +
> +/*
>   * Lookup the MFN corresponding to a domain's GFN.
>   *
>   * There are no processor functions to do a stage 2 only lookup therefore we
> @@ -226,7 +259,7 @@ static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, 
> p2m_type_t *t)
>  mfn_t mfn = INVALID_MFN;
>  paddr_t mask = 0;
>  p2m_type_t _t;
> -unsigned int level, root_table;
> +unsigned int level;
>  
>  ASSERT(p2m_is_locked(p2m));
>  BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
> @@ -236,22 +269,9 @@ static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, 
> p2m_type_t *t)
>  
>  *t = p2m_invalid;
>  
> -if ( P2M_ROOT_PAGES > 1 )
> -{
> -/*
> - * Concatenated root-level tables. The table number will be
> - * the offset at the previous level. It is not possible to
> - * concatenate a level-0 root.
> - */
> -ASSERT(P2M_ROOT_LEVEL > 0);
> -root_table = offsets[P2M_ROOT_LEVEL - 1];
> -if ( root_table >= P2M_ROOT_PAGES )
> -goto err;
> -}
> -else
> -root_table = 0;
> -
> -map = __map_domain_page(p2m->root + root_table);
> +map = p2m_get_root_pointer(p2m, gfn);
> +if ( !map )
> +return INVALID_MFN;
>  
>  ASSERT(P2M_ROOT_LEVEL < 4);
>  
> @@ -286,7 +306,6 @@ static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, 
> p2m_type_t *t)
>  *t = pte.p2m.type;
>  }
>  
> -err:
>  return mfn;
>  }
>  
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 05d9f82..a43b0fa 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -457,15 +457,19 @@ static inline int gva_to_ipa(vaddr_t va, paddr_t 
> *paddr, unsigned int flags)
>  #define LPAE_ENTRY_MASK (LPAE_ENTRIES - 1)
>  
>  #define THIRD_SHIFT(PAGE_SHIFT)
> +#define THIRD_ORDER(THIRD_SHIFT - PAGE_SHIFT)
>  #define THIRD_SIZE ((paddr_t)1 << THIRD_SHIFT)
>  #define THIRD_MASK (~(THIRD_SIZE - 1))
>  #define SECOND_SHIFT   (THIRD_SHIFT + LPAE_SHIFT)
> +#define SECOND_ORDER   (SECOND_SHIFT - PAGE_SHIFT)
>  #define SECOND_SIZE((paddr_t)1 << SECOND_SHIFT)
>  #define SECOND_MASK(~(SECOND_SIZE - 1))
>  #define FIRST_SHIFT(SECOND_SHIFT + LPAE_SHIFT)
> +#define FIRST_ORDER(FIRST_SHIFT - PAGE_SHIFT)
>  #define FIRST_SIZE ((paddr_t)1 << FIRST_SHIFT)
>  #define FIRST_MASK (~(FIRST_SIZE - 1))
>  #define ZEROETH_SHIFT  (FIRST_SHIFT + LPAE_SHIFT)
> +#define ZEROETH_ORDER  (ZEROETH_SHIFT - PAGE_SHIFT)
>  #define ZEROETH_SIZE   ((paddr_t)1 << ZEROETH_SHIFT)
>  #define ZEROETH_MASK   (~(ZEROETH_SIZE - 1))
>  
> -- 
> 1.9.1
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org

[Xen-devel] [for-4.8][PATCH v2 11/23] xen/arm: p2m: Introduce p2m_get_root_pointer and use it in __p2m_lookup

[Julien Grall]

Mapping the root table is always done the same way. To avoid duplicating
the code in a later patch, move the code in a separate helper.

Signed-off-by: Julien Grall 

---
Changes in v2:
- Use level_orders rather than level_shifts - PAGE_SHIFT
- Move the definition of level_orders in this patch
* use uint8_t rather than unsigned
* define *_ORDER in term of *_SHIFT
---
 xen/arch/arm/p2m.c | 55 +++---
 xen/include/asm-arm/page.h |  4 
 2 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 413780b..b2a29ad 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -36,6 +36,8 @@ static const paddr_t level_masks[] =
 { ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK };
 static const uint8_t level_shifts[] =
 { ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
+static const uint8_t level_orders[] =
+{ ZEROETH_ORDER, FIRST_ORDER, SECOND_ORDER, THIRD_ORDER };
 
 static bool_t p2m_valid(lpae_t pte)
 {
@@ -204,6 +206,37 @@ static void p2m_flush_tlb_sync(struct p2m_domain *p2m)
 }
 
 /*
+ * Find and map the root page table. The caller is responsible for
+ * unmapping the table.
+ *
+ * The function will return NULL if the offset of the root table is
+ * invalid.
+ */
+static lpae_t *p2m_get_root_pointer(struct p2m_domain *p2m,
+gfn_t gfn)
+{
+unsigned int root_table;
+
+if ( P2M_ROOT_PAGES == 1 )
+return __map_domain_page(p2m->root);
+
+/*
+ * Concatenated root-level tables. The table number will be the
+ * offset at the previous level. It is not possible to
+ * concatenate a level-0 root.
+ */
+ASSERT(P2M_ROOT_LEVEL > 0);
+
+root_table = gfn_x(gfn) >> (level_orders[P2M_ROOT_LEVEL - 1]);
+root_table &= LPAE_ENTRY_MASK;
+
+if ( root_table >= P2M_ROOT_PAGES )
+return NULL;
+
+return __map_domain_page(p2m->root + root_table);
+}
+
+/*
  * Lookup the MFN corresponding to a domain's GFN.
  *
  * There are no processor functions to do a stage 2 only lookup therefore we
@@ -226,7 +259,7 @@ static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, 
p2m_type_t *t)
 mfn_t mfn = INVALID_MFN;
 paddr_t mask = 0;
 p2m_type_t _t;
-unsigned int level, root_table;
+unsigned int level;
 
 ASSERT(p2m_is_locked(p2m));
 BUILD_BUG_ON(THIRD_MASK != PAGE_MASK);
@@ -236,22 +269,9 @@ static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, 
p2m_type_t *t)
 
 *t = p2m_invalid;
 
-if ( P2M_ROOT_PAGES > 1 )
-{
-/*
- * Concatenated root-level tables. The table number will be
- * the offset at the previous level. It is not possible to
- * concatenate a level-0 root.
- */
-ASSERT(P2M_ROOT_LEVEL > 0);
-root_table = offsets[P2M_ROOT_LEVEL - 1];
-if ( root_table >= P2M_ROOT_PAGES )
-goto err;
-}
-else
-root_table = 0;
-
-map = __map_domain_page(p2m->root + root_table);
+map = p2m_get_root_pointer(p2m, gfn);
+if ( !map )
+return INVALID_MFN;
 
 ASSERT(P2M_ROOT_LEVEL < 4);
 
@@ -286,7 +306,6 @@ static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, 
p2m_type_t *t)
 *t = pte.p2m.type;
 }
 
-err:
 return mfn;
 }
 
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 05d9f82..a43b0fa 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -457,15 +457,19 @@ static inline int gva_to_ipa(vaddr_t va, paddr_t *paddr, 
unsigned int flags)
 #define LPAE_ENTRY_MASK (LPAE_ENTRIES - 1)
 
 #define THIRD_SHIFT(PAGE_SHIFT)
+#define THIRD_ORDER(THIRD_SHIFT - PAGE_SHIFT)
 #define THIRD_SIZE ((paddr_t)1 << THIRD_SHIFT)
 #define THIRD_MASK (~(THIRD_SIZE - 1))
 #define SECOND_SHIFT   (THIRD_SHIFT + LPAE_SHIFT)
+#define SECOND_ORDER   (SECOND_SHIFT - PAGE_SHIFT)
 #define SECOND_SIZE((paddr_t)1 << SECOND_SHIFT)
 #define SECOND_MASK(~(SECOND_SIZE - 1))
 #define FIRST_SHIFT(SECOND_SHIFT + LPAE_SHIFT)
+#define FIRST_ORDER(FIRST_SHIFT - PAGE_SHIFT)
 #define FIRST_SIZE ((paddr_t)1 << FIRST_SHIFT)
 #define FIRST_MASK (~(FIRST_SIZE - 1))
 #define ZEROETH_SHIFT  (FIRST_SHIFT + LPAE_SHIFT)
+#define ZEROETH_ORDER  (ZEROETH_SHIFT - PAGE_SHIFT)
 #define ZEROETH_SIZE   ((paddr_t)1 << ZEROETH_SHIFT)
 #define ZEROETH_MASK   (~(ZEROETH_SIZE - 1))
 
-- 
1.9.1


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