On 18.08.2021 16:15, Paul Durrant wrote: > On 18/08/2021 13:09, Jan Beulich wrote: >> On 18.08.2021 12:51, Jan Beulich wrote: >>> back at the time I did already question your intended meaning of >>> this flag. I notice that there's presently no consumer of it being >>> set (apart from yielding non-zero flush_flags). I'm afraid this >>> model makes accumulation of flush flags not work properly: With >>> both flags set and more than a single page altered, it is >>> impossible to tell apart whether two present PTEs were altered, or >>> a non-present and a present one. >>> >>> VT-d's flushing needs to know the distinction; it may in fact be >>> necessary to issue two flushes (or a single "heavier" one) when >>> both non-present and present entries got transitioned to present >>> in one go. >> >> No two (or "heavier") flush looks to be needed upon further reading. >> I did derive this from our setting of "did" to zero in that case, >> but that looks to be wrong in the first place - it's correct only >> for context cache entry flushes. I'll make a(nother) patch ... > > Yes, the intention of the flag was simply to allow a 'lighter' flush in > the case there are no modified entries as part of the accumulation. If > it is impossible to tell the difference then I guess it's not useful.
Actually things can remain as they are, I think. The problem really was the flushing bug (patch sent earlier today): If it was necessary to flush previously present entries with their actual did but not- present ones with did 0, the flushing logic would have had a need to know. With that bug fixed (and hence with flushing only needing a yes/no indicator), all is fine as is (in this regard). Jan
