[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH RFC v2 3/3] x86/altp2m: p2m_altp2m_propagate_change() should honor present page order
On Wed, Jan 5, 2022 at 3:59 AM Jan Beulich <jbeulich@xxxxxxxx> wrote: > > On 04.01.2022 18:48, Tamas K Lengyel wrote: > >> I may be entirely wrong and hence that part of the change may also be > >> wrong, but I'm having trouble seeing why the original > >> "!mfn_eq(m, INVALID_MFN)" wasn't "mfn_eq(m, INVALID_MFN)". Isn't the > >> goal there to pre-fill entries that were previously invalid, instead of > >> undoing prior intentional divergence from the host P2M? (I have > >> intentionally not reflected this aspect in the description yet; I can't > >> really write a description of this without understanding what's going on > >> in case the original code was correct.) > > > > This function only gets called from p2m-ept when the hostp2m gets an > > update. In that case this check goes through all altp2m's to see if > > any of them has an entry for what just got changed in the host, and > > overwrites the altp2m with that from the host. If there is no entry in > > the altp2m it doesn't pre-populate. That should only happen if the > > altp2m actually needs it and runs into a pagefault. So it is correct > > as-is, albeit being a subtle (and undocumented) behavior of the > > hostp2m and its effect on the altp2m's. But that's why we never > > actually make any changes on the hostp2m, we always create an altp2m > > and apply changes (mem_access/remapping) there. > > Thanks for the explanation. Effectively this means that the call to > get_gfn_type_access() can simply be get_gfn_query(). For the patch > this means that I shouldn't check its return value and also continue > to pass the new order rather than the minimum of the two (as was the > case before), as all we're after is the locking of the GFN. It would > be nice if you could confirm this before I submit a non-RFC v3. I'm a little lost here. > > What I still don't understand is why the function blindly replaces > any possible entry in the altp2m, i.e. any possible override > mapping, not even taking into account the original p2m_access_t. I think the idea was that any changes to the hostp2m will just get reflected in the altp2m's as a short-cut. If you have many custom settings in different altp2ms, simply setting the entry in the hostp2m will ensure all altp2m's get synced to that same setting instead of having to do a hypercall for each altp2m. I don't see much use of it otherwise and if we wanted to switch it to leave the altp2m entries as-is I wouldn't object. Tamas
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |