On 09/19/2018 03:09 PM, Razvan Cojocaru wrote: > On 9/19/18 3:15 PM, George Dunlap wrote: >>> * has p2m_init_altp2m_ept() copy over max_mapped_pfn, >>> logdirty_ranges, global_logdirty, ept.ad and default_access >>> from the hostp2m (the latter more for completeness than for any >>> other reason). >> I think this is probably the right approach. These values change >> rarely, but after a misconfig are read repeatedly. So it's probably a >> lot more efficient to propagate changes when they happen, rather than >> trying to keep a single master copy. However... >> >>> We should discuss if just copying over >>> logdirty_ranges (which is a pointer) is sufficient, or if >>> this code requires more synchronization). >> It's clearly not sufficient. :-) The logdirty_ranges struct is >> protected by the lock of the p2m structure that contains it; if you >> point to it from a different p2m structure, then you'll have >> inconsistent logging, and you'll have problems if one vcpu is reading >> the structure while another is modifying it. >> >> Another issue is that it doesn't look like you're propagating updates to >> this shared state either -- if someone enables or disables >> global_logdirty, or changes default_access, the altp2ms will still have >> the old value. >> >> I wonder if we should collect the various bits that need to be kept in >> sync between hostp2m/altp2ms, put them all in a 'sync' sub-struct within >> the p2m, and enforce using a function / macro to modify the values inside. > > I'm not sure how to put ept.ad in the new sync sub-struct - putting the > full ept in there feels like overkill, and just putting a new variable > to symbolize .ad feels convoluted.
I'm pretty sure the ept there is something we need to pass to hardware to implement the separate p2m tables. That's definitely not shared. :-) -George _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
