Re: [PATCH V2 2/3] xen/vm_event: Support for guest-requested events

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 06/25/2015 11:37 AM, Jan Beulich wrote:
>>>> On 25.06.15 at 09:55, <rcojocaru@bitdefender.com> wrote:
>> On 06/24/2015 06:03 PM, Jan Beulich wrote:
>>>>>> On 24.06.15 at 16:56, <rcojocaru@bitdefender.com> wrote:
>>>> Would it be fair to say that HVMOP_request_vm_event should be an
>>>> exception to this rule (i.e. something along the lines of "if (
>>>> unlikely(r12 == HVMOP_request_vm_event) && orig_pc == regs->pc )",
>>>> etc.)? Even in debug mode, a GUEST_REQUEST vm_event should keep the
>>>> state of the guest intact.
>>>
>>> It's a hypercall, and hence should follow hypercall rules. If the
>>> guest wants certain registers preserved, it should do so itself.
>>
>> Right, that's sensible, hypercalls should behave in a consistent manner.
>> The only issue here is that (and I think this would prove useful for
>> many future introspection clients), the trigger VMCALL doesn't often
>> happen from an in-guest driver.
>>
>> From what I understand from our introspection logic team (maybe they'll
>> chime in), VMCALLs are being used as markers in the code the OS is
>> running: "now we've reached this function, check out the guest
>> environment", "now we've reached this other function", etc.
>>
>> They seem to be doing this often by overwriting (from the dom0
>> application) the beginning of a function that needs hooked, and the
>> larger this custom code becomes, the more problems arise. In the
>> registers clobber scenario, both registers holding HVMOP and
>> HVMOP_request_vm_event need to be saved and restored (pushed and
>> popped), and now additionally, for safety, all the clobbered registers,
>> so the necessary code has increased several times since the early days
>> of "push eax, mov eax MAGIC_CONSTANT, vmcall, pop eax".
>>
>> Hence the request to at least not clobber the registers in this case,
>> and a reason why this initally was not a hypercall although the code
>> _almost_ acted like one.
>>
>> Any suggestions (other than to just live with the situation)? :)
> 
> Even for the original code sequence you quote above it would be
> less intrusive to the patched code if they just patched in a call.
> Namely in Windows, with its reportedly pretty uniform function
> prologues, there shouldn't be too many stub variants necessary
> that such calls would target.

That makes great sense, that's very likely going to be the way to go
forward, so the patch can stay as it is.


Thanks,
Razvan