On 24.08.2020 15:00, Andrew Cooper wrote: > On 24/08/2020 13:34, Jan Beulich wrote: >> While in most cases code ahead of the invocation of set_gpfn_from_mfn() >> deals with shared pages, at least in set_typed_p2m_entry() I can't spot >> such handling (it's entirely possible there's code missing there). Let's >> try to play safe and add an extra check. >> >> Signed-off-by: Jan Beulich <jbeul...@suse.com> > > I agree that this is an improvement. > > Therefore, tentatively Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
Thanks, but - what do I do with a tentative ack? Take it as a "normal" one, or not take it at all? > However, I don't think it is legitimate for set_gpfn_from_mfn() to be > overriding anything. > > IMO, we should be asserting something like (pfn == SHARED_M2P_ENTRY) == > (d == dom_cow). > > Any code not passing this properly is almost certainly broken anyway, > and fixing up behind its back like this doesn't feel like a clever idea > (in debug builds at least). As said on v1: I agree in principle, but I'd like such a change to be made by the mem-sharing maintainer(s), so we wouldn't notice fallout only several months or years later. Tamas - would you be up for this? Jan