[Xen-devel,RFC,06/16] xen/arm: p2m: Introduce a helper to generate P2M table entry from a page

Message ID 20181008183352.16291-7-julien.grall@arm.com
State Accepted
Commit e03885248cb965ed23b51df0c044a762c7d339ad
Headers show
Series
  • xen/arm: Implement Set/Way operations
Related show

Commit Message

Julien Grall Oct. 8, 2018, 6:33 p.m.
Generate P2M table entry requires to set some default values which are
worth to explain in a comment. At the moment, there are 2 places where
such entry are created but only one as proper comment.

Some move the code to generate P2M table entry in a separate helper.
This will be helpful in a follow-up patch to make modification on the
defaults.

At the same time, switch the default access from p2m->default_access to
p2m_access_rwx. This should not matter as permission are ignored for
table by the hardware.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/p2m.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

Comments

Stefano Stabellini Oct. 30, 2018, 12:14 a.m. | #1
On Mon, 8 Oct 2018, Julien Grall wrote:
> Generate P2M table entry requires to set some default values which are
> worth to explain in a comment. At the moment, there are 2 places where
> such entry are created but only one as proper comment.
> 
> Some move the code to generate P2M table entry in a separate helper.
> This will be helpful in a follow-up patch to make modification on the
> defaults.
> 
> At the same time, switch the default access from p2m->default_access to
> p2m_access_rwx. This should not matter as permission are ignored for
> table by the hardware.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

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

> ---
>  xen/arch/arm/p2m.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 8fffb42889..6c76298ebc 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -538,6 +538,16 @@ static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a)
>      return e;
>  }
>  
> +/* Generate table entry with correct attributes. */
> +static lpae_t page_to_p2m_table(struct page_info *page)
> +{
> +    /*
> +     * The access value does not matter because the hardware will ignore
> +     * the permission fields for table entry.
> +     */
> +    return mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid, p2m_access_rwx);
> +}
> +
>  static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool clean_pte)
>  {
>      write_pte(p, pte);
> @@ -558,7 +568,6 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry)
>  {
>      struct page_info *page;
>      lpae_t *p;
> -    lpae_t pte;
>  
>      ASSERT(!lpae_is_valid(*entry));
>  
> @@ -576,14 +585,7 @@ static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry)
>  
>      unmap_domain_page(p);
>  
> -    /*
> -     * The access value does not matter because the hardware will ignore
> -     * the permission fields for table entry.
> -     */
> -    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
> -                           p2m->default_access);
> -
> -    p2m_write_pte(entry, pte, p2m->clean_pte);
> +    p2m_write_pte(entry, page_to_p2m_table(page), p2m->clean_pte);
>  
>      return 0;
>  }
> @@ -764,14 +766,11 @@ static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry,
>  
>      unmap_domain_page(table);
>  
> -    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
> -                           p2m->default_access);
> -
>      /*
>       * Even if we failed, we should install the newly allocated LPAE
>       * entry. The caller will be in charge to free the sub-tree.
>       */
> -    p2m_write_pte(entry, pte, p2m->clean_pte);
> +    p2m_write_pte(entry, page_to_p2m_table(page), p2m->clean_pte);
>  
>      return rv;
>  }
> -- 
> 2.11.0
>

Patch

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 8fffb42889..6c76298ebc 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -538,6 +538,16 @@  static lpae_t mfn_to_p2m_entry(mfn_t mfn, p2m_type_t t, p2m_access_t a)
     return e;
 }
 
+/* Generate table entry with correct attributes. */
+static lpae_t page_to_p2m_table(struct page_info *page)
+{
+    /*
+     * The access value does not matter because the hardware will ignore
+     * the permission fields for table entry.
+     */
+    return mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid, p2m_access_rwx);
+}
+
 static inline void p2m_write_pte(lpae_t *p, lpae_t pte, bool clean_pte)
 {
     write_pte(p, pte);
@@ -558,7 +568,6 @@  static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry)
 {
     struct page_info *page;
     lpae_t *p;
-    lpae_t pte;
 
     ASSERT(!lpae_is_valid(*entry));
 
@@ -576,14 +585,7 @@  static int p2m_create_table(struct p2m_domain *p2m, lpae_t *entry)
 
     unmap_domain_page(p);
 
-    /*
-     * The access value does not matter because the hardware will ignore
-     * the permission fields for table entry.
-     */
-    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
-                           p2m->default_access);
-
-    p2m_write_pte(entry, pte, p2m->clean_pte);
+    p2m_write_pte(entry, page_to_p2m_table(page), p2m->clean_pte);
 
     return 0;
 }
@@ -764,14 +766,11 @@  static bool p2m_split_superpage(struct p2m_domain *p2m, lpae_t *entry,
 
     unmap_domain_page(table);
 
-    pte = mfn_to_p2m_entry(page_to_mfn(page), p2m_invalid,
-                           p2m->default_access);
-
     /*
      * Even if we failed, we should install the newly allocated LPAE
      * entry. The caller will be in charge to free the sub-tree.
      */
-    p2m_write_pte(entry, pte, p2m->clean_pte);
+    p2m_write_pte(entry, page_to_p2m_table(page), p2m->clean_pte);
 
     return rv;
 }