Re: [PATCH RFC V7 4/5] xen, libxc: Request page fault injection via libxc

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 08/28/2014 04:15 PM, Tim Deegan wrote:
> At 16:49 +0100 on 26 Aug (1409068141), Jan Beulich wrote:
>>>>> On 26.08.14 at 16:56, <rcojocaru@bitdefender.com> wrote:
>>> On 08/26/2014 05:44 PM, Jan Beulich wrote:
>>>>>>> On 26.08.14 at 16:24, <rcojocaru@bitdefender.com> wrote:
>>>>> On 08/26/2014 05:13 PM, Jan Beulich wrote:
>>>>>>>>> On 13.08.14 at 17:28, <rcojocaru@bitdefender.com> wrote:
>>>>>>> --- a/xen/include/asm-x86/hvm/domain.h
>>>>>>> +++ b/xen/include/asm-x86/hvm/domain.h
>>>>>>> @@ -141,6 +141,14 @@ struct hvm_domain {
>>>>>>>       */
>>>>>>>      uint64_t sync_tsc;
>>>>>>>  
>>>>>>> +    /* Memory introspection page fault injection data. */
>>>>>>> +    struct {
>>>>>>> +        uint64_t address_space;
>>>>>>> +        uint64_t virtual_address;
>>>>>>> +        uint32_t errcode;
>>>>>>> +        bool_t valid;
>>>>>>> +    } fault_info;
>>>>>>
>>>>>> Sorry for noticing this only now, but how can this be a per-domain
>>>>>> thing rather than a per-vCPU one?
>>>>>
>>>>> The requirement for our introspection application has simply been to
>>>>> bring back in a swapped-out page, regardless of what VCPU ends up
>>>>> actually doing it.
>>>>
>>>> But please remember that what you add to the public code base
>>>> shouldn't be tied to specific needs of your application, it should
>>>> be coded in a generally useful way.
>>>
>>> Of course, perhaps I should have written "the scenario we're working
>>> with" rather than "the requirement for our application". I'm just trying
>>> to understand all the usual cases for this.
>>>
>>>> Furthermore, how would this work if you have 2 vCPU-s hit such
>>>> a condition, and you need to bring in 2 pages in parallel?
>>>
>>> Since this is all happening in the context of processing mem_events,
>>> it's not really possible for two VCPUs to need to do this in parallel,
>>> since processing mem_events is being done sequentially. A VCPU needs to
>>> put a mem_event in the ring buffer and pause before this hypercall can
>>> be called from userspace.
>>
>> I'd certainly want to hear Tim's opinion here before settling on
>> either model.
> 
> Yes, I think it's much better to have this be a per-vcpu operation; I
> see that's the way it's going already in later versions.

I've just reverted that change in V10 after discussing it with Jan. :)

I'll check into more detail if it can be done per-VCPU with no
ill-effects on our side, and if so I'll come back to the per-VCPU
implementation.


Thanks,
Razvan Cojocaru