| Message ID | 20170822145107.6877-5-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Aug 22, 2017 at 03:50:58PM +0100, Paul Durrant wrote: > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h b/tools/libs/foreignmemory/include/xenforeignmemory.h > index f4814c390f..e56eb3c4d4 100644 > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h > @@ -138,6 +138,45 @@ int xenforeignmemory_unmap(xenforeignmemory_handle *fmem, > int xenforeignmemory_restrict(xenforeignmemory_handle *fmem, > domid_t domid); > > +typedef struct xenforeignmemory_resource_handle xenforeignmemory_resource_handle; > + > +/** > + * This function maps a guest resource. > + * > + * @parm fmem handle to the open foreignmemory interface > + * @parm domid the domain id > + * @parm type the resource type > + * @parm id the type-specific resource identifier > + * @parm frame base frame index within the resource > + * @parm nr_frames number of frames to map > + * @parm paddr pointer to an address passed through to mmap(2) > + * @parm prot passed through to mmap(2) > + * @parm flags passed through to mmap(2) Passing mmap flags directly can be troublesome, POSIX only defines MAP_SHARED and MAP_PRIVATE, the rest is OS-specific AFAIK. So we either limit this to POSIX only flags (and enforce it), or we replace it with some xenforeignmemory specific flags that each OS can map to it's equivalents. > --- a/tools/libs/foreignmemory/private.h > +++ b/tools/libs/foreignmemory/private.h > @@ -42,6 +42,36 @@ void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom, > xen_pfn_t *arr, int num); > #endif > > +struct xenforeignmemory_resource_handle { > + domid_t domid; > + unsigned int type; > + unsigned int id; > + unsigned long frame; > + unsigned long nr_frames; > + void *addr; > + int prot; > + int flags; > +}; > + > +#ifndef __linux__ The common practice in libxenforeign seems to be to add those dummy handlers to each OS-specific file (see osdep_xenforeignmemory_restrict for example) instead of doing it in the header file. Roger.
> -----Original Message----- > From: Roger Pau Monne > Sent: 24 August 2017 16:53 > To: Paul Durrant <Paul.Durrant@citrix.com> > Cc: xen-devel@lists.xenproject.org; Wei Liu <wei.liu2@citrix.com>; Ian > Jackson <Ian.Jackson@citrix.com> > Subject: Re: [Xen-devel] [PATCH v2 REPOST 04/12] > tools/libxenforeignmemory: add support for resource mapping > > On Tue, Aug 22, 2017 at 03:50:58PM +0100, Paul Durrant wrote: > > diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h > b/tools/libs/foreignmemory/include/xenforeignmemory.h > > index f4814c390f..e56eb3c4d4 100644 > > --- a/tools/libs/foreignmemory/include/xenforeignmemory.h > > +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h > > @@ -138,6 +138,45 @@ int > xenforeignmemory_unmap(xenforeignmemory_handle *fmem, > > int xenforeignmemory_restrict(xenforeignmemory_handle *fmem, > > domid_t domid); > > > > +typedef struct xenforeignmemory_resource_handle > xenforeignmemory_resource_handle; > > + > > +/** > > + * This function maps a guest resource. > > + * > > + * @parm fmem handle to the open foreignmemory interface > > + * @parm domid the domain id > > + * @parm type the resource type > > + * @parm id the type-specific resource identifier > > + * @parm frame base frame index within the resource > > + * @parm nr_frames number of frames to map > > + * @parm paddr pointer to an address passed through to mmap(2) > > + * @parm prot passed through to mmap(2) > > + * @parm flags passed through to mmap(2) > > Passing mmap flags directly can be troublesome, POSIX only defines > MAP_SHARED and MAP_PRIVATE, the rest is OS-specific AFAIK. So we > either limit this to POSIX only flags (and enforce it), or we replace > it with some xenforeignmemory specific flags that each OS can map to > it's equivalents. Given that foreign memory mapping already passes through flags I guess that, for consistency, it would be best to limit use to POSIX-only flags in all cases. > > > --- a/tools/libs/foreignmemory/private.h > > +++ b/tools/libs/foreignmemory/private.h > > @@ -42,6 +42,36 @@ void > *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom, > > xen_pfn_t *arr, int num); > > #endif > > > > +struct xenforeignmemory_resource_handle { > > + domid_t domid; > > + unsigned int type; > > + unsigned int id; > > + unsigned long frame; > > + unsigned long nr_frames; > > + void *addr; > > + int prot; > > + int flags; > > +}; > > + > > +#ifndef __linux__ > > The common practice in libxenforeign seems to be to add those dummy > handlers to each OS-specific file (see osdep_xenforeignmemory_restrict > for example) instead of doing it in the header file. Yes, I know, I introduced that code :-) I think this way is actually neater. If no-one objects I can add a patch in to clean up xenforeignmemory_restrict() too. Cheers, Paul > > Roger.
diff --git a/tools/include/xen-sys/Linux/privcmd.h b/tools/include/xen-sys/Linux/privcmd.h index 732ff7c15a..9531b728f9 100644 --- a/tools/include/xen-sys/Linux/privcmd.h +++ b/tools/include/xen-sys/Linux/privcmd.h @@ -86,6 +86,15 @@ typedef struct privcmd_dm_op { const privcmd_dm_op_buf_t __user *ubufs; } privcmd_dm_op_t; +typedef struct privcmd_mmap_resource { + domid_t dom; + __u32 type; + __u32 id; + __u32 idx; + __u64 num; + __u64 addr; +} privcmd_mmap_resource_t; + /* * @cmd: IOCTL_PRIVCMD_HYPERCALL * @arg: &privcmd_hypercall_t @@ -103,5 +112,7 @@ typedef struct privcmd_dm_op { _IOC(_IOC_NONE, 'P', 5, sizeof(privcmd_dm_op_t)) #define IOCTL_PRIVCMD_RESTRICT \ _IOC(_IOC_NONE, 'P', 6, sizeof(domid_t)) +#define IOCTL_PRIVCMD_MMAP_RESOURCE \ + _IOC(_IOC_NONE, 'P', 7, sizeof(privcmd_mmap_resource_t)) #endif /* __LINUX_PUBLIC_PRIVCMD_H__ */ diff --git a/tools/libs/foreignmemory/Makefile b/tools/libs/foreignmemory/Makefile index b110076621..7eb59c78cb 100644 --- a/tools/libs/foreignmemory/Makefile +++ b/tools/libs/foreignmemory/Makefile @@ -2,7 +2,7 @@ XEN_ROOT = $(CURDIR)/../../.. include $(XEN_ROOT)/tools/Rules.mk MAJOR = 1 -MINOR = 2 +MINOR = 3 SHLIB_LDFLAGS += -Wl,--version-script=libxenforeignmemory.map CFLAGS += -Werror -Wmissing-prototypes diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c index a6897dc561..291ee44516 100644 --- a/tools/libs/foreignmemory/core.c +++ b/tools/libs/foreignmemory/core.c @@ -120,6 +120,48 @@ int xenforeignmemory_restrict(xenforeignmemory_handle *fmem, return osdep_xenforeignmemory_restrict(fmem, domid); } +xenforeignmemory_resource_handle *xenforeignmemory_map_resource( + xenforeignmemory_handle *fmem, domid_t domid, unsigned int type, + unsigned int id, unsigned long frame, unsigned long nr_frames, + void **paddr, int prot, int flags) +{ + xenforeignmemory_resource_handle *fres; + int rc; + + fres = calloc(1, sizeof(*fres)); + if ( !fres ) + return NULL; + + fres->domid = domid; + fres->type = type; + fres->id = id; + fres->frame = frame; + fres->nr_frames = nr_frames; + fres->addr = *paddr; + fres->prot = prot; + fres->flags = flags; + + rc = osdep_xenforeignmemory_map_resource(fmem, fres); + if ( rc ) + goto fail; + + *paddr = fres->addr; + return fres; + +fail: + free(fres); + + return NULL; +} + +void xenforeignmemory_unmap_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres) +{ + osdep_xenforeignmemory_unmap_resource(fmem, fres); + + free(fres); +} + /* * Local variables: * mode: C diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h b/tools/libs/foreignmemory/include/xenforeignmemory.h index f4814c390f..e56eb3c4d4 100644 --- a/tools/libs/foreignmemory/include/xenforeignmemory.h +++ b/tools/libs/foreignmemory/include/xenforeignmemory.h @@ -138,6 +138,45 @@ int xenforeignmemory_unmap(xenforeignmemory_handle *fmem, int xenforeignmemory_restrict(xenforeignmemory_handle *fmem, domid_t domid); +typedef struct xenforeignmemory_resource_handle xenforeignmemory_resource_handle; + +/** + * This function maps a guest resource. + * + * @parm fmem handle to the open foreignmemory interface + * @parm domid the domain id + * @parm type the resource type + * @parm id the type-specific resource identifier + * @parm frame base frame index within the resource + * @parm nr_frames number of frames to map + * @parm paddr pointer to an address passed through to mmap(2) + * @parm prot passed through to mmap(2) + * @parm flags passed through to mmap(2) + * @return pointer to foreignmemory resource handle on success, NULL on + * failure + * + * *paddr is used, on entry, as a hint address for foreign map placement + * (see mmap(2)) so should be set to NULL if no specific placement is + * required. On return *paddr contains the address where the resource is + * mapped. + * As for xenforeignmemory_map2() flags is a set of additional flags + * for mmap(2). Not all of the flag combinations are possible due to + * implementation details on different platforms. + */ +xenforeignmemory_resource_handle *xenforeignmemory_map_resource( + xenforeignmemory_handle *fmem, domid_t domid, unsigned int type, + unsigned int id, unsigned long frame, unsigned long nr_frames, + void **paddr, int prot, int flags); + +/** + * This function releases a previously acquired resource. + * + * @parm fmem handle to the open foreignmemory interface + * @parm fres handle to the acquired resource + */ +void xenforeignmemory_unmap_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres); + #endif /* diff --git a/tools/libs/foreignmemory/libxenforeignmemory.map b/tools/libs/foreignmemory/libxenforeignmemory.map index 716ecaf15c..d5323c87d9 100644 --- a/tools/libs/foreignmemory/libxenforeignmemory.map +++ b/tools/libs/foreignmemory/libxenforeignmemory.map @@ -14,3 +14,8 @@ VERS_1.2 { global: xenforeignmemory_map2; } VERS_1.1; +VERS_1.3 { + global: + xenforeignmemory_map_resource; + xenforeignmemory_unmap_resource; +} VERS_1.2; diff --git a/tools/libs/foreignmemory/linux.c b/tools/libs/foreignmemory/linux.c index 374e45aed5..4447723cb1 100644 --- a/tools/libs/foreignmemory/linux.c +++ b/tools/libs/foreignmemory/linux.c @@ -277,6 +277,51 @@ int osdep_xenforeignmemory_restrict(xenforeignmemory_handle *fmem, return ioctl(fmem->fd, IOCTL_PRIVCMD_RESTRICT, &domid); } +void osdep_xenforeignmemory_unmap_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres) +{ + (void) munmap(fres->addr, fres->nr_frames << PAGE_SHIFT); +} + +int osdep_xenforeignmemory_map_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres) +{ + privcmd_mmap_resource_t mr; + int rc; + + fres->addr = mmap(fres->addr, fres->nr_frames << PAGE_SHIFT, + fres->prot, fres->flags | MAP_SHARED, fmem->fd, 0); + if ( fres->addr == MAP_FAILED ) + return -1; + + memset(&mr, 0, sizeof(mr)); + mr.dom = fres->domid; + mr.type = fres->type; + mr.id = fres->id; + mr.idx = fres->frame; + mr.num = fres->nr_frames; + mr.addr = (uintptr_t)fres->addr; + + rc = ioctl(fmem->fd, IOCTL_PRIVCMD_MMAP_RESOURCE, &mr); + if ( rc ) + { + int saved_errno; + + if ( errno != ENOTTY ) + PERROR("ioctl failed"); + else + errno = EOPNOTSUPP; + + saved_errno = errno; + osdep_xenforeignmemory_unmap_resource(fmem, fres); + errno = saved_errno; + + return -1; + } + + return 0; +} + /* * Local variables: * mode: C diff --git a/tools/libs/foreignmemory/private.h b/tools/libs/foreignmemory/private.h index c5c07cc4c4..2ae9382669 100644 --- a/tools/libs/foreignmemory/private.h +++ b/tools/libs/foreignmemory/private.h @@ -42,6 +42,36 @@ void *compat_mapforeign_batch(xenforeignmem_handle *fmem, uint32_t dom, xen_pfn_t *arr, int num); #endif +struct xenforeignmemory_resource_handle { + domid_t domid; + unsigned int type; + unsigned int id; + unsigned long frame; + unsigned long nr_frames; + void *addr; + int prot; + int flags; +}; + +#ifndef __linux__ +static inline int osdep_xenforeignmemory_map_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres) +{ + errno = EOPNOTSUPP; + return -1; +} + +void osdep_xenforeignmemory_unmap_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres) +{ +} +#else +int osdep_xenforeignmemory_map_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres); +void osdep_xenforeignmemory_unmap_resource( + xenforeignmemory_handle *fmem, xenforeignmemory_resource_handle *fres); +#endif + #define PERROR(_f...) \ xtl_log(fmem->logger, XTL_ERROR, errno, "xenforeignmemory", _f)
A previous patch introduced a new HYPERVISOR_memory_op to acquire guest resources for direct priv-mapping. This patch adds new functionality into libxenforeignmemory to make use of a new privcmd ioctl [1] that uses the new memory op to make such resources available via mmap(2). [1] http://xenbits.xen.org/gitweb/?p=people/pauldu/linux.git;a=commit;h=ce59a05e6712 Signed-off-by: Paul Durrant <paul.durrant@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> v2: - Bump minor version up to 3 --- tools/include/xen-sys/Linux/privcmd.h | 11 ++++++ tools/libs/foreignmemory/Makefile | 2 +- tools/libs/foreignmemory/core.c | 42 ++++++++++++++++++++ .../libs/foreignmemory/include/xenforeignmemory.h | 39 +++++++++++++++++++ tools/libs/foreignmemory/libxenforeignmemory.map | 5 +++ tools/libs/foreignmemory/linux.c | 45 ++++++++++++++++++++++ tools/libs/foreignmemory/private.h | 30 +++++++++++++++ 7 files changed, 173 insertions(+), 1 deletion(-)