| Message ID | 20171101140316.31333-4-julien.grall@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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,
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);