Stefan Hajnoczi writes:
> 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.
Kind of. The easiest solution is to have each thread have an mmap of its own of
the device (and moving qemu_control_{0,1} into CPUState). This would be
completely explicit and easy to understand.
Cheers,
Lluis
