On 03/12/15 11:22, Ian Campbell wrote:
> It can trivially be replaced by xc_map_foreign_pages which is the
> interface I want to move to going forward (by standardising on _bulk
> but handling err=NULL as _pages does).
>
> The callers of _batch are checking a mixture of a NULL return or
> looking to see if the top nibble of the (usually sole) mfn they pass
> has been modified to be non-zero to detect errors. _pages never
> modifies the mfn it was given (it's const) and returns NULL on
> failure, so adjust the error handling where necessary. Some callers
> use a copy of the mfn array, for reuse on failure with _batch, which
> is no longer necessary as _pages doesn't modify the array, however I
> haven't cleaned that up here.
>
> 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.
"New OS ports should always implement xc_map_foreign_pages instead"?
Other than that:
Acked-by: George Dunlap <george.dun...@citrix.com>
>
> Signed-off-by: Ian Campbell <ian.campb...@citrix.com>
> Cc: George Dunlap <george.dun...@eu.citrix.com>
> ---
> v6: Switch to xc_map_foreign_pages not xc_map_foreign_bulk, since the
> former has more similar error handling semantics to the current
> usage.
>
> Dropped acks.
> ---
> 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 | 18 ++++++++++---
> tools/xenmon/xenbaked.c | 12 ++++++++-
> tools/xenpaging/xenpaging.c | 14 +++++-----
> tools/xentrace/xentrace.c | 3 ++-
> 10 files changed, 48 insertions(+), 110 deletions(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 78400d3..cb41c07 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1387,16 +1387,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 b205bca..2413e75 100644
> --- a/tools/libxc/xc_foreign_memory.c
> +++ b/tools/libxc/xc_foreign_memory.c
> @@ -55,6 +55,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)
> @@ -75,7 +77,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 e68c495..39c88ce 100644
> --- a/tools/libxc/xc_linux_osdep.c
> +++ b/tools/libxc/xc_linux_osdep.c
> @@ -91,8 +91,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;
> @@ -113,59 +113,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
> @@ -305,7 +252,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 e3c8241..3ea3124 100644
> --- a/tools/libxc/xc_minios.c
> +++ b/tools/libxc/xc_minios.c
> @@ -73,28 +73,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 d7f7f31..6a7ff9f 100644
> --- a/tools/libxc/xc_netbsd.c
> +++ b/tools/libxc/xc_netbsd.c
> @@ -67,16 +67,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;
> }
>
> @@ -87,7 +87,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 5e72dee..2df7a06 100644
> --- a/tools/libxc/xc_solaris.c
> +++ b/tools/libxc/xc_solaris.c
> @@ -67,9 +67,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 2fef96a..d2d99e4 100644
> --- a/tools/libxc/xc_vm_event.c
> +++ b/tools/libxc/xc_vm_event.c
> @@ -72,9 +72,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,
> + ring_page = xc_map_foreign_pages(xch, domain_id, PROT_READ | PROT_WRITE,
> &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + if ( !ring_page )
> {
> /* Map failed, populate ring page */
> rc1 = xc_domain_populate_physmap_exact(xch, domain_id, 1, 0, 0,
> @@ -86,9 +86,9 @@ 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,
> + ring_page = xc_map_foreign_pages(xch, domain_id, PROT_READ |
> PROT_WRITE,
> &mmap_pfn, 1);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + if ( !ring_page )
> {
> PERROR("Could not map the ring page\n");
> goto out;
> @@ -156,3 +156,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 e4602ef..da46126 100644
> --- a/tools/xenmon/xenbaked.c
> +++ b/tools/xenmon/xenbaked.c
> @@ -411,7 +411,7 @@ static struct t_struct *map_tbufs(unsigned long
> tbufs_mfn, unsigned int num,
> 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,
> + tbufs.meta[i] = xc_map_foreign_pages(xc_handle, DOMID_XEN,
> PROT_READ | PROT_WRITE,
> pfn_list,
> tbufs.t_info->tbuf_size);
> @@ -1175,3 +1175,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/xenpaging.c b/tools/xenpaging/xenpaging.c
> index df99c6a..c4bc713 100644
> --- a/tools/xenpaging/xenpaging.c
> +++ b/tools/xenpaging/xenpaging.c
> @@ -342,9 +342,9 @@ 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);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + xc_map_foreign_pages(xch, paging->vm_event.domain_id,
> + PROT_READ | PROT_WRITE, &mmap_pfn, 1);
> + if ( !paging->vm_event.ring_page )
> {
> /* Map failed, populate ring page */
> rc = xc_domain_populate_physmap_exact(paging->xc_handle,
> @@ -356,11 +356,11 @@ static struct xenpaging *xenpaging_init(int argc, char
> *argv[])
> goto err;
> }
>
> - 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);
> - if ( mmap_pfn & XEN_DOMCTL_PFINFO_XTAB )
> + xc_map_foreign_pages(xch, paging->vm_event.domain_id,
> + PROT_READ | PROT_WRITE,
> + &mmap_pfn, 1);
> + if ( !paging->vm_event.ring_page )
> {
> PERROR("Could not map the ring page\n");
> goto err;
> diff --git a/tools/xentrace/xentrace.c b/tools/xentrace/xentrace.c
> index c970d42..6cbe0ac 100644
> --- a/tools/xentrace/xentrace.c
> +++ b/tools/xentrace/xentrace.c
> @@ -508,7 +508,7 @@ static struct t_struct *map_tbufs(unsigned long
> tbufs_mfn, unsigned int num,
> 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,
> + tbufs.meta[i] = xc_map_foreign_pages(xc_handle, DOMID_XEN,
> PROT_READ | PROT_WRITE,
> pfn_list,
> tbufs.t_info->tbuf_size);
> @@ -1221,6 +1221,7 @@ int main(int argc, char **argv)
>
> return ret;
> }
> +
> /*
> * Local variables:
> * mode: C
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
