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

["Jan Beulich" <JBeulich@suse.com>]

>>> On 29.11.18 at 14:23, <rcojocaru@bitdefender.com> wrote:
> On 11/29/18 12:04 PM, Jan Beulich wrote:
>>>>> On 28.11.18 at 22:56, <rcojocaru@bitdefender.com> wrote:
>>> Changes since V9:
>>>  - Removed the patch RFC (replaced by a printk(XENLOG_G_WARNING).
>>>  - Reused start and end in change_type_range() and removed the
>>>    intermediary variables range_start and range_end.
>>>  - Added an extra explanation for the if ( start > end ) return;
>>>    code in the comment.
>> 
>> This last item isn't really taking care of the comments I gave on v9.
>> The _incoming_ start being larger than the _incoming_ end is
>> something worth to point out. But you put that check after clipping
>> end. Furthermore it looks like you continue to break the case
>> where ->max_mapped_pfn increases subsequently, i.e. you still
>> don't update the rangeset with the unmodified incoming values.
>> Or otherwise I would have expected an explanation (as a reply
>> to my comments, not necessarily by adding to description or
>> comments of the patch here) why either this is not an issue or I'm
>> misreading anything.
> 
> max_mapped_pfn _should_ end up being >= the logdirty range upper bound,
> since AFAICT the logdirty ranges are tied to ept_set_entry() calls,
> which always end up calling p2m_altp2m_propagate_change() when they
> occur on the hostp2m (which in turn calls p2m_set_entry() on the
> altp2ms, and so on).

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.

> Also, apologies if I'm speaking out of turn and the question was really
> addressed to George (who is the proper author of the patch).

That's okay, since you've made the change in question. I did
expect George to reply to my comments, but you volunteering
to take care of addressing them is fine with me.

Jan



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