[Xen-devel] RE: [PATCH 13/13] Nested Virtualiztion: hap-on-hap

["Dong, Eddie" <eddie.dong@xxxxxxxxx>]

Christoph Egger wrote:
> On Wednesday 08 September 2010 16:04:51 Tim Deegan wrote:
>> Hi,
>> 
>> This is looking better - I think the TLB flushing is almost right.
>> A few more detailed comments below.
>> 
>> Content-Description: xen_nh13_haphap.diff
>> 
>>> # HG changeset patch
>>> # User cegger
>>> # Date 1283345895 -7200
>>> Implement Nested-on-Nested.
>>> This allows the guest to run nested guest with hap enabled.
>>> 
>>> diff -r 50b3e6c73d7c -r 24daea64bdff xen/arch/x86/hvm/hvm.c ---
>>> a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1042,12 +1042,56 @@ void hvm_inject_exception(unsigned int t
>>>      hvm_funcs.inject_exception(trapnr, errcode, cr2);  }
>>> 
>>> -bool_t hvm_hap_nested_page_fault(unsigned long gfn)
>>> +bool_t hvm_hap_nested_page_fault(paddr_t gpa, struct cpu_user_regs
>>>      *regs) { p2m_type_t p2mt;
>>>      mfn_t mfn;
>>>      struct vcpu *v = current;
>>>      struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
>>> +    unsigned long gfn = gpa >> PAGE_SHIFT;
>>> +    int rv;
>>> +
>>> +    /* On Nested Virtualization, walk the guest page table.
>>> +     * If this succeeds, all is fine.
>>> +     * If this fails, inject a nested page fault into the guest. +
>>> */ +    if ( nestedhvm_enabled(v->domain)
>>> +        && nestedhvm_vcpu_in_guestmode(v)
>>> +        && nestedhvm_paging_mode_hap(v) )
>>> +    {
>>> +        enum nestedhvm_vmexits nsret;
>>> +        struct nestedhvm *hvm = &vcpu_nestedhvm(v); +
>>> +        /* nested guest gpa == guest gva */
>> 
>> This comment is confusing to me.  The nested guest gpa isn't a
>> virtual address, and certainly not the same as an L1-guest VA.  Can
>> you reword it, maybe just to say what the call to
>> nestedhvm_hap_nested_page_fault() is going to do?
> 
> New wording is:
> 
>   /* The vcpu is in guest mdoe and the l1 guest
>    * uses hap. That means 'gpa' is in l2 guest
>    * physical adderss space.
>    * Fix the nested p2m or inject nested page fault
>    * into l1 guest if not fixable. The algorithm is
>    * the same as for shadow paging.
>    */
> 
> Is this fine with you ?

wording is always a challenge in nested virtualization :(

I have similar feeling and thinking. In all the explaination text, we use the 
term l1 guest, l2 guest which makes everybody easy to understand, but in the 
code we are avoiding those clear prefix both here and in Qing's patch. How 
about we use l1/l2 prefix more to explicitly differentiate among them? Just 2 
cents, it may be too later.


Thx, Eddie
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel