Re: [Xen-devel] [PATCH RFC v2 2/4] x86/mem_access: mem_access and mem_event changes to support PV domains

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.07.14 at 01:52, <aravindp@xxxxxxxxx> wrote:
>> > +static int mem_access_set_default(struct domain *d, uint64_t
>>*start_page,
>>> +                           xenmem_access_t access) {
>>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>>> +    struct page_info *page;
>>> +    struct page_list_head head;
>>> +    p2m_access_t a;
>>> +    int rc = 0, ctr = 0;
>>> +
>>> +    if ( !is_pv_domain(d) )
>>> +        return -ENOSYS;
>>> +
>>> +    ASSERT(shadow_mode_enabled(d));
>>> +
>>> +    rc = p2m_convert_xenmem_access(p2m, access, &a);
>>> +    if ( rc != 0 )
>>> +        return rc;
>>> +
>>> +    /*
>>> +     * For PV domains we only support r, rw, rx, rx2rw and rwx access
>>> +     * permissions
>>> +     */
>>> +    switch ( a )
>>> +    {
>>> +    case p2m_access_n:
>>> +    case p2m_access_w:
>>> +    case p2m_access_x:
>>> +    case p2m_access_wx:
>>> +    case p2m_access_n2rwx:
>>> +        return -EINVAL;
>>> +    default:
>>> +        break;
>>> +    }
>>> +
>>> +    paging_lock_recursive(d);
>>> +
>>> +    if ( *start_page )
>>> +    {
>>> +        head.next = (struct page_info *)*start_page;
>>
>>What guarantees that the continuation page is still on d->page_list, or that
>>now other page got inserted ahead of it? And anyway you're iterating the list
>>without holding d->page_alloc_lock.
> 
> Good point. Should I grab the lock and release it only when the hypercall 
> completes?

If "completes" means after any eventual continuation, then you
should be able to answer this with "no" yourself. If "completes" is
meant only up to the next continuation, then this wouldn't help
you anyway. IOW this needs a more sophisticated solution, or
you need to restrict the memory size of guests that can be subject
to mem-access handling.

Jan


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