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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 12/13/18 10:43 AM, Jan Beulich wrote:
>>>> On 13.12.18 at 11:22, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> For my own part, I see no reason why not clipping end should not work
>> when updating the ranges only (as long as start continues to be <=
>> unclipped_end).
>>
>> Would that modification + testing of it help this series continue?
> 
> I think so, at least as far as I'm concerned. But I think we really need
> George's opinion as well.

We are going off into the weeds a little bit here I think.

If I understand Jan's concern properly, he's concerned about a situation
like this:

[start] p2m->max_mapped_pfn == 0xfff
1. change_type_range ram => logdirty, [0x900, 0x1200)

Obviously the actual p2m entries can only be changed from 0x900 to
0xfff; but what about the logdirty ranges?  At the moment, the result
will be a rangeset with [0x900, 0xfff].

Jan is asking whether the rangeset should instead be [0x900, 0x11ff].

So the time when it would matter would be a situation like the following:

2. p2m_set_entry(0x1100, M)

3. change_entry_type_global(ram => logdirty)

4. change_entry_type_global(logdirty => ram)

Under the current regime gfn 0x1100 would be have type ram_rw both after
step 2, and after step 4.

If we used Jan's suggestion, then it would be marked as ram_rw after
step 2, and logdirty after step 4.

But of course that's no different than what would happen if
max_mapped_pfn were 0x2000, but gfns 0x1000-11ff just happened to be empty.

Under normal circumstances, neither of these situations should happen;
and in neither case will catastrophic consequences happen (unless you
were relying on hap_track_dirty_vram for something other than tracking
dirty vram).

I'm inclined to say that ideally, change_type_range should pass an error
up if end > max_mapped_pfn.

But of course, it doesn't return an error at the moment, so that's out
of scope for this series.

I take it, Jan, that in the absence of changing the behavior, you'd like
the comment to look something like this?

"Always clip the rangeset down to the host p2m.  NB that this means the
logdirty_range will also be clipped, so in the future gfns in
(host_max_pfn, end) range won't be affected by change_entry_type_global.
 We should probably return an error in this case instead, as it's almost
certainly a mistake; but that's left as a clean-up for another time."

 -George

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