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

["Lluís Vilanova" <vilanova@ac.upc.edu>]

Stefan Hajnoczi writes:

> On Mon, Dec 26, 2016 at 09:34:54PM +0100, Lluís Vilanova wrote:
>> +static void segv_handler(int signum, siginfo_t *siginfo, void *sigctxt)
>> +{
>> +    CPUState *vcpu = current_cpu;
>> +    void *control_0 = vcpu->hypertrace_control;
>> +    void *control_1 = vcpu->hypertrace_control + config.control_size / 2;
>> +    void *control_2 = control_1 + config.control_size / 2;
>> +
>> +    if (control_0 <= siginfo->si_addr && siginfo->si_addr < control_1) {
>> +
>> +        /* 1st fault (guest will write cmd) */
>> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
>> +        swap_control(control_0, control_1);
>> +
>> +    } else if (control_1 <= siginfo->si_addr && siginfo->si_addr < control_2) {
>> +        size_t client = (siginfo->si_addr - control_1) / sizeof(uint64_t);
>> +        uint64_t vcontrol = ((uint64_t *)control_0)[client];
>> +        uint64_t *data_ptr = &qemu_data[client * config.client_data_size];
>> +
>> +        /* 2nd fault (invoke) */
>> +        assert(((unsigned long)siginfo->si_addr % sizeof(uint64_t)) == 0);
>> +        hypertrace_emit(current_cpu, vcontrol, data_ptr);
>> +        swap_control(control_1, control_0);

> A simpler and faster approach is to permanently mprotect just one region
> and load all arguments from data[] (including the first argument).  Then
> swapping isn't necessary.

I'm don't understand what you propose.

With a single protected region, you don't know when to restore protection of it
so that later accesses will be detected too. That could be solved if we used
single-stepping (maybe that's what you meant):

* trap access
* unprotect memory region
* single-step guest
* read written data and emit event
* protect memory region again
* resume guest

If the single-stepping can be done without too much complexity, that'd be a
faster option, and that piece of code might be cleaner too.

We could only avoid the protect/unprotect sequence if we added target-specific
code to "skip" the failed instruction (assuming all useful writes go to the data
channel), but I wanted to make all code target-agnostic.


Cheers,
  Lluis