Re: [Xen-devel] [PATCH] nestedhvm: fix write access fault on ro mapping

[Christoph Egger <Christoph.Egger@xxxxxxx>]

On 08/02/12 12:45, Tim Deegan wrote:

> At 13:57 +0200 on 27 Jul (1343397454), Christoph Egger wrote:
>>>> @@ -1291,6 +1291,8 @@ int hvm_hap_nested_page_fault(unsigned l
>>>>              if ( !handle_mmio() )
>>>>                  hvm_inject_hw_exception(TRAP_gp_fault, 0);
>>>>              return 1;
>>>> +        case NESTEDHVM_PAGEFAULT_READONLY:
>>>> +            break;
>>>
>>> Don't we have to translate the faulting PA into an L1 address before
>>> letting the rest of this fault handler run?  It explicitly operates on
>>> the hostp2m.  
>>>
>>> If we do that, we should probably do it for NESTEDHVM_PAGEFAULT_ERROR,
>>> rather than special-casing READONLY.  That way any other
>>> automatically-fixed types (like the p2m_access magic) will be covered
>>> too.
>>
>> How do you differentiate if the error happened from walking l1 npt or
>> host npt ?
>> In the first case it isn't possible to provide l1 address.
> 
> It must be _possible_; after all we managed to detect the error. :)  In
> any case it's definitely wrong to carry on with this handler with the
> wrong address in hand.  So I wonder why this patch actually works for
> you.  Does replacing the 'break' above with 'return 1' also fix the
> problem?


No. Two things have to happen:

1. Calling paging_mark_dirty() and
2. using the same p2mt from the hostp2m in the nestedp2m.

>

> In the short term, do you only care about pages that are read-only for
> log-dirty tracking?  For the L1 walk, that should be handled by the PT
> walker's own calls to paging_mark_dirty(), and the nested-p2m handler
> could potentially take care of the other case by calling
> paging_mark_dirty() (for writes!) before calling nestedhap_walk_L0_p2m().


Ok, I consider this as a performance improvement rather a bugfix.

New version is attached.

Christoph

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Attachment: xen_nh_p2m.diff
Description: xen_nh_p2m.diff

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