On 12/13/18 10:43 AM, Jan Beulich wrote: >>>> On 13.12.18 at 11:22, <rcojoc...@bitdefender.com> 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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
