Re: [Xen-devel] [xen-unstable test] 16788: regressions - FAIL

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.03.13 at 13:42, Tim Deegan <tim@xxxxxxx> wrote:
> At 12:06 +0000 on 04 Mar (1362398796), Jan Beulich wrote:
>> >>> On 04.03.13 at 12:45, Tim Deegan <tim@xxxxxxx> wrote:
>> > At 11:29 +0000 on 04 Mar (1362396574), Jan Beulich wrote:
>> >> >>> On 04.03.13 at 12:12, Tim Deegan <tim@xxxxxxx> wrote:
>> >> > At 10:57 +0000 on 04 Mar (1362394626), Jan Beulich wrote:
>> >> >> Question - is there really no better way than this to deal with
>> >> >> cross page emulated writes?
>> >> > 
>> >> > There probably is -- all that's wanted here is two mappings that are
>> >> > guaranteed to be contiguous in virtual address.  The LDT slots were
>> >> > available; it would have been better to document this use where
>> >> > LDT_VIRT_START is defined, though.
>> >> > 
>> >> > AFAICT, we'll need to:
>> >> >  - cause some equivalent pair of PTEs to be available (either per-cpu or
>> >> >    per-vcpu); or
>> >> >  - add a map_two_domain_pages() function to use the mapcache for this; 
>> >> > or
>> >> 
>> >> Would vmap() do? Or do we expect this code path to be performance
>> >> sensitive?
>> > 
>> > I can't really tell what vmap() is for, since it seems to have no
>> > documentation, and the checkin description just says "implement vmap()".
>> > But it looks plausible, and emulating page-crossing writes oughtn't to
>> > be performance-sensitive.
>> 
>> Like this then (compile tested only so far):
> 
>> @@ -4748,14 +4743,8 @@ static void emulate_unmap_dest(struct vc
>>  
>>      if ( unlikely(mfn_valid(sh_ctxt->mfn2)) )
>>      {
>> -        unsigned long offset;
>>          paging_mark_dirty(v->domain, mfn_x(sh_ctxt->mfn2));
>> -        /* Undo the hacky two-frame contiguous map. */
>> -        ASSERT(((unsigned long) addr & PAGE_MASK) == LDT_VIRT_START(v));
>> -        offset = l1_linear_offset((unsigned long) addr);
>> -        l1e_write(&__linear_l1_table[offset], l1e_empty());
>> -        l1e_write(&__linear_l1_table[offset + 1], l1e_empty());
>> -        flush_tlb_all();
>> +        vunmap(addr - ((unsigned long)addr & ~PAGE_MASK));
> 
> I'd prefer 'vunmap((void *)((unsigned long) addr & PAGE_MASK));'.

That's fine by me, just wanted to avoid needing to cast twice.

> Otherwise, Acked-by: Tim Deegan <tim@xxxxxxx>

Having some problem testing this - not with the change itself,
but with something else that I don't understand yet
(XENMEM_add_to_physmap:XENMAPSPACE_grant_table failing
during domain construction, but only when hap=0 in the config
file).

Jan


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