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

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

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 608 bytes --]

tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED.

From: Miguel Clara <miguelmclara@gmail.com>
Signed-off-by: Christoph Egger <chegger@amazon.de>

diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c
--- a/tools/misc/xencov.c	Fri May 10 13:45:02 2013 +0000
+++ b/tools/misc/xencov.c	Thu May 30 12:40:13 2013 +0000
@@ -26,6 +26,11 @@
 #include <err.h>
 #include <sys/mman.h>
 
+#ifdef __NetBSD__
+/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */
+#define MAP_LOCKED MAP_WIRED
+#endif
+
 static xc_interface *gcov_xch = NULL;
 
 static void gcov_init(void)

[-- Attachment #2: patch_xencov.diff --]
[-- Type: text/plain, Size: 794 bytes --]

# HG changeset patch
# User Christoph Egger <chegger@amazon.de>
# Date 1369917613 0
# Node ID 97a4f7e36a8fb0bc37f8a6dc04a0e33b1bc9835d
# Parent  f1fa13daf4b78b027f081aca5be24cfa96813b42
tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED.

From: Miguel Clara <miguelmclara@gmail.com>
Signed-off-by: Christoph Egger <chegger@amazon.de>

diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c
--- a/tools/misc/xencov.c	Fri May 10 13:45:02 2013 +0000
+++ b/tools/misc/xencov.c	Thu May 30 12:40:13 2013 +0000
@@ -26,6 +26,11 @@
 #include <err.h>
 #include <sys/mman.h>
 
+#ifdef __NetBSD__
+/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */
+#define MAP_LOCKED MAP_WIRED
+#endif
+
 static xc_interface *gcov_xch = NULL;
 
 static void gcov_init(void)

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

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

[Ian Campbell]

On Thu, 2013-05-30 at 14:46 +0200, Christoph Egger wrote:
> tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED.

It looks to me as if xencov.c's shouldn't be using mmap in the first
place. It should be using the libxc hypercall buffer code.

I think xencov.c should be using xc_hypercall_buffer_alloc(_pages) and
gcov_get_info should be xc_gcov_get_info & inside libxc not open coded
in this tool. Not least because of its abuse of the GUEST_HANDLE
interface (going directly at .p instead of using xen_set_guest_handle(),
which is forbidden).

Frediano, do you have time to sort this out?

NB On Linux the hypercall buffers currently turn into MAP_PRIVATE|
MAP_ANONYMOUS|MAP_LOCKED but on netbsd it is just xc_memalign.
Presumably netbsd doesn't have to worry about the hypercall buffer being
unmapped/swapped/paged/etc under its feet? Or if it does it needs to be
fixed in libxc...

Thanks,
Ian.

> From: Miguel Clara <miguelmclara@gmail.com>
> Signed-off-by: Christoph Egger <chegger@amazon.de>
> 
> diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c
> --- a/tools/misc/xencov.c	Fri May 10 13:45:02 2013 +0000
> +++ b/tools/misc/xencov.c	Thu May 30 12:40:13 2013 +0000
> @@ -26,6 +26,11 @@
>  #include <err.h>
>  #include <sys/mman.h>
>  
> +#ifdef __NetBSD__
> +/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */
> +#define MAP_LOCKED MAP_WIRED
> +#endif
> +
>  static xc_interface *gcov_xch = NULL;
>  
>  static void gcov_init(void)

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

[Frediano Ziglio]

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);
 
     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)
-- 
1.7.10.4


On Fri, 2013-05-31 at 12:25 +0100, Ian Campbell wrote:
> On Thu, 2013-05-30 at 14:46 +0200, Christoph Egger wrote:
> > tools/xencov: MAP_LOCKED is Linux specific. NetBSD's equivalent is MAP_WIRED.
> 
> It looks to me as if xencov.c's shouldn't be using mmap in the first
> place. It should be using the libxc hypercall buffer code.
> 
> I think xencov.c should be using xc_hypercall_buffer_alloc(_pages) and
> gcov_get_info should be xc_gcov_get_info & inside libxc not open coded
> in this tool. Not least because of its abuse of the GUEST_HANDLE
> interface (going directly at .p instead of using xen_set_guest_handle(),
> which is forbidden).
> 
> Frediano, do you have time to sort this out?
> 
> NB On Linux the hypercall buffers currently turn into MAP_PRIVATE|
> MAP_ANONYMOUS|MAP_LOCKED but on netbsd it is just xc_memalign.
> Presumably netbsd doesn't have to worry about the hypercall buffer being
> unmapped/swapped/paged/etc under its feet? Or if it does it needs to be
> fixed in libxc...
> 
> Thanks,
> Ian.
> 
> > From: Miguel Clara <miguelmclara@gmail.com>
> > Signed-off-by: Christoph Egger <chegger@amazon.de>
> > 
> > diff -r f1fa13daf4b7 -r 97a4f7e36a8f tools/misc/xencov.c
> > --- a/tools/misc/xencov.c	Fri May 10 13:45:02 2013 +0000
> > +++ b/tools/misc/xencov.c	Thu May 30 12:40:13 2013 +0000
> > @@ -26,6 +26,11 @@
> >  #include <err.h>
> >  #include <sys/mman.h>
> >  
> > +#ifdef __NetBSD__
> > +/* MAP_LOCKED is Linux specific. MAP_WIRED is NetBSD's equivalent. */
> > +#define MAP_LOCKED MAP_WIRED
> > +#endif
> > +
> >  static xc_interface *gcov_xch = NULL;
> >  
> >  static void gcov_init(void)
> 
> 

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

[Ian Campbell]

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)

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

[Frediano Ziglio]

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 |   22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 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..fb4b2ff 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,16 +34,17 @@ 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, struct xc_hypercall_buffer *ptr)
 {
     struct xen_sysctl_coverage_op *cov;
+    DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr);
 
     memset(sys, 0, sizeof(*sys));
     sys->cmd = XEN_SYSCTL_coverage_op;
 
     cov = &sys->u.coverage_op;
     cov->cmd = op;
-    cov->u.raw_info.p = ptr;
+    set_xen_guest_handle(cov->u.raw_info, ptr);
 
     return xc_sysctl(gcov_xch, sys);
 }
@@ -52,10 +52,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,16 +69,13 @@ 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);
-    if ( gcov_get_info(op, &sys, p) < 0 )
+    if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 )
         err(1, "getting coverage information");
 
     /* write to a file */
@@ -94,6 +89,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)
-- 
1.7.10.4

On Wed, 2013-06-12 at 10:21 +0100, Ian Campbell wrote:
> 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)
> 
> 

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

[Ian Campbell]

On Wed, 2013-06-12 at 13:02 +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>

Looks good to me, thanks,

Acked-by: Ian Campbell <ian.campbell@citrix.com>

George, what is your feeling for the release? This is a somewhat obscure
tool and IMHO the risk of regressing is small. The change fixes the
default build on NetBSD.

> ---
>  tools/misc/xencov.c |   22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> Here you are. It get tested on a x64 machine, if somebody could test it
> with a NetBSD would be great.

That would certainly be useful and appreciated although since it
currently doesn't build on NetBSD there is not much chance of regressing
there so I'll probably apply (subject to George's approval) in a day or
so regardless (unless the test is done and fails of course).


> 
> diff --git a/tools/misc/xencov.c b/tools/misc/xencov.c
> index 6645a30..fb4b2ff 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,16 +34,17 @@ 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, struct xc_hypercall_buffer *ptr)
>  {
>      struct xen_sysctl_coverage_op *cov;
> +    DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr);
>  
>      memset(sys, 0, sizeof(*sys));
>      sys->cmd = XEN_SYSCTL_coverage_op;
>  
>      cov = &sys->u.coverage_op;
>      cov->cmd = op;
> -    cov->u.raw_info.p = ptr;
> +    set_xen_guest_handle(cov->u.raw_info, ptr);
>  
>      return xc_sysctl(gcov_xch, sys);
>  }
> @@ -52,10 +52,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,16 +69,13 @@ 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);
> -    if ( gcov_get_info(op, &sys, p) < 0 )
> +    if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 )
>          err(1, "getting coverage information");
>  
>      /* write to a file */
> @@ -94,6 +89,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)

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

[George Dunlap]

On 12/06/13 13:14, Ian Campbell wrote:
> On Wed, 2013-06-12 at 13:02 +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>
> Looks good to me, thanks,
>
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
>
> George, what is your feeling for the release? This is a somewhat obscure
> tool and IMHO the risk of regressing is small. The change fixes the
> default build on NetBSD.

Did we even have gcov in 4.2?  Anyway I agree, the benefits are worth 
more than the risk.

Re the release:
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

  -George

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

[Ian Campbell]

On Wed, 2013-06-12 at 14:13 +0100, George Dunlap wrote:
> On 12/06/13 13:14, Ian Campbell wrote:
> > On Wed, 2013-06-12 at 13:02 +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>
> > Looks good to me, thanks,
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > George, what is your feeling for the release? This is a somewhat obscure
> > tool and IMHO the risk of regressing is small. The change fixes the
> > default build on NetBSD.
> 
> Did we even have gcov in 4.2?

Good point, no we didn't.

>   Anyway I agree, the benefits are worth 
> more than the risk.
> 
> Re the release:
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
>   -George
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Frediano Ziglio]

On Wed, 2013-06-12 at 14:13 +0100, George Dunlap wrote:
> On 12/06/13 13:14, Ian Campbell wrote:
> > On Wed, 2013-06-12 at 13:02 +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>
> > Looks good to me, thanks,
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > George, what is your feeling for the release? This is a somewhat obscure
> > tool and IMHO the risk of regressing is small. The change fixes the
> > default build on NetBSD.
> 
> Did we even have gcov in 4.2?  Anyway I agree, the benefits are worth 
> more than the risk.
> 

No, was introduced for 4.3.

> Re the release:
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
>   -George
> 

Frediano

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

[Egger, Christoph]

On 12.06.13 14:02, 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>

Acked-by: Christoph Egger <chegger@amazon.de>

> ---
>  tools/misc/xencov.c |   22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 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..fb4b2ff 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,16 +34,17 @@ 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, struct xc_hypercall_buffer *ptr)
>  {
>      struct xen_sysctl_coverage_op *cov;
> +    DECLARE_HYPERCALL_BUFFER_ARGUMENT(ptr);
>  
>      memset(sys, 0, sizeof(*sys));
>      sys->cmd = XEN_SYSCTL_coverage_op;
>  
>      cov = &sys->u.coverage_op;
>      cov->cmd = op;
> -    cov->u.raw_info.p = ptr;
> +    set_xen_guest_handle(cov->u.raw_info, ptr);
>  
>      return xc_sysctl(gcov_xch, sys);
>  }
> @@ -52,10 +52,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,16 +69,13 @@ 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);
> -    if ( gcov_get_info(op, &sys, p) < 0 )
> +    if ( gcov_get_info(op, &sys, HYPERCALL_BUFFER(p)) < 0 )
>          err(1, "getting coverage information");
>  
>      /* write to a file */
> @@ -94,6 +89,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)
> 

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

[Ian Campbell]

On Wed, 2013-06-12 at 14:13 +0100, George Dunlap wrote:
> On 12/06/13 13:14, Ian Campbell wrote:
> > On Wed, 2013-06-12 at 13:02 +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>
> > Looks good to me, thanks,
> >
> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >
> > George, what is your feeling for the release? This is a somewhat obscure
> > tool and IMHO the risk of regressing is small. The change fixes the
> > default build on NetBSD.
> 
> Did we even have gcov in 4.2?  Anyway I agree, the benefits are worth 
> more than the risk.
> 
> Re the release:
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>

This was a little while ago, but I've committed.