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

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 08/12/2014 06:16 PM, Jan Beulich wrote:
>>>> On 11.08.14 at 17:08, <rcojocaru@bitdefender.com> wrote:
>> +    if ( (curr->arch.hvm_vcpu.guest_cr[3] & mask) !=
>> +         (currd->arch.hvm_domain.fault_info.address_space & mask) )
>> +        return 0;
>> +
>> +    vmx_vmcs_enter(curr);
>> +    __vmread(VM_ENTRY_INTR_INFO, &ev);
>> +
>> +    if ( (ev & INTR_INFO_VALID_MASK) &&
>> +         hvm_event_needs_reinjection(MASK_EXTR(ev, INTR_INFO_INTR_TYPE_MASK),
>> +                                     ev & INTR_INFO_VECTOR_MASK) )
>> +        pending_event = ev;
>> +
>> +    vmx_vmcs_exit(curr);
> 
> Again - is there a reason not to move this right after the __vmread()
> above?

No, sorry, I've missed that. No reason, I'll move it.

>> +    uint64_t virtual_address = currd->arch.hvm_domain.fault_info.virtual_address;
>> +
>> +    currd->arch.hvm_domain.fault_info.valid = 0;
>> +
>> +    if ( currd->arch.hvm_domain.fault_info.write_access )
>> +        errcode |= PFEC_write_access;
> 
> Isn't this pretty limited a set of error codes you're able to generate
> here?

For the purpose of simply bringing in pages that the guest OS has
swapped out for inspection it's been enough.

I've thought about making this a parameter to the new libxc function,
but it seems that the PFEC_... codes didn't make it into the public Xen
headers.

>> --- 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 write_access;
> 
> Is this meaningfully a 32-bit field (rather than just a boolean one)?

No, it's just a boolean one. I'll modify the code to properly reflect that.


Thanks,
Razvan Cojocaru