From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59653) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbRfn-0000Em-U1 for qemu-devel@nongnu.org; Sun, 21 Aug 2016 08:16:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bbRfj-0000S5-OM for qemu-devel@nongnu.org; Sun, 21 Aug 2016 08:16:18 -0400 Received: from roura.ac.upc.edu ([147.83.33.10]:52084 helo=roura.ac.upc.es) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bbRfj-0000Rd-D9 for qemu-devel@nongnu.org; Sun, 21 Aug 2016 08:16:15 -0400 From: =?utf-8?Q?Llu=C3=ADs_Vilanova?= References: <147041636348.2523.2954972609232949598.stgit@fimbulvetr.bsc.es> <147041637969.2523.4570342042982870131.stgit@fimbulvetr.bsc.es> <20160818101732.GC4850@stefanha-x1.localdomain> Date: Sun, 21 Aug 2016 14:15:31 +0200 In-Reply-To: <20160818101732.GC4850@stefanha-x1.localdomain> (Stefan Hajnoczi's message of "Thu, 18 Aug 2016 11:17:32 +0100") Message-ID: <87inuupbbw.fsf@fimbulvetr.bsc.es> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Riku Voipio , qemu-devel@nongnu.org, Stefan Hajnoczi Stefan Hajnoczi writes: > On Fri, Aug 05, 2016 at 06:59:39PM +0200, Llu=C3=ADs Vilanova wrote: >> +static void init_channel(const char *base, const char *suffix, size_t s= ize, >> + char ** path, int *fd, uint64_t **addr) >> +{ >> + *path =3D g_malloc(strlen(base) + strlen(suffix) + 1); >> + sprintf(*path, "%s%s", base, suffix); > Use g_strdup_printf() instead. >> +static void swap_control(void *from, void *to) >> +{ >> + if (mprotect(from, getpagesize(), PROT_READ | PROT_WRITE) =3D=3D -1= ) { >> + error_report("error: mprotect(from): %s", strerror(errno)); >> + abort(); >> + } >> + if (mprotect(to, getpagesize(), PROT_READ) =3D=3D -1) { >> + error_report("error: mprotect(to): %s", strerror(errno)); >> + abort(); >> + } >> +} >> + >> +#include "hypertrace/emit.c" >> + >> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt) >> +{ >> + if (qemu_control_0 <=3D siginfo->si_addr && >> + siginfo->si_addr < qemu_control_1) { >> + >> + /* 1st fault (guest will write cmd) */ >> + assert(((unsigned long)siginfo->si_addr % getpagesize()) =3D=3D= sizeof(uint64_t)); >> + swap_control(qemu_control_0, qemu_control_1); >> + >> + } else if (qemu_control_1 <=3D siginfo->si_addr && >> + siginfo->si_addr < qemu_control_1 + getpagesize()) { >> + uint64_t vcontrol =3D ((uint64_t*)qemu_control_0)[2]; >> + uint64_t *data_ptr =3D &qemu_data[vcontrol * CONFIG_HYPERTRACE_= ARGS * sizeof(uint64_t)]; >> + >> + /* 2nd fault (invoke) */ >> + assert(((unsigned long)siginfo->si_addr % getpagesize()) =3D=3D= sizeof(uint64_t)); >> + hypertrace_emit(current_cpu, data_ptr); >> + swap_control(qemu_control_1, qemu_control_0); >> + >> + } else { >> + /* proxy to next handler */ >> + if (segv_next.sa_sigaction !=3D NULL) { >> + segv_next.sa_sigaction(signum, siginfo, sigctxt); >> + } else if (segv_next.sa_handler !=3D NULL) { >> + segv_next.sa_handler(signum); >> + } >> + } >> +} > Can this approach be made thread-safe? > If not then it would be good to consider the problem right away and > switch to something that is thread-safe, even if it depends on the > target architecture. Kind of. The easiest solution is to have each thread have an mmap of its ow= n of the device (and moving qemu_control_{0,1} into CPUState). This would be completely explicit and easy to understand. Cheers, Lluis