Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD

[Ian Campbell <Ian.Campbell@citrix.com>]

On Tue, 2013-06-11 at 14:46 +0100, Frediano Ziglio wrote:
> Subject: [PATCH] gcov: Do not use mmap directly but use xc_hypercall_buffer_alloc
> 
> xencov.c did not compile on NetBSD so use xc_hypercall_buffer which is
> more portable.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
> ---
>  tools/misc/xencov.c |   19 +++++++------------
>  1 file changed, 7 insertions(+), 12 deletions(-)
> 
> Here you are. It get tested on a x64 machine, if somebody could test it
> with a NetBSD would be great.
> 
> diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c
> index 6645a30..e38d473 100644
> --- a/tools/misc/xencov.c
> +++ b/tools/misc/xencov.c
> @@ -24,7 +24,6 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <err.h>
> -#include <sys/mman.h>
>  
>  static xc_interface *gcov_xch = NULL;
>  
> @@ -35,7 +34,7 @@ static void gcov_init(void)
>          err(1, "opening interface");
>  }
>  
> -int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr)
> +int gcov_get_info(int op, struct xen_sysctl *sys, uint8_t *ptr)
>  {
>      struct xen_sysctl_coverage_op *cov;
>  
> @@ -44,7 +43,7 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr)
>  
>      cov = &sys->u.coverage_op;
>      cov->cmd = op;
> -    cov->u.raw_info.p = ptr;
> +    set_xen_guest_handle_raw(cov->u.raw_info, ptr);

This isn't right I'm afraid. Your "void *ptr" here should become "struct
xc_hypercall_buffer *ptr" and you should use
DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr) in the body of the function and
then drop the _raw from the set.

The caller of this function then needs to pass HYPERCALL_BUFFER(name) to
the function (where it looks like name == p in this case).

xc_perfc_query() and its callers are an example of this pattern.
Likewise xc_lockprof_query().

>  
>      return xc_sysctl(gcov_xch, sys);
>  }
> @@ -52,10 +51,8 @@ int gcov_get_info(int op, struct xen_sysctl *sys, void *ptr)
>  static void gcov_read(const char *fn, int reset)
>  {
>      struct xen_sysctl sys;
> -    unsigned page_size = sysconf(_SC_PAGESIZE);
>      uint32_t total_len;
> -    uint8_t *p;
> -    size_t size;
> +    DECLARE_HYPERCALL_BUFFER(uint8_t, p);
>      FILE *f;
>      int op = reset ? XEN_SYSCTL_COVERAGE_read_and_reset :
>                       XEN_SYSCTL_COVERAGE_read;
> @@ -71,12 +68,9 @@ static void gcov_read(const char *fn, int reset)
>          errx(1, "coverage size too big %u bytes\n", total_len);
>  
>      /* allocate */
> -    size = total_len + page_size;
> -    size -= (size % page_size);
> -    p = mmap(0, size, PROT_WRITE|PROT_READ,
> -             MAP_PRIVATE|MAP_ANON|MAP_LOCKED, -1, 0);
> -    if ( p == (uint8_t *) -1 )
> -        err(1, "mapping memory for coverage");
> +    p = xc_hypercall_buffer_alloc(gcov_xch, p, total_len);
> +    if ( p == NULL )
> +        err(1, "allocating memory for coverage");
>  
>      /* get data */
>      memset(p, 0, total_len);
> @@ -94,6 +88,7 @@ static void gcov_read(const char *fn, int reset)
>          err(1, "writing coverage to file");
>      if (f != stdout)
>          fclose(f);
> +    xc_hypercall_buffer_free(gcov_xch, p);
>  }
>  
>  static void gcov_reset(void)