Re: [Xen-devel] [PATCH RFC v2 1/4] x86/mm: Shadow and p2m changes for PV mem_access

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.08.14 at 21:48, <aravindp@xxxxxxxxx> wrote:
>>>> As Andrew already pointed out, you absolutely need to deal with page
>>>> crossing accesses,
>>> Is this for say an unsigned long that lives across two pages? Off the top of
>>my head, I think always allowing writes to the page in question and the next
>>followed by reverting to default for both pages at the end of the write 
> should
>>take care of this. I would have to walk the page tables to figure out the 
> next
>>mfn. Or am I on the wrong track here?
>>
>>create_bounce_frame puts several adjacent words on a guest stack, and this
>>is very capable of crossing a page boundary.
>>
>>Even an unaligned uint16_t can cross a page boundary.
> 
> OK, so marking two adjacent pages as writable and reverting after the write 
> went through should solve this problem.

For create_bounce_frame() yes, but not for the generic
copy_to_user(). But there you'd have the option of reverting the
first page's permissions when you hit a fault on the third one (i.e.
two slots for tracking pages should still suffice).

>>>>> +                if ( guest_l1e_get_flags(gw.l1e) & _PAGE_RW )
>>>>>                  {
>>>>> -                    cr0 &= ~X86_CR0_WP;
>>>>> -                    write_cr0(cr0);
>>>>> -                    v->arch.pv_vcpu.need_cr0_wp_set = 1;
>>>>> +                    domain_pause_nosync(d);
>>>> I don't think a "nosync" pause is enough here, as that leaves a
>>>> window for the guest to write to the page. Since the sync version may
>>>> take some time to complete it may become difficult for you to
>>>> actually handle this in an acceptable way.
>>> Are you worried about performance or is there some other issue?
>>
>>Both performance and correctness.  With nosync(), guest vcpus can still be
>>running on other pcpus, and playing with this pagetable entry.
>>
>>The synchronous variants can block for a moderate period of time.
> 
> OK, I don't follow why pausing the other vcpus synchronously is an issue 
> here.

So what is you don't understand? The remote vCPU-s won't
necessarily be paused by the time domain_pause_nosync()
returns (that's the nature of the "nosync"). Since the page table
entry then gets modified, those remote vCPU-s might access
memory using that entry, and the listener wouldn't see the write
access. And when you use the synchronous form, the waiting for
the remote vCPU-s to become de-scheduled will take some time,
which is particularly bad for many-vCPU guests with the way
domain_pause() currently works (of course this could be improved
by parallelizing the pausing of the individual vCPU-s, first issuing
vcpu_sleep_nosync() on all of them and then waiting for all of
them to become non-running).

Jan


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