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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

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@xxxxxxxxxx>
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 
> ---
> 
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>


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

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
> 

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