Re: [Xen-devel] [PATCH] x86/mm: Take the p2m lock even in shadow mode.

[Andres Lagar-Cavilla <andreslc@xxxxxxxxxxxxxx>]

On Feb 21, 2013, at 12:42 PM, Tim Deegan <tim@xxxxxxx> wrote:

> At 09:58 -0500 on 21 Feb (1361440689), Andres Lagar-Cavilla wrote:
>>> The remaining locked lookups are in sh_page_fault() (in a path that's
>>> almost always already serializing on the paging lock), and in
>>> emulate_map_dest() (which can probably be updated to use
>>> get_page_from_gfn()).
>> That is absolutely the case. Here is a rough patch:
> 
> Applied, thanks.  I've taken your S-o-b below to apply to this patch
> too; hope that's OK. 

Cool, thanks
Andres

> 
> Tim.
> 
>> diff -r 7324955b35ad xen/arch/x86/mm/shadow/multi.c
>> --- a/xen/arch/x86/mm/shadow/multi.c
>> +++ b/xen/arch/x86/mm/shadow/multi.c
>> @@ -4861,6 +4861,7 @@ static mfn_t emulate_gva_to_mfn(struct v
>>                                 struct sh_emulate_ctxt *sh_ctxt)
>> {
>>     unsigned long gfn;
>> +    struct page_info *page;
>>     mfn_t mfn;
>>     p2m_type_t p2mt;
>>     uint32_t pfec = PFEC_page_present | PFEC_write_access;
>> @@ -4878,22 +4879,30 @@ static mfn_t emulate_gva_to_mfn(struct v
>> 
>>     /* Translate the GFN to an MFN */
>>     ASSERT(!paging_locked_by_me(v->domain));
>> -    mfn = get_gfn(v->domain, _gfn(gfn), &p2mt);
>> -        
>> +    page = get_page_from_gfn(v->domain, gfn, &p2mt, P2M_ALLOC);
>> +
>> +    /* Sanity checking */
>> +    if ( page == NULL )
>> +    {
>> +        return _mfn(BAD_GFN_TO_MFN);
>> +    }
>>     if ( p2m_is_readonly(p2mt) )
>>     {
>> -        put_gfn(v->domain, gfn);
>> +        put_page(page);
>>         return _mfn(READONLY_GFN);
>>     }
>>     if ( !p2m_is_ram(p2mt) )
>>     {
>> -        put_gfn(v->domain, gfn);
>> +        put_page(page);
>>         return _mfn(BAD_GFN_TO_MFN);
>>     }
>> -
>> +    mfn = page_to_mfn(page);
>>     ASSERT(mfn_valid(mfn));
>> +
>>     v->arch.paging.last_write_was_pt = !!sh_mfn_is_a_page_table(mfn);
>> -    put_gfn(v->domain, gfn);
>> +    /* Note shadow cannot page out or unshare this mfn, so the map won't
>> +     * disappear. Otherwise, caller must hold onto page until done. */
>> +    put_page(page);
>>     return mfn;
>> }
>> 
>> 
>> 
>>> They're not addressed here but may be in a
>>> follow-up patch.
>>> 
>> 
>> Acked-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>
>> or Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx>? see 
>> http://lists.xen.org/archives/html/xen-devel/2012-04/msg00962.html
>> 


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