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

From: Razvan Cojocaru <rcojocaru@bitdefender.com>
To: Jan Beulich <JBeulich@suse.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	george.dunlap@citrix.com,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: [PATCH V10 4/5] p2m: Always use hostp2m when clipping rangesets
Date: Thu, 29 Nov 2018 15:23:05 +0200	[thread overview]
Message-ID: <5543e709-6dd0-4c88-ad87-5837490ea441@bitdefender.com> (raw)
In-Reply-To: <5BFFB9C0020000780020119C@prv1-mh.provo.novell.com>

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).

Long story short, all modifications to the hostp2m's max_mapped_pfn will
end up updating it for all active altp2ms. The other way around is not
true if I understand the code correctly, so it is theoretically possible
for altp2m->max_mapped_pfn > hostp2m->max_mapped_pfn (although, again
AFAICT, this should not really affect the logdirty case where we're now
doing our best to keep the hostp2m in sync with altp2ms).

So this is why I believe that this is not an issue, however I might be
missing something or am (quite likely) possibly misunderstanding the
question.

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).

Thanks,
Razvan

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