Re: [PATCH v10 6/6] x86/ioreq server: Synchronously reset outstanding p2m_ioreq_server entries when an ioreq server unmaps.

[Yu Zhang <yu.c.zhang@linux.intel.com>]

On 4/3/2017 11:23 PM, Jan Beulich wrote:
>>>> On 02.04.17 at 14:24, <yu.c.zhang@linux.intel.com> wrote:
>> After an ioreq server has unmapped, the remaining p2m_ioreq_server
>> entries need to be reset back to p2m_ram_rw. This patch does this
>> synchronously by iterating the p2m table.
>>
>> The synchronous resetting is necessary because we need to guarantee
>> the p2m table is clean before another ioreq server is mapped. And
>> since the sweeping of p2m table could be time consuming, it is done
>> with hypercall continuation.
>>
>> Signed-off-by: Yu Zhang <yu.c.zhang@linux.intel.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit I think ...
>
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -1031,6 +1031,35 @@ void p2m_change_type_range(struct domain *d,
>>       p2m_unlock(p2m);
>>   }
>>   
>> +/* Synchronously modify the p2m type for a range of gfns from ot to nt. */
>> +void p2m_finish_type_change(struct domain *d,
>> +                            gfn_t first_gfn, unsigned long max_nr,
>> +                            p2m_type_t ot, p2m_type_t nt)
>> +{
>> +    struct p2m_domain *p2m = p2m_get_hostp2m(d);
>> +    p2m_type_t t;
>> +    unsigned long gfn = gfn_x(first_gfn);
>> +    unsigned long last_gfn = gfn + max_nr - 1;
>> +
>> +    ASSERT(ot != nt);
>> +    ASSERT(p2m_is_changeable(ot) && p2m_is_changeable(nt));
>> +
>> +    p2m_lock(p2m);
>> +
>> +    last_gfn = min(last_gfn, p2m->max_mapped_pfn);
>> +    while ( gfn <= last_gfn )
>> +    {
>> +        get_gfn_query_unlocked(d, gfn, &t);
>> +
>> +        if ( t == ot )
>> +            p2m_change_type_one(d, gfn, t, nt);
>> +
>> +        gfn++;
>> +    }
>> +
>> +    p2m_unlock(p2m);
>> +}
> ... this could be improved for reduction of overall latency: A fixed
> limit as input if not very useful, as it does matter quite a bit whether
> need to call p2m_change_type_one(). Doing 256 iteration with no
> single call is going to be pretty fast I think, so it would be desirable
> to weigh differently iterations with and without call to that function.

Oh, thanks for your advice, very enlightening.
With code freeze date approaching, how about we solve this in a separate 
patch? :)
I've drafted one, and will send it out in another mail. Hope the 
direction is correct.

B.R.
Yu

> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel


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