[Xen-devel,v3,for-next,3/4] xen/tmem: Convert the file common/tmem_xen.c to use typesafe MFN

Message ID 20171101140316.31333-4-julien.grall@linaro.org
State New
Headers show
Series
  • xen: Convert __page_to_mfn and _mfn_to_page to use typesafe MFN
Related show

Commit Message

Julien Grall Nov. 1, 2017, 2:03 p.m.
The file common/tmem_xen.c is now converted to use typesafe. This is
requiring to override the macro page_to_mfn to make it work with mfn_t.

Note that all variables converted to mfn_t havem there initial value,
when set, switch from 0 to INVALID_MFN. This is fine because the initial
values was always overriden before used.

Also add a couple of missing newlines suggested by Andrew in the code.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

---

Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

    Changes in v2:
        - Add missing newlines
        - Add Andrew's reviewed-by
---
 xen/common/tmem_xen.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

Comments

Konrad Rzeszutek Wilk Nov. 1, 2017, 6:54 p.m. | #1
On Wed, Nov 01, 2017 at 02:03:15PM +0000, Julien Grall wrote:
> The file common/tmem_xen.c is now converted to use typesafe. This is
> requiring to override the macro page_to_mfn to make it work with mfn_t.
> 
> Note that all variables converted to mfn_t havem there initial value,
> when set, switch from 0 to INVALID_MFN. This is fine because the initial
> values was always overriden before used.
> 
> Also add a couple of missing newlines suggested by Andrew in the code.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> ---
> 
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>


Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

But could you confirm that you did compile this on x86 and with CONFIG_TMEM=y in the .config?

Thanks!

> 
>     Changes in v2:
>         - Add missing newlines
>         - Add Andrew's reviewed-by
> ---
>  xen/common/tmem_xen.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
> index 20f74b268f..bd52e44faf 100644
> --- a/xen/common/tmem_xen.c
> +++ b/xen/common/tmem_xen.c
> @@ -14,6 +14,10 @@
>  #include <xen/cpu.h>
>  #include <xen/init.h>
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef page_to_mfn
> +#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
> +
>  bool __read_mostly opt_tmem;
>  boolean_param("tmem", opt_tmem);
>  
> @@ -31,7 +35,7 @@ static DEFINE_PER_CPU_READ_MOSTLY(unsigned char *, dstmem);
>  static DEFINE_PER_CPU_READ_MOSTLY(void *, scratch_page);
>  
>  #if defined(CONFIG_ARM)
> -static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
> +static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
>                                   struct page_info **pcli_pfp, bool cli_write)
>  {
>      ASSERT_UNREACHABLE();
> @@ -39,14 +43,14 @@ static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
>  }
>  
>  static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
> -                                unsigned long cli_mfn, bool mark_dirty)
> +                                mfn_t cli_mfn, bool mark_dirty)
>  {
>      ASSERT_UNREACHABLE();
>  }
>  #else
>  #include <asm/p2m.h>
>  
> -static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
> +static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
>                                   struct page_info **pcli_pfp, bool cli_write)
>  {
>      p2m_type_t t;
> @@ -68,16 +72,17 @@ static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
>  
>      *pcli_mfn = page_to_mfn(page);
>      *pcli_pfp = page;
> -    return map_domain_page(_mfn(*pcli_mfn));
> +
> +    return map_domain_page(*pcli_mfn);
>  }
>  
>  static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
> -                                unsigned long cli_mfn, bool mark_dirty)
> +                                mfn_t cli_mfn, bool mark_dirty)
>  {
>      if ( mark_dirty )
>      {
>          put_page_and_type(cli_pfp);
> -        paging_mark_dirty(current->domain, _mfn(cli_mfn));
> +        paging_mark_dirty(current->domain, cli_mfn);
>      }
>      else
>          put_page(cli_pfp);
> @@ -88,14 +93,14 @@ static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
>  int tmem_copy_from_client(struct page_info *pfp,
>      xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
>  {
> -    unsigned long tmem_mfn, cli_mfn = 0;
> +    mfn_t tmem_mfn, cli_mfn = INVALID_MFN;
>      char *tmem_va, *cli_va = NULL;
>      struct page_info *cli_pfp = NULL;
>      int rc = 1;
>  
>      ASSERT(pfp != NULL);
>      tmem_mfn = page_to_mfn(pfp);
> -    tmem_va = map_domain_page(_mfn(tmem_mfn));
> +    tmem_va = map_domain_page(tmem_mfn);
>      if ( guest_handle_is_null(clibuf) )
>      {
>          cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
> @@ -125,7 +130,7 @@ int tmem_compress_from_client(xen_pfn_t cmfn,
>      unsigned char *wmem = this_cpu(workmem);
>      char *scratch = this_cpu(scratch_page);
>      struct page_info *cli_pfp = NULL;
> -    unsigned long cli_mfn = 0;
> +    mfn_t cli_mfn = INVALID_MFN;
>      void *cli_va = NULL;
>  
>      if ( dmem == NULL || wmem == NULL )
> @@ -152,7 +157,7 @@ int tmem_compress_from_client(xen_pfn_t cmfn,
>  int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
>      tmem_cli_va_param_t clibuf)
>  {
> -    unsigned long tmem_mfn, cli_mfn = 0;
> +    mfn_t tmem_mfn, cli_mfn = INVALID_MFN;
>      char *tmem_va, *cli_va = NULL;
>      struct page_info *cli_pfp = NULL;
>      int rc = 1;
> @@ -165,7 +170,8 @@ int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
>              return -EFAULT;
>      }
>      tmem_mfn = page_to_mfn(pfp);
> -    tmem_va = map_domain_page(_mfn(tmem_mfn));
> +    tmem_va = map_domain_page(tmem_mfn);
> +
>      if ( cli_va )
>      {
>          memcpy(cli_va, tmem_va, PAGE_SIZE);
> @@ -181,7 +187,7 @@ int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
>  int tmem_decompress_to_client(xen_pfn_t cmfn, void *tmem_va,
>                                      size_t size, tmem_cli_va_param_t clibuf)
>  {
> -    unsigned long cli_mfn = 0;
> +    mfn_t cli_mfn = INVALID_MFN;
>      struct page_info *cli_pfp = NULL;
>      void *cli_va = NULL;
>      char *scratch = this_cpu(scratch_page);
> -- 
> 2.11.0
>
Julien Grall Nov. 2, 2017, 3:18 p.m. | #2
Hi Konrad,

On 01/11/17 18:54, Konrad Rzeszutek Wilk wrote:
> On Wed, Nov 01, 2017 at 02:03:15PM +0000, Julien Grall wrote:
>> The file common/tmem_xen.c is now converted to use typesafe. This is
>> requiring to override the macro page_to_mfn to make it work with mfn_t.
>>
>> Note that all variables converted to mfn_t havem there initial value,
>> when set, switch from 0 to INVALID_MFN. This is fine because the initial
>> values was always overriden before used.
>>
>> Also add a couple of missing newlines suggested by Andrew in the code.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> ---
>>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> 
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> But could you confirm that you did compile this on x86 and with CONFIG_TMEM=y in the .config?

Yes, I have CONFIG_TMEM enabled in .config.

Thank you for the ack,

Patch

diff --git a/xen/common/tmem_xen.c b/xen/common/tmem_xen.c
index 20f74b268f..bd52e44faf 100644
--- a/xen/common/tmem_xen.c
+++ b/xen/common/tmem_xen.c
@@ -14,6 +14,10 @@ 
 #include <xen/cpu.h>
 #include <xen/init.h>
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef page_to_mfn
+#define page_to_mfn(pg) _mfn(__page_to_mfn(pg))
+
 bool __read_mostly opt_tmem;
 boolean_param("tmem", opt_tmem);
 
@@ -31,7 +35,7 @@  static DEFINE_PER_CPU_READ_MOSTLY(unsigned char *, dstmem);
 static DEFINE_PER_CPU_READ_MOSTLY(void *, scratch_page);
 
 #if defined(CONFIG_ARM)
-static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
+static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
                                  struct page_info **pcli_pfp, bool cli_write)
 {
     ASSERT_UNREACHABLE();
@@ -39,14 +43,14 @@  static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
 }
 
 static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
-                                unsigned long cli_mfn, bool mark_dirty)
+                                mfn_t cli_mfn, bool mark_dirty)
 {
     ASSERT_UNREACHABLE();
 }
 #else
 #include <asm/p2m.h>
 
-static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
+static inline void *cli_get_page(xen_pfn_t cmfn, mfn_t *pcli_mfn,
                                  struct page_info **pcli_pfp, bool cli_write)
 {
     p2m_type_t t;
@@ -68,16 +72,17 @@  static inline void *cli_get_page(xen_pfn_t cmfn, unsigned long *pcli_mfn,
 
     *pcli_mfn = page_to_mfn(page);
     *pcli_pfp = page;
-    return map_domain_page(_mfn(*pcli_mfn));
+
+    return map_domain_page(*pcli_mfn);
 }
 
 static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
-                                unsigned long cli_mfn, bool mark_dirty)
+                                mfn_t cli_mfn, bool mark_dirty)
 {
     if ( mark_dirty )
     {
         put_page_and_type(cli_pfp);
-        paging_mark_dirty(current->domain, _mfn(cli_mfn));
+        paging_mark_dirty(current->domain, cli_mfn);
     }
     else
         put_page(cli_pfp);
@@ -88,14 +93,14 @@  static inline void cli_put_page(void *cli_va, struct page_info *cli_pfp,
 int tmem_copy_from_client(struct page_info *pfp,
     xen_pfn_t cmfn, tmem_cli_va_param_t clibuf)
 {
-    unsigned long tmem_mfn, cli_mfn = 0;
+    mfn_t tmem_mfn, cli_mfn = INVALID_MFN;
     char *tmem_va, *cli_va = NULL;
     struct page_info *cli_pfp = NULL;
     int rc = 1;
 
     ASSERT(pfp != NULL);
     tmem_mfn = page_to_mfn(pfp);
-    tmem_va = map_domain_page(_mfn(tmem_mfn));
+    tmem_va = map_domain_page(tmem_mfn);
     if ( guest_handle_is_null(clibuf) )
     {
         cli_va = cli_get_page(cmfn, &cli_mfn, &cli_pfp, 0);
@@ -125,7 +130,7 @@  int tmem_compress_from_client(xen_pfn_t cmfn,
     unsigned char *wmem = this_cpu(workmem);
     char *scratch = this_cpu(scratch_page);
     struct page_info *cli_pfp = NULL;
-    unsigned long cli_mfn = 0;
+    mfn_t cli_mfn = INVALID_MFN;
     void *cli_va = NULL;
 
     if ( dmem == NULL || wmem == NULL )
@@ -152,7 +157,7 @@  int tmem_compress_from_client(xen_pfn_t cmfn,
 int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
     tmem_cli_va_param_t clibuf)
 {
-    unsigned long tmem_mfn, cli_mfn = 0;
+    mfn_t tmem_mfn, cli_mfn = INVALID_MFN;
     char *tmem_va, *cli_va = NULL;
     struct page_info *cli_pfp = NULL;
     int rc = 1;
@@ -165,7 +170,8 @@  int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
             return -EFAULT;
     }
     tmem_mfn = page_to_mfn(pfp);
-    tmem_va = map_domain_page(_mfn(tmem_mfn));
+    tmem_va = map_domain_page(tmem_mfn);
+
     if ( cli_va )
     {
         memcpy(cli_va, tmem_va, PAGE_SIZE);
@@ -181,7 +187,7 @@  int tmem_copy_to_client(xen_pfn_t cmfn, struct page_info *pfp,
 int tmem_decompress_to_client(xen_pfn_t cmfn, void *tmem_va,
                                     size_t size, tmem_cli_va_param_t clibuf)
 {
-    unsigned long cli_mfn = 0;
+    mfn_t cli_mfn = INVALID_MFN;
     struct page_info *cli_pfp = NULL;
     void *cli_va = NULL;
     char *scratch = this_cpu(scratch_page);