| Message ID | 1464191801-1294-2-git-send-email-julien.grall@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 25.05.16 at 17:56, <julien.grall@arm.com> wrote: > --- a/xen/common/compat/memory.c > +++ b/xen/common/compat/memory.c > @@ -253,6 +253,8 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) > unsigned int size = cmp.atpb.size; > xen_ulong_t *idxs = (void *)(nat.atpb + 1); > xen_pfn_t *gpfns = (void *)(idxs + limit); > + enum XLAT_add_to_physmap_batch_u u = > + XLAT_add_to_physmap_batch_u_res0; Here you're cheating, and to help future readers understand you are you should say why this is okay in a comment. Or alternatively handle things properly. > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -259,7 +259,15 @@ struct xen_add_to_physmap_batch { > > /* Number of pages to go through */ > uint16_t size; > - domid_t foreign_domid; /* IFF gmfn_foreign */ > + > +#if __XEN_INTERFACE_VERSION__ < 0x00040700 > + domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other > spaces. */ > +#else > + union add_to_physmap_batch_extra { This lacks a xen_ prefix. Jan
Hi Jan, On 27/05/16 10:58, Jan Beulich wrote: >>>> On 25.05.16 at 17:56, <julien.grall@arm.com> wrote: >> --- a/xen/common/compat/memory.c >> +++ b/xen/common/compat/memory.c >> @@ -253,6 +253,8 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) >> unsigned int size = cmp.atpb.size; >> xen_ulong_t *idxs = (void *)(nat.atpb + 1); >> xen_pfn_t *gpfns = (void *)(idxs + limit); >> + enum XLAT_add_to_physmap_batch_u u = >> + XLAT_add_to_physmap_batch_u_res0; > > Here you're cheating, and to help future readers understand you are > you should say why this is okay in a comment. Or alternatively handle > things properly. Well, this is the case on other place having to convert union (see XENMEM_get_vnumainfo). So I though it was valid. I will add a comment here. > >> --- a/xen/include/public/memory.h >> +++ b/xen/include/public/memory.h >> @@ -259,7 +259,15 @@ struct xen_add_to_physmap_batch { >> >> /* Number of pages to go through */ >> uint16_t size; >> - domid_t foreign_domid; /* IFF gmfn_foreign */ >> + >> +#if __XEN_INTERFACE_VERSION__ < 0x00040700 >> + domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other >> spaces. */ >> +#else >> + union add_to_physmap_batch_extra { > > This lacks a xen_ prefix. I will fix it. Regards,
>>> On 27.05.16 at 17:16, <julien.grall@arm.com> wrote: > On 27/05/16 10:58, Jan Beulich wrote: >>>>> On 25.05.16 at 17:56, <julien.grall@arm.com> wrote: >>> --- a/xen/common/compat/memory.c >>> +++ b/xen/common/compat/memory.c >>> @@ -253,6 +253,8 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) >>> unsigned int size = cmp.atpb.size; >>> xen_ulong_t *idxs = (void *)(nat.atpb + 1); >>> xen_pfn_t *gpfns = (void *)(idxs + limit); >>> + enum XLAT_add_to_physmap_batch_u u = >>> + XLAT_add_to_physmap_batch_u_res0; >> >> Here you're cheating, and to help future readers understand you are >> you should say why this is okay in a comment. Or alternatively handle >> things properly. > > Well, this is the case on other place having to convert union (see > XENMEM_get_vnumainfo). So I though it was valid. It depends on context whether you can go this way or need to use switch(). Jan
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c index b46e23e..79f4310 100644 --- a/xen/arch/arm/mm.c +++ b/xen/arch/arm/mm.c @@ -1047,7 +1047,7 @@ void share_xen_page_with_privileged_guests( int xenmem_add_to_physmap_one( struct domain *d, unsigned int space, - domid_t foreign_domid, + union add_to_physmap_batch_extra extra, unsigned long idx, xen_pfn_t gpfn) { @@ -1103,7 +1103,8 @@ int xenmem_add_to_physmap_one( { struct domain *od; p2m_type_t p2mt; - od = rcu_lock_domain_by_any_id(foreign_domid); + + od = rcu_lock_domain_by_any_id(extra.foreign_domid); if ( od == NULL ) return -ESRCH; @@ -1143,6 +1144,10 @@ int xenmem_add_to_physmap_one( break; } case XENMAPSPACE_dev_mmio: + /* extra should be 0. Reserved for future use. */ + if ( extra.res0 ) + return -EOPNOTSUPP; + rc = map_dev_mmio_region(d, gpfn, 1, idx); return rc; diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 03a4d5b..0b67103 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4769,7 +4769,7 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p) int xenmem_add_to_physmap_one( struct domain *d, unsigned int space, - domid_t foreign_domid, + union add_to_physmap_batch_extra extra, unsigned long idx, xen_pfn_t gpfn) { @@ -4830,7 +4830,7 @@ int xenmem_add_to_physmap_one( break; } case XENMAPSPACE_gmfn_foreign: - return p2m_add_foreign(d, idx, gpfn, foreign_domid); + return p2m_add_foreign(d, idx, gpfn, extra.foreign_domid); default: break; } diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c index 19a914d..e56e187 100644 --- a/xen/common/compat/memory.c +++ b/xen/common/compat/memory.c @@ -253,6 +253,8 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) unsigned int size = cmp.atpb.size; xen_ulong_t *idxs = (void *)(nat.atpb + 1); xen_pfn_t *gpfns = (void *)(idxs + limit); + enum XLAT_add_to_physmap_batch_u u = + XLAT_add_to_physmap_batch_u_res0; if ( copy_from_guest(&cmp.atpb, compat, 1) || !compat_handle_okay(cmp.atpb.idxs, size) || diff --git a/xen/common/memory.c b/xen/common/memory.c index 644f81a..b1ac8b9 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -639,9 +639,15 @@ static int xenmem_add_to_physmap(struct domain *d, { unsigned int done = 0; long rc = 0; + union add_to_physmap_batch_extra extra; + + if ( xatp->space != XENMAPSPACE_gmfn_foreign ) + extra.res0 = 0; + else + extra.foreign_domid = DOMID_INVALID; if ( xatp->space != XENMAPSPACE_gmfn_range ) - return xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID, + return xenmem_add_to_physmap_one(d, xatp->space, extra, xatp->idx, xatp->gpfn); if ( xatp->size < start ) @@ -658,7 +664,7 @@ static int xenmem_add_to_physmap(struct domain *d, while ( xatp->size > done ) { - rc = xenmem_add_to_physmap_one(d, xatp->space, DOMID_INVALID, + rc = xenmem_add_to_physmap_one(d, xatp->space, extra, xatp->idx, xatp->gpfn); if ( rc < 0 ) break; @@ -719,7 +725,7 @@ static int xenmem_add_to_physmap_batch(struct domain *d, } rc = xenmem_add_to_physmap_one(d, xatpb->space, - xatpb->foreign_domid, + xatpb->u, idx, gpfn); if ( unlikely(__copy_to_guest_offset(xatpb->errs, 0, &rc, 1)) ) diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index fe52ee1..a5ab139 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -259,7 +259,15 @@ struct xen_add_to_physmap_batch { /* Number of pages to go through */ uint16_t size; - domid_t foreign_domid; /* IFF gmfn_foreign */ + +#if __XEN_INTERFACE_VERSION__ < 0x00040700 + domid_t foreign_domid; /* IFF gmfn_foreign. Should be 0 for other spaces. */ +#else + union add_to_physmap_batch_extra { + domid_t foreign_domid; /* gmfn_foreign */ + uint16_t res0; /* All the other spaces. Should be 0 */ + } u; +#endif /* Indexes into space being mapped. */ XEN_GUEST_HANDLE(xen_ulong_t) idxs; diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h index d4721fc..d82e0a4 100644 --- a/xen/include/xen/mm.h +++ b/xen/include/xen/mm.h @@ -50,6 +50,7 @@ #include <xen/list.h> #include <xen/spinlock.h> #include <xen/typesafe.h> +#include <public/memory.h> TYPE_SAFE(unsigned long, mfn); #define PRI_mfn "05lx" @@ -505,7 +506,7 @@ void scrub_one_page(struct page_info *); #endif int xenmem_add_to_physmap_one(struct domain *d, unsigned int space, - domid_t foreign_domid, + union add_to_physmap_batch_extra extra, unsigned long idx, xen_pfn_t gpfn); /* Returns 1 on success, 0 on error, negative if the ring
The field 'foreign_id' is not used when the space is dev_mmio. As the space is not yet part of the stable ABI, the field is marked as reserved for future use. The value should always be 0, other values will return -EOPNOTSUPP. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Changes in v2: - Return -EOPNOTSUPP rather than -ENOSYS - Introduce a union in the sturcutre xenmem_add_to_physmap_batch --- xen/arch/arm/mm.c | 9 +++++++-- xen/arch/x86/mm.c | 4 ++-- xen/common/compat/memory.c | 2 ++ xen/common/memory.c | 12 +++++++++--- xen/include/public/memory.h | 10 +++++++++- xen/include/xen/mm.h | 3 ++- 6 files changed, 31 insertions(+), 9 deletions(-)