Re: [Xen-devel] [PATCH v2 09/12] x86/altp2m: add remaining support routines.

[Ed White <edmund.h.white@xxxxxxxxx>]

On 06/25/2015 10:42 AM, Lengyel, Tamas wrote:
> On Thu, Jun 25, 2015 at 12:31 PM, Ed White <edmund.h.white@xxxxxxxxx> wrote:
> 
>> On 06/24/2015 07:44 PM, Lengyel, Tamas wrote:
>>>> +    if ( altp2m_active )
>>>> +    {
>>>> +        if ( altp2mhvm_hap_nested_page_fault(v, gpa, gla, npfec, &p2m)
>> ==
>>>> 1 )
>>>> +        {
>>>> +            /* entry was lazily copied from host -- retry */
>>>>
>>>
>>> So I'm not fully following this logic here. I can see that the altp2m
>> entry
>>> got copied from the host. Why is there a need for the retry, why not just
>>> continue?
>>
>> At this point the EPT's that the hardware is using have been made valid
>> by software, but the hardware has already failed the access so you have
>> to restart the operation. This isn't in any way specific to altp2m,
>> it's how page fault logic works generally.
>>
>> Ed
>>
> 
> Oh I see, you are working with the assumption that the fault was triggered
> by the entry not being present in the altp2m EPT, thus it's enough to copy
> it to resolve the fault. However, if the hostp2m permissions are
> restricted, there will be a follow-up fault again. Would it maybe make
> sense to check for that condition and save having to hit two faults?

It's not an assumption, it's a fact because the altp2m nested page fault
handler returns 1 IFF it has copied from the host p2m.

Once again this is standard page fault handling. Preemptively checking
for a condition that would cause another fault shortens the path for
cases that would re-fault, but lengthens it for all the cases that would
not. In a typical scenario (which your current experiments are not) you
expect most cases not to re-fault. The cases that do re-fault are much
more expensive anyway.

There are other reasons not to preemptively check, but that's the most
straightforward one.

Ed


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel