Re: [Xen-devel] [PATCH V10 4/5] p2m: Always use hostp2m when clipping rangesets

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.11.18 at 22:59, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 11/29/18 3:58 PM, Jan Beulich wrote:
>> Altp2m-s don't matter here at all. My point is that the present,
>> unpatched p2m_change_type_range() updates the log-dirty
>> ranges with the unclipped [start,end), but calls
>> p2m->change_entry_type_range() with a possibly reduced
>> range. Any subsequent caller of p2m_is_logdirty_range() may
>> thus be mislead if the rangeset update now also used only the
>> clipped range.
> 
> I've been reading and re-reading the code and I'm still not sure I follow:
> 
>  973     if ( unlikely(end > p2m->max_mapped_pfn) )
>  974     {
>  975         if ( !gfn )
>  976         {
>  977             p2m->change_entry_type_global(p2m, ot, nt);
>  978             gfn = end;
>  979         }
>  980         end = p2m->max_mapped_pfn + 1;
> 
> end is being clipped here ...
> 
>  981     }
>  982     if ( gfn < end )
>  983         rc = p2m->change_entry_type_range(p2m, ot, nt, gfn, end - 1);
> 
> ... and the if() above is not an else if(), so if ( unlikely(end >
> p2m->max_mapped_pfn) ) we always clip end. What this new patch does in
> that regard is just making sure it uses the hostp2m's max_mapped_pfn
> instead of the altp2m's.

Oh, good point. I was focussing too much on "start", the clipping
of which is prevented by having the "gfn" local variable. And I
think current code is wrong then too (and I further think your
change then just extends badness to certain cases of "start"). So
unless this can be explained as correct behavior, I'd hope for
the situation to at least not be made worse than it is. Ideally it
would be improved, but I realize the incentive may be low as it's
presumably just a theoretical consideration.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel