Re: [Xen-devel] [PATCH V2] x86/p2m: fixed p2m_change_type_range() start / end check

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 04/23/2018 02:47 PM, George Dunlap wrote:
> On 04/18/2018 02:12 PM, Razvan Cojocaru wrote:
>> p2m_change_type_range() handles end > max_mapped_pfn, but not
>> start > max_mapped_pfn. Check the latter just after grabbing the
>> lock and bail if true.
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> 
> Sorry, I meant to reply to this earlier but I haven't been able to make
> the time.
> 
> On reflection, I think this is the wrong approach actually.  First, my
> assertion was incorrect: the p2m_altp2m_propagate_change() is gated on
> p2m->max_remapped_gfn, not max_mapped_gfn (nb the 're').  So setting
> max_mapped_gfn shouldn't cause 'unnecessary' propagations.
> 
> Secondly, we do actually need to keep the logdirty ranges of all the
> p2ms in sync, even if they're past the max_remapped_gfn.  Otherwise we
> could have the following situation:
> * altp2m created, max_remapped_gfn 0x1000
> * screen resized, logdirty range [0x2000-0x3000]; change dropped
> * guest accesses 0x4000, max_remapped_gfn set to 0x4000
> * change_p2m_type happens, and the 0x2000-0x3000 range is not marked
> logrdirty #
> 
> So while it would be an improvement to make the assertion more explicit,
> I don't (anymore) think it would actually be an improvement to discard
> changes that are above max_mapped_gfn.  (And thus your original patch,
> which copied max_mapped_gfn into the altp2ms, was probably closer to the
> right approach).
> 
> Sorry for the confusion -- we obviously need a bit more thought about
> how altp2m and logdirty interact.

Thanks for the reply! Fair enough.

FWIW, the attached patch works well for me, resizes and all (but it
could very well be just luck).


Thanks,
Razvan

Attachment: altp2m_logdirty4.patch
Description: Text Data

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