From mboxrd@z Thu Jan 1 00:00:00 1970 From: Frediano Ziglio Subject: Re: [PATCH] tools/misc/xencov.c: Use MAP_WIRED on NetBSD Date: Tue, 11 Jun 2013 14:46:56 +0100 Message-ID: <1370958416.1654.3.camel@hamster.uk.xensource.com> References: <51A74A37.5070502@amazon.de> <1369999519.5199.99.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1369999519.5199.99.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Frediano Ziglio , Christoph Egger , Miguel Clara , xen-devel List-Id: xen-devel@lists.xenproject.org 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 --- 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 #include #include -#include 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 > > Signed-off-by: Christoph Egger > > > > 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 > > #include > > > > +#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) > >