Re: [Qemu-devel] [PATCH 3/6] hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event

[Stefan Hajnoczi]

On Fri, Aug 05, 2016 at 06:59:39PM +0200, Lluís Vilanova wrote:
> +static void init_channel(const char *base, const char *suffix, size_t size,
> +                         char ** path, int *fd, uint64_t **addr)
> +{
> +    *path = 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) == -1) {
> +        error_report("error: mprotect(from): %s", strerror(errno));
> +        abort();
> +    }
> +    if (mprotect(to, getpagesize(), PROT_READ) == -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 <= siginfo->si_addr &&
> +        siginfo->si_addr < qemu_control_1) {
> +
> +        /* 1st fault (guest will write cmd) */
> +        assert(((unsigned long)siginfo->si_addr % getpagesize()) == 
> sizeof(uint64_t));
> +        swap_control(qemu_control_0, qemu_control_1);
> +
> +    } else if (qemu_control_1 <= siginfo->si_addr &&
> +               siginfo->si_addr < qemu_control_1 + getpagesize()) {
> +        uint64_t vcontrol = ((uint64_t*)qemu_control_0)[2];
> +        uint64_t *data_ptr = &qemu_data[vcontrol * CONFIG_HYPERTRACE_ARGS * 
> sizeof(uint64_t)];
> +
> +        /* 2nd fault (invoke) */
> +        assert(((unsigned long)siginfo->si_addr % getpagesize()) == 
> 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 != NULL) {
> +            segv_next.sa_sigaction(signum, siginfo, sigctxt);
> +        } else if (segv_next.sa_handler != 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.

signature.asc
Description: PGP signature