| Message ID | 20171012162603.3016-6-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 12.10.17 at 18:25, <paul.durrant@citrix.com> wrote: > @@ -402,14 +469,56 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) > rc = do_memory_op(cmd, nat.hnd); > if ( rc < 0 ) > { > - if ( rc == -ENOBUFS && op == XENMEM_get_vnumainfo ) > + switch ( op) Missing blank. > { > - cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes; > - cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus; > - cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges; > - if ( __copy_to_guest(compat, &cmp.vnuma, 1) ) > - rc = -EFAULT; > + case XENMEM_get_vnumainfo: > + if ( rc == -ENOBUFS ) > + { > + cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes; > + cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus; > + cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges; > + if ( __copy_to_guest(compat, &cmp.vnuma, 1) ) > + rc = -EFAULT; > + } > + > + break; > + > + case XENMEM_acquire_resource: > + { > + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); const > + if ( rc == -EINVAL && xen_frame_list[0] != 0 ) I think this will go wrong if you get -EINVAL for other than the specific reason you consider here, in particular when caller passed in a valid array. You'd need to also check for cmp.mar.nr_frames being zero. But see also below. > + { > + /* > + * The value of nr_frames passed to the implementation > + * was not the value passed by the caller, it was > + * overridden. > + * The value in xen_frame_list[0] is the maximum > + * number of frames that can be bounced so we need > + * to set cmp.nr_frames to the minimum of this and > + * the maximum number of frames allowed by the > + * implementation before passing back to the caller. > + */ > + cmp.mar.nr_frames = min_t(unsigned int, > + xen_frame_list[0], > + nat.mar->nr_frames); > + rc = -E2BIG; > + } > + > + /* In either of these cases nr_frames is an OUT value */ > + if ( rc == -EINVAL || rc == -E2BIG ) > + { > + if ( copy_to_guest(compat, &cmp.mar, 1) ) > + rc = -EFAULT; The two if()s should be combined. Also - __copy_field_to_guest()? > + } > + > + break; > + } > + default: > + break; No real need for a default label. Yet if you want to keep it, please have a blank line ahead of it. > @@ -535,6 +644,30 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) > rc = -EFAULT; > break; > > + case XENMEM_acquire_resource: > + { > + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); const > + compat_ulong_t *compat_frame_list = > + (compat_ulong_t *)(nat.mar + 1); > + > + /* NOTE: the compat array overwrites the native array */ Perhaps "the smaller compat array ..."? > + for ( i = 0; i < cmp.mar.nr_frames; i++ ) > + { > + compat_ulong_t frame = xen_frame_list[i]; > + > + if ( frame != xen_frame_list[i] ) > + return -ERANGE; > + > + compat_frame_list[i] = frame; > + } > + > + if ( __copy_to_compat_offset(cmp.mar.frame_list, 0, > + compat_frame_list, > + cmp.mar.nr_frames) ) > + return -EFAULT; > + > + break; > + } > default: Again missing a blank line above here. > --- a/xen/common/memory.c > +++ b/xen/common/memory.c > @@ -965,6 +965,88 @@ static long xatp_permission_check(struct domain *d, unsigned int space) > return xsm_add_to_physmap(XSM_TARGET, current->domain, d); > } > > +static int acquire_resource(XEN_GUEST_HANDLE_PARAM(void) arg) > +{ > + struct domain *d, *currd = current->domain; > + xen_mem_acquire_resource_t xmar; > + unsigned long mfn_list[2]; > + int rc; > + > + if ( copy_from_guest(&xmar, arg, 1) ) > + return -EFAULT; > + > + if ( xmar.pad != 0 ) > + return -EINVAL; > + > + if ( xmar.nr_frames == 0 || > + xmar.nr_frames > ARRAY_SIZE(mfn_list) ) > + { > + rc = xmar.nr_frames == 0 ? -EINVAL : -E2BIG; Querying the implementation limit should be possible without receiving an error. Hence my original suggestion to key this off of the handle being a null one (in which case non-zero nr_frames would indeed be -EINVAL), which afaics would also simplify some of the compat handling. > + xmar.nr_frames = ARRAY_SIZE(mfn_list); > + > + if ( copy_to_guest(arg, &xmar, 1) ) > + return -EFAULT; > + > + return rc; > + } > + > + d = rcu_lock_domain_by_any_id(xmar.domid); > + if ( d == NULL ) > + return -ESRCH; > + > + rc = xsm_domain_resource_map(XSM_DM_PRIV, d); > + if ( rc ) > + goto out; > + > + switch ( xmar.type ) > + { > + default: > + rc = -EOPNOTSUPP; > + break; > + } > + > + if ( rc ) > + goto out; > + > + if ( !paging_mode_translate(currd) ) > + { > + if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) ) > + rc = -EFAULT; > + } > + else > + { > + xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)]; > + unsigned int i; > + > + rc = -EFAULT; > + if ( copy_from_guest(gfn_list, xmar.frame_list, > + ARRAY_SIZE(gfn_list)) ) You shouldn't copy more than xmar.nr_frames here, or else you risk running past a page boundary and perhaps into a non- present page. You consume ... > + goto out; > + > + for ( i = 0; i < xmar.nr_frames; i++ ) ... exactly this many frames anyway. > + { > + rc = set_foreign_p2m_entry(currd, gfn_list[i], > + _mfn(mfn_list[i])); > + if ( rc ) > + { > + while ( i-- != 0 ) > + { > + int ignore; > + > + ignore = guest_physmap_remove_page( > + currd, _gfn(gfn_list[i]), _mfn(mfn_list[i]), 0); Why would an error here be plain ignored? > @@ -1406,6 +1488,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) > } > #endif > > + case XENMEM_acquire_resource: > +#ifdef CONFIG_X86 > + rc = acquire_resource(arg); > +#else > + rc = -EOPNOTSUPP; > +#endif I think this will cause an "unused static function" warning on ARM. > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -599,6 +599,47 @@ struct xen_reserved_device_memory_map { > typedef struct xen_reserved_device_memory_map xen_reserved_device_memory_map_t; > DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t); > > +/* > + * Get the pages for a particular guest resource, so that they can be > + * mapped directly by a tools domain. > + */ > +#define XENMEM_acquire_resource 28 > +struct xen_mem_acquire_resource { > + /* IN - the domain whose resource is to be mapped */ > + domid_t domid; > + /* IN - the type of resource */ > + uint16_t type; > + /* > + * IN - a type-specific resource identifier, which must be zero > + * unless stated otherwise. > + */ > + uint32_t id; > + /* IN/OUT - As an IN parameter number of (4K) frames of the resource Please don't say 4k here - this not being an x86-specific interface other system page sizes ought to be permitted. > + * to be mapped. However, if the specified value is 0 then > + * -EINVAL will be returned and this field will be set to the > + * maximum value supported by the implementation. Also, > + * if the specified value exceeds the implementaton limit > + * then -E2BIG will be returned and, similarly, this field > + * will be set the maximum value supported by the > + * implementation. > + */ > + uint32_t nr_frames; > + uint32_t pad; > + /* IN - the index of the initial frame to be mapped. This parameter > + * is optional if nr_frames is 0. > + */ > + uint64_aligned_t frame; > + /* IN/OUT - If the tools domain is PV then, upon return, frame_list > + * will be populated with the MFNs of the resource. > + * If the tools domain is HVM then it is expected that, on > + * entry, frame_list will be populated with a list of GFNs > + * that will be mapped to the MFNs of the resource. > + * This parameter is optional if nr_frames is 0. > + */ For both of these comments - s/optional/ignored/? And this, afaics, also applies to domid, type, and id, so perhaps better state once in the comment to (currently) nr_frames that all other fields except for pad will be ignored. > --- a/xen/include/xsm/dummy.h > +++ b/xen/include/xsm/dummy.h > @@ -724,3 +724,9 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op) > return xsm_default_action(XSM_PRIV, current->domain, NULL); > } > } > + > +static XSM_INLINE int xsm_domain_resource_map(XSM_DEFAULT_ARG struct domain *d) > +{ > + XSM_ASSERT_ACTION(XSM_DM_PRIV); > + return xsm_default_action(action, current->domain, d); > +} Perhaps better place this near something similar/related (also for some of the other additions further down)? Jan
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 16 October 2017 14:53 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Wei Liu > <wei.liu2@citrix.com>; George Dunlap <George.Dunlap@citrix.com>; Ian > Jackson <Ian.Jackson@citrix.com>; Stefano Stabellini > <sstabellini@kernel.org>; xen-devel@lists.xenproject.org; Konrad Rzeszutek > Wilk <konrad.wilk@oracle.com>; Tim (Xen.org) <tim@xen.org> > Subject: Re: [PATCH v11 05/11] x86/mm: add HYPERVISOR_memory_op to > acquire guest resources > > >>> On 12.10.17 at 18:25, <paul.durrant@citrix.com> wrote: > > @@ -402,14 +469,56 @@ int compat_memory_op(unsigned int cmd, > XEN_GUEST_HANDLE_PARAM(void) compat) > > rc = do_memory_op(cmd, nat.hnd); > > if ( rc < 0 ) > > { > > - if ( rc == -ENOBUFS && op == XENMEM_get_vnumainfo ) > > + switch ( op) > > Missing blank. Oh yes. > > > { > > - cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes; > > - cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus; > > - cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges; > > - if ( __copy_to_guest(compat, &cmp.vnuma, 1) ) > > - rc = -EFAULT; > > + case XENMEM_get_vnumainfo: > > + if ( rc == -ENOBUFS ) > > + { > > + cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes; > > + cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus; > > + cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges; > > + if ( __copy_to_guest(compat, &cmp.vnuma, 1) ) > > + rc = -EFAULT; > > + } > > + > > + break; > > + > > + case XENMEM_acquire_resource: > > + { > > + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); > > const Ok. > > > + if ( rc == -EINVAL && xen_frame_list[0] != 0 ) > > I think this will go wrong if you get -EINVAL for other than the > specific reason you consider here, in particular when caller > passed in a valid array. You'd need to also check for > cmp.mar.nr_frames being zero. But see also below. > > > + { > > + /* > > + * The value of nr_frames passed to the implementation > > + * was not the value passed by the caller, it was > > + * overridden. > > + * The value in xen_frame_list[0] is the maximum > > + * number of frames that can be bounced so we need > > + * to set cmp.nr_frames to the minimum of this and > > + * the maximum number of frames allowed by the > > + * implementation before passing back to the caller. > > + */ > > + cmp.mar.nr_frames = min_t(unsigned int, > > + xen_frame_list[0], > > + nat.mar->nr_frames); > > + rc = -E2BIG; > > + } > > + > > + /* In either of these cases nr_frames is an OUT value */ > > + if ( rc == -EINVAL || rc == -E2BIG ) > > + { > > + if ( copy_to_guest(compat, &cmp.mar, 1) ) > > + rc = -EFAULT; > > The two if()s should be combined. Also - __copy_field_to_guest()? Yes, maybe that would neater. > > > + } > > + > > + break; > > + } > > + default: > > + break; > > No real need for a default label. Yet if you want to keep it, please > have a blank line ahead of it. > Ok. > > @@ -535,6 +644,30 @@ int compat_memory_op(unsigned int cmd, > XEN_GUEST_HANDLE_PARAM(void) compat) > > rc = -EFAULT; > > break; > > > > + case XENMEM_acquire_resource: > > + { > > + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); > > const Ok. > > > + compat_ulong_t *compat_frame_list = > > + (compat_ulong_t *)(nat.mar + 1); > > + > > + /* NOTE: the compat array overwrites the native array */ > > Perhaps "the smaller compat array ..."? Ok. > > > + for ( i = 0; i < cmp.mar.nr_frames; i++ ) > > + { > > + compat_ulong_t frame = xen_frame_list[i]; > > + > > + if ( frame != xen_frame_list[i] ) > > + return -ERANGE; > > + > > + compat_frame_list[i] = frame; > > + } > > + > > + if ( __copy_to_compat_offset(cmp.mar.frame_list, 0, > > + compat_frame_list, > > + cmp.mar.nr_frames) ) > > + return -EFAULT; > > + > > + break; > > + } > > default: > > Again missing a blank line above here. Ok. > > > --- a/xen/common/memory.c > > +++ b/xen/common/memory.c > > @@ -965,6 +965,88 @@ static long xatp_permission_check(struct domain > *d, unsigned int space) > > return xsm_add_to_physmap(XSM_TARGET, current->domain, d); > > } > > > > +static int acquire_resource(XEN_GUEST_HANDLE_PARAM(void) arg) > > +{ > > + struct domain *d, *currd = current->domain; > > + xen_mem_acquire_resource_t xmar; > > + unsigned long mfn_list[2]; > > + int rc; > > + > > + if ( copy_from_guest(&xmar, arg, 1) ) > > + return -EFAULT; > > + > > + if ( xmar.pad != 0 ) > > + return -EINVAL; > > + > > + if ( xmar.nr_frames == 0 || > > + xmar.nr_frames > ARRAY_SIZE(mfn_list) ) > > + { > > + rc = xmar.nr_frames == 0 ? -EINVAL : -E2BIG; > > Querying the implementation limit should be possible without > receiving an error. Hence my original suggestion to key this > off of the handle being a null one (in which case non-zero > nr_frames would indeed be -EINVAL), which afaics would also > simplify some of the compat handling. Ok, FAOD, do you mean that passing in nr_frames and a NULL handle should not yield an error but should pass back the implementation limit of nr_frames? > > > + xmar.nr_frames = ARRAY_SIZE(mfn_list); > > + > > + if ( copy_to_guest(arg, &xmar, 1) ) > > + return -EFAULT; > > + > > + return rc; > > + } > > + > > + d = rcu_lock_domain_by_any_id(xmar.domid); > > + if ( d == NULL ) > > + return -ESRCH; > > + > > + rc = xsm_domain_resource_map(XSM_DM_PRIV, d); > > + if ( rc ) > > + goto out; > > + > > + switch ( xmar.type ) > > + { > > + default: > > + rc = -EOPNOTSUPP; > > + break; > > + } > > + > > + if ( rc ) > > + goto out; > > + > > + if ( !paging_mode_translate(currd) ) > > + { > > + if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) ) > > + rc = -EFAULT; > > + } > > + else > > + { > > + xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)]; > > + unsigned int i; > > + > > + rc = -EFAULT; > > + if ( copy_from_guest(gfn_list, xmar.frame_list, > > + ARRAY_SIZE(gfn_list)) ) > > You shouldn't copy more than xmar.nr_frames here, or else you > risk running past a page boundary and perhaps into a non- > present page. Good point. That's clearly wrong. > You consume ... > > > + goto out; > > + > > + for ( i = 0; i < xmar.nr_frames; i++ ) > > ... exactly this many frames anyway. > > > + { > > + rc = set_foreign_p2m_entry(currd, gfn_list[i], > > + _mfn(mfn_list[i])); > > + if ( rc ) > > + { > > + while ( i-- != 0 ) > > + { > > + int ignore; > > + > > + ignore = guest_physmap_remove_page( > > + currd, _gfn(gfn_list[i]), _mfn(mfn_list[i]), 0); > > Why would an error here be plain ignored? > What could I usefully do with it? Should I just crash the domain at this point, since I can't restore a consistent state? > > @@ -1406,6 +1488,14 @@ long do_memory_op(unsigned long cmd, > XEN_GUEST_HANDLE_PARAM(void) arg) > > } > > #endif > > > > + case XENMEM_acquire_resource: > > +#ifdef CONFIG_X86 > > + rc = acquire_resource(arg); > > +#else > > + rc = -EOPNOTSUPP; > > +#endif > > I think this will cause an "unused static function" warning on ARM. ...which is why I originally had the function wrapped in the #ifdef as well. What do you want me to do? > > > --- a/xen/include/public/memory.h > > +++ b/xen/include/public/memory.h > > @@ -599,6 +599,47 @@ struct xen_reserved_device_memory_map { > > typedef struct xen_reserved_device_memory_map > xen_reserved_device_memory_map_t; > > DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t); > > > > +/* > > + * Get the pages for a particular guest resource, so that they can be > > + * mapped directly by a tools domain. > > + */ > > +#define XENMEM_acquire_resource 28 > > +struct xen_mem_acquire_resource { > > + /* IN - the domain whose resource is to be mapped */ > > + domid_t domid; > > + /* IN - the type of resource */ > > + uint16_t type; > > + /* > > + * IN - a type-specific resource identifier, which must be zero > > + * unless stated otherwise. > > + */ > > + uint32_t id; > > + /* IN/OUT - As an IN parameter number of (4K) frames of the resource > > Please don't say 4k here - this not being an x86-specific interface > other system page sizes ought to be permitted. I was under the impression that resources such as grant tables were only every mapped in 4k chunks. Perhaps the 4k should type-specific? It needs to be specified somewhere. > > > + * to be mapped. However, if the specified value is 0 then > > + * -EINVAL will be returned and this field will be set to the > > + * maximum value supported by the implementation. Also, > > + * if the specified value exceeds the implementaton limit > > + * then -E2BIG will be returned and, similarly, this field > > + * will be set the maximum value supported by the > > + * implementation. > > + */ > > + uint32_t nr_frames; > > + uint32_t pad; > > + /* IN - the index of the initial frame to be mapped. This parameter > > + * is optional if nr_frames is 0. > > + */ > > + uint64_aligned_t frame; > > + /* IN/OUT - If the tools domain is PV then, upon return, frame_list > > + * will be populated with the MFNs of the resource. > > + * If the tools domain is HVM then it is expected that, on > > + * entry, frame_list will be populated with a list of GFNs > > + * that will be mapped to the MFNs of the resource. > > + * This parameter is optional if nr_frames is 0. > > + */ > > For both of these comments - s/optional/ignored/? And this, afaics, > also applies to domid, type, and id, so perhaps better state once > in the comment to (currently) nr_frames that all other fields except > for pad will be ignored. No, I think it's reasonable for domid, type and id to always be valid even if nr_frames is zero. > > > --- a/xen/include/xsm/dummy.h > > +++ b/xen/include/xsm/dummy.h > > @@ -724,3 +724,9 @@ static XSM_INLINE int xsm_xen_version > (XSM_DEFAULT_ARG uint32_t op) > > return xsm_default_action(XSM_PRIV, current->domain, NULL); > > } > > } > > + > > +static XSM_INLINE int xsm_domain_resource_map(XSM_DEFAULT_ARG > struct domain *d) > > +{ > > + XSM_ASSERT_ACTION(XSM_DM_PRIV); > > + return xsm_default_action(action, current->domain, d); > > +} > > Perhaps better place this near something similar/related (also for > some of the other additions further down)? Ok. Paul > > Jan
>>> On 16.10.17 at 16:07, <Paul.Durrant@citrix.com> wrote: >> From: Jan Beulich [mailto:JBeulich@suse.com] >> Sent: 16 October 2017 14:53 >> >>> On 12.10.17 at 18:25, <paul.durrant@citrix.com> wrote: >> > --- a/xen/common/memory.c >> > +++ b/xen/common/memory.c >> > @@ -965,6 +965,88 @@ static long xatp_permission_check(struct domain >> *d, unsigned int space) >> > return xsm_add_to_physmap(XSM_TARGET, current->domain, d); >> > } >> > >> > +static int acquire_resource(XEN_GUEST_HANDLE_PARAM(void) arg) >> > +{ >> > + struct domain *d, *currd = current->domain; >> > + xen_mem_acquire_resource_t xmar; >> > + unsigned long mfn_list[2]; >> > + int rc; >> > + >> > + if ( copy_from_guest(&xmar, arg, 1) ) >> > + return -EFAULT; >> > + >> > + if ( xmar.pad != 0 ) >> > + return -EINVAL; >> > + >> > + if ( xmar.nr_frames == 0 || >> > + xmar.nr_frames > ARRAY_SIZE(mfn_list) ) >> > + { >> > + rc = xmar.nr_frames == 0 ? -EINVAL : -E2BIG; >> >> Querying the implementation limit should be possible without >> receiving an error. Hence my original suggestion to key this >> off of the handle being a null one (in which case non-zero >> nr_frames would indeed be -EINVAL), which afaics would also >> simplify some of the compat handling. > > Ok, FAOD, do you mean that passing in nr_frames and a NULL handle should not > yield an error but should pass back the implementation limit of nr_frames? If you mean "passing in zero nr_frames and a null handle", then yes. Non-zero nr_frames and a null handle, as said, could certainly be -EINVAL (you could as well try to read/write from/to that handle, but I think Andrew wouldn't like that). >> > + { >> > + rc = set_foreign_p2m_entry(currd, gfn_list[i], >> > + _mfn(mfn_list[i])); >> > + if ( rc ) >> > + { >> > + while ( i-- != 0 ) >> > + { >> > + int ignore; >> > + >> > + ignore = guest_physmap_remove_page( >> > + currd, _gfn(gfn_list[i]), _mfn(mfn_list[i]), 0); >> >> Why would an error here be plain ignored? >> > > What could I usefully do with it? Should I just crash the domain at this > point, since I can't restore a consistent state? Not being silent is the most important aspect. Crashing the domain is one approach. Reporting the error (and documenting the possibly resulting inconsistent state) is another (and a sub-option thereof is to return the number of failed entries, perhaps allowing the caller some way to recover). Plus note that if the error happens on the first iteration, no inconsistency would result. >> > @@ -1406,6 +1488,14 @@ long do_memory_op(unsigned long cmd, >> XEN_GUEST_HANDLE_PARAM(void) arg) >> > } >> > #endif >> > >> > + case XENMEM_acquire_resource: >> > +#ifdef CONFIG_X86 >> > + rc = acquire_resource(arg); >> > +#else >> > + rc = -EOPNOTSUPP; >> > +#endif >> >> I think this will cause an "unused static function" warning on ARM. > > ...which is why I originally had the function wrapped in the #ifdef as well. > What do you want me to do? As said before - I'd like to see the #ifdef placed inside the function around the smallest possible range of code that still allows ARM to build (with the alternative of introducing a dummy stub or two on ARM to avoid #ifdef-s altogether). >> > --- a/xen/include/public/memory.h >> > +++ b/xen/include/public/memory.h >> > @@ -599,6 +599,47 @@ struct xen_reserved_device_memory_map { >> > typedef struct xen_reserved_device_memory_map >> xen_reserved_device_memory_map_t; >> > DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t); >> > >> > +/* >> > + * Get the pages for a particular guest resource, so that they can be >> > + * mapped directly by a tools domain. >> > + */ >> > +#define XENMEM_acquire_resource 28 >> > +struct xen_mem_acquire_resource { >> > + /* IN - the domain whose resource is to be mapped */ >> > + domid_t domid; >> > + /* IN - the type of resource */ >> > + uint16_t type; >> > + /* >> > + * IN - a type-specific resource identifier, which must be zero >> > + * unless stated otherwise. >> > + */ >> > + uint32_t id; >> > + /* IN/OUT - As an IN parameter number of (4K) frames of the resource >> >> Please don't say 4k here - this not being an x86-specific interface >> other system page sizes ought to be permitted. > > I was under the impression that resources such as grant tables were only > every mapped in 4k chunks. Perhaps the 4k should type-specific? It needs to be > specified somewhere. I don't think there's an inherent limit to granted pages being larger than 4k; v2 sub-page grants limit things to less than 64k though. There's no single mention of "4k" throughout the public grant_table.h or the implementation in grant_table.c. >> > + * to be mapped. However, if the specified value is 0 then >> > + * -EINVAL will be returned and this field will be set to the >> > + * maximum value supported by the implementation. Also, >> > + * if the specified value exceeds the implementaton limit >> > + * then -E2BIG will be returned and, similarly, this field >> > + * will be set the maximum value supported by the >> > + * implementation. >> > + */ >> > + uint32_t nr_frames; >> > + uint32_t pad; >> > + /* IN - the index of the initial frame to be mapped. This parameter >> > + * is optional if nr_frames is 0. >> > + */ >> > + uint64_aligned_t frame; >> > + /* IN/OUT - If the tools domain is PV then, upon return, frame_list >> > + * will be populated with the MFNs of the resource. >> > + * If the tools domain is HVM then it is expected that, on >> > + * entry, frame_list will be populated with a list of GFNs >> > + * that will be mapped to the MFNs of the resource. >> > + * This parameter is optional if nr_frames is 0. >> > + */ >> >> For both of these comments - s/optional/ignored/? And this, afaics, >> also applies to domid, type, and id, so perhaps better state once >> in the comment to (currently) nr_frames that all other fields except >> for pad will be ignored. > > No, I think it's reasonable for domid, type and id to always be valid even > if nr_frames is zero. Okay, I can see why, but then you also need to check all of them. Jan
> -----Original Message----- > > > --- a/xen/include/xsm/dummy.h > > +++ b/xen/include/xsm/dummy.h > > @@ -724,3 +724,9 @@ static XSM_INLINE int xsm_xen_version > (XSM_DEFAULT_ARG uint32_t op) > > return xsm_default_action(XSM_PRIV, current->domain, NULL); > > } > > } > > + > > +static XSM_INLINE int xsm_domain_resource_map(XSM_DEFAULT_ARG > struct domain *d) > > +{ > > + XSM_ASSERT_ACTION(XSM_DM_PRIV); > > + return xsm_default_action(action, current->domain, d); > > +} > > Perhaps better place this near something similar/related (also for > some of the other additions further down)? Looking at this again it seems that various related things, e.g. domain_memory_map, are x86 only so adding at the end seems like the best thing to do. Paul > > Jan
>>> On 17.10.17 at 14:28, <Paul.Durrant@citrix.com> wrote: >> -----Original Message----- >> >> > --- a/xen/include/xsm/dummy.h >> > +++ b/xen/include/xsm/dummy.h >> > @@ -724,3 +724,9 @@ static XSM_INLINE int xsm_xen_version >> (XSM_DEFAULT_ARG uint32_t op) >> > return xsm_default_action(XSM_PRIV, current->domain, NULL); >> > } >> > } >> > + >> > +static XSM_INLINE int xsm_domain_resource_map(XSM_DEFAULT_ARG >> struct domain *d) >> > +{ >> > + XSM_ASSERT_ACTION(XSM_DM_PRIV); >> > + return xsm_default_action(action, current->domain, d); >> > +} >> >> Perhaps better place this near something similar/related (also for >> some of the other additions further down)? > > Looking at this again it seems that various related things, e.g. > domain_memory_map, are x86 only so adding at the end seems like the best > thing to do. Well, okay then (unless Daniel, whom it looks like you forgot to Cc, has a better suggestion). Jan
> -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 17 October 2017 13:53 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; George Dunlap > <George.Dunlap@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Wei Liu > <wei.liu2@citrix.com>; Stefano Stabellini <sstabellini@kernel.org>; xen- > devel@lists.xenproject.org; KonradRzeszutek Wilk > <konrad.wilk@oracle.com>; Daniel de Graaf <dgdegra@tycho.nsa.gov>; Tim > (Xen.org) <tim@xen.org> > Subject: RE: [PATCH v11 05/11] x86/mm: add HYPERVISOR_memory_op to > acquire guest resources > > >>> On 17.10.17 at 14:28, <Paul.Durrant@citrix.com> wrote: > >> -----Original Message----- > >> > >> > --- a/xen/include/xsm/dummy.h > >> > +++ b/xen/include/xsm/dummy.h > >> > @@ -724,3 +724,9 @@ static XSM_INLINE int xsm_xen_version > >> (XSM_DEFAULT_ARG uint32_t op) > >> > return xsm_default_action(XSM_PRIV, current->domain, NULL); > >> > } > >> > } > >> > + > >> > +static XSM_INLINE int > xsm_domain_resource_map(XSM_DEFAULT_ARG > >> struct domain *d) > >> > +{ > >> > + XSM_ASSERT_ACTION(XSM_DM_PRIV); > >> > + return xsm_default_action(action, current->domain, d); > >> > +} > >> > >> Perhaps better place this near something similar/related (also for > >> some of the other additions further down)? > > > > Looking at this again it seems that various related things, e.g. > > domain_memory_map, are x86 only so adding at the end seems like the > best > > thing to do. > > Well, okay then (unless Daniel, whom it looks like you forgot to Cc, > has a better suggestion). > Yes, I realised that I forgot to cc him in since the code was added. He's on the v12 list. Paul > Jan
diff --git a/tools/flask/policy/modules/xen.if b/tools/flask/policy/modules/xen.if index 55437496f6..07cba8a15d 100644 --- a/tools/flask/policy/modules/xen.if +++ b/tools/flask/policy/modules/xen.if @@ -52,7 +52,8 @@ define(`create_domain_common', ` settime setdomainhandle getvcpucontext set_misc_info }; allow $1 $2:domain2 { set_cpuid settsc setscheduler setclaim set_max_evtchn set_vnumainfo get_vnumainfo cacheflush - psr_cmt_op psr_cat_op soft_reset set_gnttab_limits }; + psr_cmt_op psr_cat_op soft_reset set_gnttab_limits + resource_map }; allow $1 $2:security check_context; allow $1 $2:shadow enable; allow $1 $2:mmu { map_read map_write adjust memorymap physmap pinpage mmuext_op updatemp }; @@ -152,6 +153,7 @@ define(`device_model', ` allow $1 $2_target:domain { getdomaininfo shutdown }; allow $1 $2_target:mmu { map_read map_write adjust physmap target_hack }; allow $1 $2_target:hvm { getparam setparam hvmctl cacheattr dm }; + allow $1 $2_target:domain2 resource_map; ') # make_device_model(priv, dm_dom, hvm_dom) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index c72a3cdebb..71bb9b4f93 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -1132,8 +1132,7 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn_l, } /* Set foreign mfn in the given guest's p2m table. */ -static int set_foreign_p2m_entry(struct domain *d, unsigned long gfn, - mfn_t mfn) +int set_foreign_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn) { return set_typed_p2m_entry(d, gfn, mfn, PAGE_ORDER_4K, p2m_map_foreign, p2m_get_hostp2m(d)->default_access); diff --git a/xen/common/compat/memory.c b/xen/common/compat/memory.c index 35bb259808..031d1a48ae 100644 --- a/xen/common/compat/memory.c +++ b/xen/common/compat/memory.c @@ -71,6 +71,7 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) struct xen_remove_from_physmap *xrfp; struct xen_vnuma_topology_info *vnuma; struct xen_mem_access_op *mao; + struct xen_mem_acquire_resource *mar; } nat; union { struct compat_memory_reservation rsrv; @@ -79,6 +80,7 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) struct compat_add_to_physmap_batch atpb; struct compat_vnuma_topology_info vnuma; struct compat_mem_access_op mao; + struct compat_mem_acquire_resource mar; } cmp; set_xen_guest_handle(nat.hnd, COMPAT_ARG_XLAT_VIRT_BASE); @@ -395,6 +397,71 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) } #endif + case XENMEM_acquire_resource: + { + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); + unsigned int max_nr_frames; + + if ( copy_from_guest(&cmp.mar, compat, 1) || + !compat_handle_okay(cmp.mar.frame_list, + cmp.mar.nr_frames) ) + return -EFAULT; + + /* + * The number of frames handled is currently limited to a + * small number by the underlying implementation, so the + * scratch space should be sufficient for bouncing the + * frame addresses. + */ + max_nr_frames = (COMPAT_ARG_XLAT_SIZE - sizeof(*nat.mar)) / + sizeof(*xen_frame_list); + + /* If the number of frames does exceed the maximum that can + * be bounced then -E2BIG is returned, but to satisfy the + * semantics of the hypercall then we need to get the + * maximum number of frames support by the implementation. + * This can be done by set nr_frames to zero. + */ + if ( cmp.mar.nr_frames > max_nr_frames ) + { + cmp.mar.nr_frames = 0; + + /* + * Stash the maximum number of frames that can be bounced + * in xen_frame_list[0]. It is needed in the return path. + */ + xen_frame_list[0] = max_nr_frames; + } + else if ( cmp.mar.nr_frames == 0 ) + { + /* + * Make sure that xen_frame_list[0] is zero in the case + * when the caller set nr_frames to zero, so that we + * can distinguish this case from the above on return. + */ + xen_frame_list[0] = 0; + } + + for ( i = 0; i < cmp.mar.nr_frames; i++ ) + { + compat_ulong_t frame; + + if ( __copy_from_compat_offset(&frame, cmp.mar.frame_list, + i, 1) ) + return -EFAULT; + + xen_frame_list[i] = frame; + } + +#define XLAT_mem_acquire_resource_HNDL_frame_list(_d_, _s_) \ + set_xen_guest_handle((_d_)->frame_list, xen_frame_list) + + XLAT_mem_acquire_resource(nat.mar, &cmp.mar); + +#undef XLAT_mem_acquire_resource_HNDL_frame_list + + break; + } default: return compat_arch_memory_op(cmd, compat); } @@ -402,14 +469,56 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) rc = do_memory_op(cmd, nat.hnd); if ( rc < 0 ) { - if ( rc == -ENOBUFS && op == XENMEM_get_vnumainfo ) + switch ( op) { - cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes; - cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus; - cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges; - if ( __copy_to_guest(compat, &cmp.vnuma, 1) ) - rc = -EFAULT; + case XENMEM_get_vnumainfo: + if ( rc == -ENOBUFS ) + { + cmp.vnuma.nr_vnodes = nat.vnuma->nr_vnodes; + cmp.vnuma.nr_vcpus = nat.vnuma->nr_vcpus; + cmp.vnuma.nr_vmemranges = nat.vnuma->nr_vmemranges; + if ( __copy_to_guest(compat, &cmp.vnuma, 1) ) + rc = -EFAULT; + } + + break; + + case XENMEM_acquire_resource: + { + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); + + if ( rc == -EINVAL && xen_frame_list[0] != 0 ) + { + /* + * The value of nr_frames passed to the implementation + * was not the value passed by the caller, it was + * overridden. + * The value in xen_frame_list[0] is the maximum + * number of frames that can be bounced so we need + * to set cmp.nr_frames to the minimum of this and + * the maximum number of frames allowed by the + * implementation before passing back to the caller. + */ + cmp.mar.nr_frames = min_t(unsigned int, + xen_frame_list[0], + nat.mar->nr_frames); + rc = -E2BIG; + } + + /* In either of these cases nr_frames is an OUT value */ + if ( rc == -EINVAL || rc == -E2BIG ) + { + if ( copy_to_guest(compat, &cmp.mar, 1) ) + rc = -EFAULT; + } + + break; + } + default: + break; } + + /* Break out of the do loop */ break; } @@ -535,6 +644,30 @@ int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat) rc = -EFAULT; break; + case XENMEM_acquire_resource: + { + xen_ulong_t *xen_frame_list = (xen_ulong_t *)(nat.mar + 1); + compat_ulong_t *compat_frame_list = + (compat_ulong_t *)(nat.mar + 1); + + /* NOTE: the compat array overwrites the native array */ + for ( i = 0; i < cmp.mar.nr_frames; i++ ) + { + compat_ulong_t frame = xen_frame_list[i]; + + if ( frame != xen_frame_list[i] ) + return -ERANGE; + + compat_frame_list[i] = frame; + } + + if ( __copy_to_compat_offset(cmp.mar.frame_list, 0, + compat_frame_list, + cmp.mar.nr_frames) ) + return -EFAULT; + + break; + } default: domain_crash(current->domain); split = 0; diff --git a/xen/common/memory.c b/xen/common/memory.c index ad987e0f29..a88fc83565 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -965,6 +965,88 @@ static long xatp_permission_check(struct domain *d, unsigned int space) return xsm_add_to_physmap(XSM_TARGET, current->domain, d); } +static int acquire_resource(XEN_GUEST_HANDLE_PARAM(void) arg) +{ + struct domain *d, *currd = current->domain; + xen_mem_acquire_resource_t xmar; + unsigned long mfn_list[2]; + int rc; + + if ( copy_from_guest(&xmar, arg, 1) ) + return -EFAULT; + + if ( xmar.pad != 0 ) + return -EINVAL; + + if ( xmar.nr_frames == 0 || + xmar.nr_frames > ARRAY_SIZE(mfn_list) ) + { + rc = xmar.nr_frames == 0 ? -EINVAL : -E2BIG; + xmar.nr_frames = ARRAY_SIZE(mfn_list); + + if ( copy_to_guest(arg, &xmar, 1) ) + return -EFAULT; + + return rc; + } + + d = rcu_lock_domain_by_any_id(xmar.domid); + if ( d == NULL ) + return -ESRCH; + + rc = xsm_domain_resource_map(XSM_DM_PRIV, d); + if ( rc ) + goto out; + + switch ( xmar.type ) + { + default: + rc = -EOPNOTSUPP; + break; + } + + if ( rc ) + goto out; + + if ( !paging_mode_translate(currd) ) + { + if ( copy_to_guest(xmar.frame_list, mfn_list, xmar.nr_frames) ) + rc = -EFAULT; + } + else + { + xen_pfn_t gfn_list[ARRAY_SIZE(mfn_list)]; + unsigned int i; + + rc = -EFAULT; + if ( copy_from_guest(gfn_list, xmar.frame_list, + ARRAY_SIZE(gfn_list)) ) + goto out; + + for ( i = 0; i < xmar.nr_frames; i++ ) + { + rc = set_foreign_p2m_entry(currd, gfn_list[i], + _mfn(mfn_list[i])); + if ( rc ) + { + while ( i-- != 0 ) + { + int ignore; + + ignore = guest_physmap_remove_page( + currd, _gfn(gfn_list[i]), _mfn(mfn_list[i]), 0); + } + + goto out; + } + } + } + + out: + rcu_unlock_domain(d); + return rc; +} + long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) { struct domain *d, *curr_d = current->domain; @@ -1406,6 +1488,14 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) } #endif + case XENMEM_acquire_resource: +#ifdef CONFIG_X86 + rc = acquire_resource(arg); +#else + rc = -EOPNOTSUPP; +#endif + break; + default: rc = arch_memory_op(cmd, arg); break; diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index 17b1d0c8d3..44f7ec088c 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -620,6 +620,9 @@ void p2m_memory_type_changed(struct domain *d); int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start, unsigned long end); +/* Set foreign entry in the p2m table (for priv-mapping) */ +int set_foreign_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn); + /* Set mmio addresses in the p2m table (for pass-through) */ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn, unsigned int order, p2m_access_t access); diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h index 29386df98b..b7cf753d75 100644 --- a/xen/include/public/memory.h +++ b/xen/include/public/memory.h @@ -599,6 +599,47 @@ struct xen_reserved_device_memory_map { typedef struct xen_reserved_device_memory_map xen_reserved_device_memory_map_t; DEFINE_XEN_GUEST_HANDLE(xen_reserved_device_memory_map_t); +/* + * Get the pages for a particular guest resource, so that they can be + * mapped directly by a tools domain. + */ +#define XENMEM_acquire_resource 28 +struct xen_mem_acquire_resource { + /* IN - the domain whose resource is to be mapped */ + domid_t domid; + /* IN - the type of resource */ + uint16_t type; + /* + * IN - a type-specific resource identifier, which must be zero + * unless stated otherwise. + */ + uint32_t id; + /* IN/OUT - As an IN parameter number of (4K) frames of the resource + * to be mapped. However, if the specified value is 0 then + * -EINVAL will be returned and this field will be set to the + * maximum value supported by the implementation. Also, + * if the specified value exceeds the implementaton limit + * then -E2BIG will be returned and, similarly, this field + * will be set the maximum value supported by the + * implementation. + */ + uint32_t nr_frames; + uint32_t pad; + /* IN - the index of the initial frame to be mapped. This parameter + * is optional if nr_frames is 0. + */ + uint64_aligned_t frame; + /* IN/OUT - If the tools domain is PV then, upon return, frame_list + * will be populated with the MFNs of the resource. + * If the tools domain is HVM then it is expected that, on + * entry, frame_list will be populated with a list of GFNs + * that will be mapped to the MFNs of the resource. + * This parameter is optional if nr_frames is 0. + */ + XEN_GUEST_HANDLE(xen_ulong_t) frame_list; +}; +typedef struct xen_mem_acquire_resource xen_mem_acquire_resource_t; + #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */ /* @@ -650,7 +691,7 @@ struct xen_vnuma_topology_info { typedef struct xen_vnuma_topology_info xen_vnuma_topology_info_t; DEFINE_XEN_GUEST_HANDLE(xen_vnuma_topology_info_t); -/* Next available subop number is 28 */ +/* Next available subop number is 29 */ #endif /* __XEN_PUBLIC_MEMORY_H__ */ diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst index 0f17000ea7..5835872334 100644 --- a/xen/include/xlat.lst +++ b/xen/include/xlat.lst @@ -83,6 +83,7 @@ ! memory_map memory.h ! memory_reservation memory.h ! mem_access_op memory.h +! mem_acquire_resource memory.h ! pod_target memory.h ! remove_from_physmap memory.h ! reserved_device_memory_map memory.h diff --git a/xen/include/xsm/dummy.h b/xen/include/xsm/dummy.h index ba89ea4bc1..8f04d59a3e 100644 --- a/xen/include/xsm/dummy.h +++ b/xen/include/xsm/dummy.h @@ -724,3 +724,9 @@ static XSM_INLINE int xsm_xen_version (XSM_DEFAULT_ARG uint32_t op) return xsm_default_action(XSM_PRIV, current->domain, NULL); } } + +static XSM_INLINE int xsm_domain_resource_map(XSM_DEFAULT_ARG struct domain *d) +{ + XSM_ASSERT_ACTION(XSM_DM_PRIV); + return xsm_default_action(action, current->domain, d); +} diff --git a/xen/include/xsm/xsm.h b/xen/include/xsm/xsm.h index 7f7feffc68..d0db860ae0 100644 --- a/xen/include/xsm/xsm.h +++ b/xen/include/xsm/xsm.h @@ -180,6 +180,7 @@ struct xsm_operations { int (*dm_op) (struct domain *d); #endif int (*xen_version) (uint32_t cmd); + int (*domain_resource_map) (struct domain *d); }; #ifdef CONFIG_XSM @@ -692,6 +693,11 @@ static inline int xsm_xen_version (xsm_default_t def, uint32_t op) return xsm_ops->xen_version(op); } +static inline int xsm_domain_resource_map(xsm_default_t def, struct domain *d) +{ + return xsm_ops->domain_resource_map(d); +} + #endif /* XSM_NO_WRAPPERS */ #ifdef CONFIG_MULTIBOOT diff --git a/xen/xsm/dummy.c b/xen/xsm/dummy.c index 479b103614..6e751199ee 100644 --- a/xen/xsm/dummy.c +++ b/xen/xsm/dummy.c @@ -157,4 +157,5 @@ void __init xsm_fixup_ops (struct xsm_operations *ops) set_to_dummy_if_null(ops, dm_op); #endif set_to_dummy_if_null(ops, xen_version); + set_to_dummy_if_null(ops, domain_resource_map); } diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 7b005af834..7e9efcd865 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -1718,6 +1718,11 @@ static int flask_xen_version (uint32_t op) } } +static int flask_domain_resource_map(struct domain *d) +{ + return current_has_perm(d, SECCLASS_DOMAIN2, DOMAIN2__RESOURCE_MAP); +} + long do_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op); int compat_flask_op(XEN_GUEST_HANDLE_PARAM(xsm_op_t) u_flask_op); @@ -1851,6 +1856,7 @@ static struct xsm_operations flask_ops = { .dm_op = flask_dm_op, #endif .xen_version = flask_xen_version, + .domain_resource_map = flask_domain_resource_map, }; void __init flask_init(const void *policy_buffer, size_t policy_size) diff --git a/xen/xsm/flask/policy/access_vectors b/xen/xsm/flask/policy/access_vectors index 3a2d863b8f..341ade1f7d 100644 --- a/xen/xsm/flask/policy/access_vectors +++ b/xen/xsm/flask/policy/access_vectors @@ -250,6 +250,8 @@ class domain2 psr_cat_op # XEN_DOMCTL_set_gnttab_limits set_gnttab_limits +# XENMEM_resource_map + resource_map } # Similar to class domain, but primarily contains domctls related to HVM domains
Certain memory resources associated with a guest are not necessarily present in the guest P2M. This patch adds the boilerplate for new memory op to allow such a resource to be priv-mapped directly, by either a PV or HVM tools domain. NOTE: Whilst the new op is not intrinsicly specific to the x86 architecture, I have no means to test it on an ARM platform and so cannot verify that it functions correctly. Hence it is currently only implemented for x86. Signed-off-by: Paul Durrant <paul.durrant@citrix.com> --- Cc: George Dunlap <george.dunlap@eu.citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> Cc: George Dunlap <George.Dunlap@eu.citrix.com> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Cc: Stefano Stabellini <sstabellini@kernel.org> Cc: Tim Deegan <tim@xen.org> Cc: Wei Liu <wei.liu2@citrix.com> v11: - Addressed more comments from Jan. v9: - Addressed more comments from Jan. v8: - Move the code into common as requested by Jan. - Make the gmfn_list handle a 64-bit type to avoid limiting the MFN range for a 32-bit tools domain. - Add missing pad. - Add compat code. - Make this patch deal with purely boilerplate. - Drop George's A-b and Wei's R-b because the changes are non-trivial, and update Cc list now the boilerplate is common. v5: - Switched __copy_to/from_guest_offset() to copy_to/from_guest_offset(). --- tools/flask/policy/modules/xen.if | 4 +- xen/arch/x86/mm/p2m.c | 3 +- xen/common/compat/memory.c | 145 ++++++++++++++++++++++++++++++++++-- xen/common/memory.c | 90 ++++++++++++++++++++++ xen/include/asm-x86/p2m.h | 3 + xen/include/public/memory.h | 43 ++++++++++- xen/include/xlat.lst | 1 + xen/include/xsm/dummy.h | 6 ++ xen/include/xsm/xsm.h | 6 ++ xen/xsm/dummy.c | 1 + xen/xsm/flask/hooks.c | 6 ++ xen/xsm/flask/policy/access_vectors | 2 + 12 files changed, 300 insertions(+), 10 deletions(-)