|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH XEN v2 12/15] tools: Remove xc_map_foreign_batch
On 07/15/2015 04:47 PM, Ian Campbell wrote:
> It can trivially be replaced by xc_map_foreign_bulk which is the
> interface I want to move to going forward. All in tree users are
> trivially converted by supplying the appropriate error array and
> adjusting the what error handling exists (which in many cases is not
> much).
>
> This reduces the twist maze of xc_map_foreign_* by one, which will be
> useful when trying to come up with an underlying stable interface.
>
> NetBSD and Solaris implemented xc_map_foreign_bulk in terms of
> xc_map_foreign_batch via a compat layer, so xc_map_foreign_batch
> becomes an internal osdep for them. New OS ports should always
> implement xc_map_foreign_bulk instead.
>
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
xentrace and xenbaked:
Acked-by: George Dunlap <george.dunlap@xxxxxxxxxx>
(Note that I'm going to be switching to using @citrix.com in tags post-4.6)
> ---
> docs/misc/toolstack-library-abis.pandoc | 17 +++-------
> tools/libxc/include/xenctrl.h | 10 ------
> tools/libxc/xc_foreign_memory.c | 4 ++-
> tools/libxc/xc_linux_osdep.c | 59
> ++-------------------------------
> tools/libxc/xc_minios.c | 22 ------------
> tools/libxc/xc_netbsd.c | 10 +++---
> tools/libxc/xc_solaris.c | 6 ++--
> tools/libxc/xc_vm_event.c | 25 ++++++++++----
> tools/xenmon/xenbaked.c | 19 ++++++++---
> tools/xenpaging/pagein.c | 4 ++-
> tools/xenpaging/xenpaging.c | 12 ++++---
> tools/xentrace/xentrace.c | 10 +++---
> 12 files changed, 68 insertions(+), 130 deletions(-)
>
> diff --git a/docs/misc/toolstack-library-abis.pandoc
> b/docs/misc/toolstack-library-abis.pandoc
> index 271e4b5..4ad6c7d 100644
> --- a/docs/misc/toolstack-library-abis.pandoc
> +++ b/docs/misc/toolstack-library-abis.pandoc
> @@ -274,7 +274,6 @@ Interface Underlying interface
> Known external
> `xc_map_foreign_pages` `IOCTL_PRIVCMD_MMAP...` qemu-pv
> `xc_map_foreign_range` `IOCTL_PRIVCMD_MMAP...` qemu-dm,
> and qemu-pv
> `xc_map_foreign_ranges`
> -`xc_map_foreign_batch`
> `xc_map_foreign_bulk` `IOCTL_PRIVCMD_MMAPBATCH_V2` qemu-dm
>
> ### Managing guest memory map
> @@ -883,7 +882,6 @@ osdep level interfaces:
>
> Interface Linux Minios Netbsd Freebsd Solaris
> -------------------- ----- ------ ------ ------- -------
> -`map_foreign_batch` Y Y Y Y
> `map_foreign_bulk` Y Y compat Y compat
> `map_foreign_range` Y Y Y Y Y
> `map_foreign_ranges` Y Y Y Y Y
> @@ -897,7 +895,6 @@ Some osdep call other `xc_map_foreign_*` recursivley:
>
> Interface Linux FreeBSD
> -------------------- ---------------------------- -------------------------
> -`map_foreign_batch` `IOCTL_PRIVCMD_MMAPBATCH` N/A
> `map_foreign_bulk` `IOCTL_PRIVCMD_MMAPBATCH_V2` `IOCTL_PRIVCMD_MMAPBATCH`
> `map_foreign_range` `xc_map_foreign_pages` `xc_map_foreign_pages`
> `map_foreign_ranges` `xc_map_foreign_pages` `xc_map_foreign_pages`
> @@ -909,17 +906,14 @@ Interface Underlying interface
> `xc_map_foreign_pages` `xc_map_foreign_bulk`
> `xc_map_foreign_range` `osdep: map_foreign_range`
> `xc_map_foreign_ranges` `osdep: map_foreign_ranges`
> -`xc_map_foreign_batch` `osdep: map_foreign_batch`
> `xc_map_foreign_bulk` `osdep: map_foreign_bulk`
>
> `osdep: *` have been collapsed since osdep layer was removed.
>
> -`xc_map_foreign_bulk_compat` is an internal helper used by osdep
> -backends which have not implemented there own version. It uses
> -`xc_map_foreign_batch`.
> -
> -`xc_map_foreign_batch_single` is part of linux osdep, but name has
> -leaked.
> +`xc_map_foreign_bulk` can be an internal compat helper used by oses
> +which have not implemented their own version. It used to use
> +`xc_map_foreign_batch` which is now an osdep for those OSes which need
> +it.
>
> Interface Internal users External users
> ---------------------- -------------- --------------
> @@ -928,11 +922,10 @@ Interface Internal users
> External users
> mfndump, memshrtool, xenmon,
> xenstored, xentrace
> `xc_map_foreign_ranges` NONE
> -`xc_map_foreign_batch` xenmon, xenpaging, xentrace
> `xc_map_foreign_bulk` used recursively qemu-dm
>
> Only one interface should become a stable API. Something based on
> `xc_map_foreign_bulk` seems most appropriate (it is used by
> `xc_map_foreign_pages` as well as by xc_map_foreign_range` and
> `xc_map_foreign_ranges` on Linux + FreeBSD. A compat version exists in
> -terms for `xc_map_foreign_batch` for other OSes.
> +terms of `xc_map_foreign_batch` for other OSes.
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 61f68bb..201fd70 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1367,16 +1367,6 @@ void *xc_map_foreign_pages(xc_interface *xch, uint32_t
> dom, int prot,
> const xen_pfn_t *arr, int num );
>
> /**
> - * DEPRECATED - use xc_map_foreign_bulk() instead.
> - *
> - * Like xc_map_foreign_pages(), except it can succeeed partially.
> - * When a page cannot be mapped, its PFN in @arr is or'ed with
> - * 0xF0000000 to indicate the error.
> - */
> -void *xc_map_foreign_batch(xc_interface *xch, uint32_t dom, int prot,
> - xen_pfn_t *arr, int num );
> -
> -/**
> * Like xc_map_foreign_pages(), except it can succeed partially.
> * When a page cannot be mapped, its respective field in @err is
> * set to the corresponding errno value.
> diff --git a/tools/libxc/xc_foreign_memory.c b/tools/libxc/xc_foreign_memory.c
> index da77e9d..ae133a1 100644
> --- a/tools/libxc/xc_foreign_memory.c
> +++ b/tools/libxc/xc_foreign_memory.c
> @@ -56,6 +56,8 @@ void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom,
> int prot,
> * just implement xc_map_foreign_bulk.
> */
> #if defined(__NetBSD__) || defined(__sun__)
> +void *osdep_map_foreign_batch(xc_interface *xch, uint32_t dom, int prot,
> + xen_pfn_t *arr, int num );
> void *xc_map_foreign_bulk(xc_interface *xch,
> uint32_t dom, int prot,
> const xen_pfn_t *arr, int *err, unsigned int num)
> @@ -76,7 +78,7 @@ void *xc_map_foreign_bulk(xc_interface *xch,
> }
>
> memcpy(pfn, arr, num * sizeof(*arr));
> - ret = xc_map_foreign_batch(xch, dom, prot, pfn, num);
> + ret = osdep_map_foreign_batch(xch, dom, prot, pfn, num);
>
> if (ret) {
> for (i = 0; i < num; ++i)
> diff --git a/tools/libxc/xc_linux_osdep.c b/tools/libxc/xc_linux_osdep.c
> index 488e8df..6641bd1 100644
> --- a/tools/libxc/xc_linux_osdep.c
> +++ b/tools/libxc/xc_linux_osdep.c
> @@ -86,8 +86,8 @@ int osdep_privcmd_close(xc_interface *xch)
> return close(fd);
> }
>
> -static int xc_map_foreign_batch_single(int fd, uint32_t dom,
> - xen_pfn_t *mfn, unsigned long addr)
> +static int map_foreign_batch_single(int fd, uint32_t dom,
> + xen_pfn_t *mfn, unsigned long addr)
> {
> privcmd_mmapbatch_t ioctlx;
> int rc;
> @@ -108,59 +108,6 @@ static int xc_map_foreign_batch_single(int fd, uint32_t
> dom,
> return rc;
> }
>
> -void *xc_map_foreign_batch(xc_interface *xch,
> - uint32_t dom, int prot,
> - xen_pfn_t *arr, int num)
> -{
> - int fd = xch->privcmdfd;
> - privcmd_mmapbatch_t ioctlx;
> - void *addr;
> - int rc;
> -
> - addr = mmap(NULL, num << XC_PAGE_SHIFT, prot, MAP_SHARED, fd, 0);
> - if ( addr == MAP_FAILED )
> - {
> - PERROR("xc_map_foreign_batch: mmap failed");
> - return NULL;
> - }
> -
> - ioctlx.num = num;
> - ioctlx.dom = dom;
> - ioctlx.addr = (unsigned long)addr;
> - ioctlx.arr = arr;
> -
> - rc = ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx);
> - if ( (rc < 0) && (errno == ENOENT) )
> - {
> - int i;
> -
> - for ( i = 0; i < num; i++ )
> - {
> - if ( (arr[i] & PRIVCMD_MMAPBATCH_MFN_ERROR) ==
> - PRIVCMD_MMAPBATCH_PAGED_ERROR )
> - {
> - unsigned long paged_addr = (unsigned long)addr + (i <<
> XC_PAGE_SHIFT);
> - rc = xc_map_foreign_batch_single(fd, dom, &arr[i],
> - paged_addr);
> - if ( rc < 0 )
> - goto out;
> - }
> - }
> - }
> -
> - out:
> - if ( rc < 0 )
> - {
> - int saved_errno = errno;
> - PERROR("xc_map_foreign_batch: ioctl failed");
> - (void)munmap(addr, num << XC_PAGE_SHIFT);
> - errno = saved_errno;
> - return NULL;
> - }
> -
> - return addr;
> -}
> -
> /*
> * Retry mmap of all paged gfns in batches
> * retuns < 0 on fatal error
> @@ -300,7 +247,7 @@ void *xc_map_foreign_bulk(xc_interface *xch,
> err[i] = rc ?: -EINVAL;
> continue;
> }
> - rc = xc_map_foreign_batch_single(fd, dom, pfn + i,
> + rc = map_foreign_batch_single(fd, dom, pfn + i,
> (unsigned long)addr + ((unsigned
> long)i<<XC_PAGE_SHIFT));
> if ( rc < 0 )
> {
> diff --git a/tools/libxc/xc_minios.c b/tools/libxc/xc_minios.c
> index 27a4c4a..ed20652 100644
> --- a/tools/libxc/xc_minios.c
> +++ b/tools/libxc/xc_minios.c
> @@ -74,28 +74,6 @@ void *xc_map_foreign_bulk(xc_interface *xch,
> return map_frames_ex(arr, num, 1, 0, 1, dom, err, pt_prot);
> }
>
> -void *xc_map_foreign_batch(xc_interface *xch,
> - uint32_t dom, int prot,
> - xen_pfn_t *arr, int num)
> -{
> - unsigned long pt_prot = 0;
> - int err[num];
> - int i;
> - unsigned long addr;
> -
> - if (prot & PROT_READ)
> - pt_prot = L1_PROT_RO;
> - if (prot & PROT_WRITE)
> - pt_prot = L1_PROT;
> -
> - addr = (unsigned long) map_frames_ex(arr, num, 1, 0, 1, dom, err,
> pt_prot);
> - for (i = 0; i < num; i++) {
> - if (err[i])
> - arr[i] |= 0xF0000000;
> - }
> - return (void *) addr;
> -}
> -
> void *xc_map_foreign_range(xc_interface *xch,
> uint32_t dom,
> int size, int prot,
> diff --git a/tools/libxc/xc_netbsd.c b/tools/libxc/xc_netbsd.c
> index a61e52f..f02015c 100644
> --- a/tools/libxc/xc_netbsd.c
> +++ b/tools/libxc/xc_netbsd.c
> @@ -68,16 +68,16 @@ int osdep_privcmd_close(xc_interface *xch)
> return close(fd);
> }
>
> -void *xc_map_foreign_batch(xc_interface *xch,
> - uint32_t dom, int prot,
> - xen_pfn_t *arr, int num)
> +void *osdep_map_foreign_batch(xc_interface *xch,
> + uint32_t dom, int prot,
> + xen_pfn_t *arr, int num)
> {
> int fd = xch->privcmdfd;
> privcmd_mmapbatch_t ioctlx;
> void *addr;
> addr = mmap(NULL, num*XC_PAGE_SIZE, prot, MAP_ANON | MAP_SHARED, -1, 0);
> if ( addr == MAP_FAILED ) {
> - PERROR("xc_map_foreign_batch: mmap failed");
> + PERROR("osdep_map_foreign_batch: mmap failed");
> return NULL;
> }
>
> @@ -88,7 +88,7 @@ void *xc_map_foreign_batch(xc_interface *xch,
> if ( ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH, &ioctlx) < 0 )
> {
> int saved_errno = errno;
> - PERROR("xc_map_foreign_batch: ioctl failed");
> + PERROR("osdep_map_foreign_batch: ioctl failed");
> (void)munmap(addr, num*XC_PAGE_SIZE);
> errno = saved_errno;
> return NULL;
> diff --git a/tools/libxc/xc_solaris.c b/tools/libxc/xc_solaris.c
> index 1e1a0c4..0a0fd14 100644
> --- a/tools/libxc/xc_solaris.c
> +++ b/tools/libxc/xc_solaris.c
> @@ -68,9 +68,9 @@ int osdep_privcmd_close(xc_interface *xch)
> return close(fd);
> }
>
> -void *xc_map_foreign_batch(xc_interface *xch,
> - uint32_t dom, int prot,
> - xen_pfn_t *arr, int num)
> +void *osdep_map_foreign_batch(xc_interface *xch,
> + uint32_t dom, int prot,
> + xen_pfn_t *arr, int num)
> {
> int fd = xch->privcmdfd;
> privcmd_mmapbatch_t ioctlx;
> diff --git a/tools/libxc/xc_vm_event.c b/tools/libxc/xc_vm_event.c
> index a5b3277..4d7e7cc 100644
> --- a/tools/libxc/xc_vm_event.c
> +++ b/tools/libxc/xc_vm_event.c
> @@ -46,6 +46,7 @@ void *xc_vm_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
> void *ring_page = NULL;
> uint64_t pfn;
> xen_pfn_t ring_pfn, mmap_pfn;
> + int err;
> unsigned int op, mode;
> int rc1, rc2, saved_errno;
>
> @@ -73,9 +74,9 @@ void *xc_vm_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
>
> ring_pfn = pfn;
> mmap_pfn = pfn;
> - ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ | PROT_WRITE,
> - &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + ring_page = xc_map_foreign_bulk(xch, domain_id, PROT_READ | PROT_WRITE,
> + &mmap_pfn, &err, 1);
> + if ( !ring_page )
> {
> /* Map failed, populate ring page */
> rc1 = xc_domain_populate_physmap_exact(xch, domain_id, 1, 0, 0,
> @@ -87,11 +88,11 @@ void *xc_vm_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
> }
>
> mmap_pfn = ring_pfn;
> - ring_page = xc_map_foreign_batch(xch, domain_id, PROT_READ |
> PROT_WRITE,
> - &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + ring_page = xc_map_foreign_bulk(xch, domain_id, PROT_READ |
> PROT_WRITE,
> + &mmap_pfn, &err, 1);
> + if ( !ring_page )
> {
> - PERROR("Could not map the ring page\n");
> + PERROR("Could not map the ring page: %d\n", err);
> goto out;
> }
> }
> @@ -157,3 +158,13 @@ void *xc_vm_event_enable(xc_interface *xch, domid_t
> domain_id, int param,
>
> return ring_page;
> }
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/xenmon/xenbaked.c b/tools/xenmon/xenbaked.c
> index f092aec..9e2a49b 100644
> --- a/tools/xenmon/xenbaked.c
> +++ b/tools/xenmon/xenbaked.c
> @@ -408,14 +408,15 @@ static struct t_struct *map_tbufs(unsigned long
> tbufs_mfn, unsigned int num,
> + tbufs.t_info->mfn_offset[i];
> int j;
> xen_pfn_t pfn_list[tbufs.t_info->tbuf_size];
> + int err_list[tbufs.t_info->tbuf_size];
>
> for ( j=0; j<tbufs.t_info->tbuf_size; j++)
> pfn_list[j] = (xen_pfn_t)mfn_list[j];
>
> - tbufs.meta[i] = xc_map_foreign_batch(xc_handle, DOMID_XEN,
> - PROT_READ | PROT_WRITE,
> - pfn_list,
> - tbufs.t_info->tbuf_size);
> + tbufs.meta[i] = xc_map_foreign_bulk(xc_handle, DOMID_XEN,
> + PROT_READ | PROT_WRITE,
> + pfn_list, err_list,
> + tbufs.t_info->tbuf_size);
> if ( tbufs.meta[i] == NULL )
> {
> PERROR("Failed to map cpu buffer!");
> @@ -1176,3 +1177,13 @@ static int process_record(int cpu, struct t_rec *r)
>
> return 4 + (r->cycles_included ? 8 : 0) + (r->extra_u32 * 4);
> }
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/tools/xenpaging/pagein.c b/tools/xenpaging/pagein.c
> index 7cb0f33..18699ec 100644
> --- a/tools/xenpaging/pagein.c
> +++ b/tools/xenpaging/pagein.c
> @@ -23,6 +23,7 @@ static void *page_in(void *arg)
> void *page;
> int i, num;
> xen_pfn_t gfns[XENPAGING_PAGEIN_QUEUE_SIZE];
> + int errs[XENPAGING_PAGEIN_QUEUE_SIZE];
>
> while (1)
> {
> @@ -42,7 +43,8 @@ static void *page_in(void *arg)
> pthread_mutex_unlock(&page_in_mutex);
>
> /* Ignore errors */
> - page = xc_map_foreign_pages(pia->xch, pia->dom, PROT_READ, gfns,
> num);
> + page = xc_map_foreign_bulk(pia->xch, pia->dom, PROT_READ,
> + gfns, errs, num);
> if (page)
> munmap(page, PAGE_SIZE * num);
> }
> diff --git a/tools/xenpaging/xenpaging.c b/tools/xenpaging/xenpaging.c
> index 6776896..a819a5b 100644
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -282,6 +282,7 @@ static struct xenpaging *xenpaging_init(int argc, char
> *argv[])
> char *p;
> int rc;
> unsigned long ring_pfn, mmap_pfn;
> + int mmap_err;
>
> /* Allocate memory */
> paging = calloc(1, sizeof(struct xenpaging));
> @@ -343,8 +344,8 @@ static struct xenpaging *xenpaging_init(int argc, char
> *argv[])
> HVM_PARAM_PAGING_RING_PFN, &ring_pfn);
> mmap_pfn = ring_pfn;
> paging->vm_event.ring_page =
> - xc_map_foreign_batch(xch, paging->vm_event.domain_id,
> - PROT_READ | PROT_WRITE, &mmap_pfn, 1);
> + xc_map_foreign_bulk(xch, paging->vm_event.domain_id,
> + PROT_READ | PROT_WRITE, &mmap_pfn, &mmap_err, 1);
> if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> {
> /* Map failed, populate ring page */
> @@ -359,11 +360,12 @@ static struct xenpaging *xenpaging_init(int argc, char
> *argv[])
>
> mmap_pfn = ring_pfn;
> paging->vm_event.ring_page =
> - xc_map_foreign_batch(xch, paging->vm_event.domain_id,
> - PROT_READ | PROT_WRITE, &mmap_pfn, 1);
> + xc_map_foreign_bulk(xch, paging->vm_event.domain_id,
> + PROT_READ | PROT_WRITE,
> + &mmap_pfn, &mmap_err, 1);
> if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> {
> - PERROR("Could not map the ring page\n");
> + PERROR("Could not map the ring page: %d\n", mmap_err);
> goto err;
> }
> }
> diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c
> index c970d42..f66a77c 100644
> --- a/tools/xentrace/xentrace.c
> +++ b/tools/xentrace/xentrace.c
> @@ -504,14 +504,15 @@ static struct t_struct *map_tbufs(unsigned long
> tbufs_mfn, unsigned int num,
> + tbufs.t_info->mfn_offset[i];
> int j;
> xen_pfn_t pfn_list[tbufs.t_info->tbuf_size];
> + int pfn_err[tbufs.t_info->tbuf_size];
>
> for ( j=0; j<tbufs.t_info->tbuf_size; j++)
> pfn_list[j] = (xen_pfn_t)mfn_list[j];
>
> - tbufs.meta[i] = xc_map_foreign_batch(xc_handle, DOMID_XEN,
> - PROT_READ | PROT_WRITE,
> - pfn_list,
> - tbufs.t_info->tbuf_size);
> + tbufs.meta[i] = xc_map_foreign_bulk(xc_handle, DOMID_XEN,
> + PROT_READ | PROT_WRITE,
> + pfn_list, pfn_err,
> + tbufs.t_info->tbuf_size);
> if ( tbufs.meta[i] == NULL )
> {
> PERROR("Failed to map cpu buffer!");
> @@ -1221,6 +1222,7 @@ int main(int argc, char **argv)
>
> return ret;
> }
> +
> /*
> * Local variables:
> * mode: C
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |