meaning and use of IOMMU_FLUSHF_added

meaning and use of IOMMU_FLUSHF_added

[Jan Beulich]

Paul,

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. Luckily no flush accumulation has been committed so
far (besides some during Dom0 construction), meaning this has only
been a latent issue until now that I try to get large page
mappings to work. (I think I have page table construction working,
but after the removal of some debug output I'm now facing faults
on non-present entries which I believe are actually present in the
page tables, albeit I yet have to check that.)

Question therefore is: Do we want to re-purpose the flag (my
preference), or do I need to add a 3rd one (in which case I'm
afraid I can't think of a good name, with "added" already in use)?

Jan

Re: meaning and use of IOMMU_FLUSHF_added

[Jan Beulich]

On 18.08.2021 12:51, Jan Beulich wrote:
> Paul,
> 
> 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 ...

Jan

Re: meaning and use of IOMMU_FLUSHF_added

[Paul Durrant]

On 18/08/2021 13:09, Jan Beulich wrote:
> On 18.08.2021 12:51, Jan Beulich wrote:
>> Paul,
>>
>> 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.

   Paul

Re: meaning and use of IOMMU_FLUSHF_added

[Jan Beulich]

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