* meaning and use of IOMMU_FLUSHF_added
@ 2021-08-18 10:51 Jan Beulich
2021-08-18 12:09 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2021-08-18 10:51 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: meaning and use of IOMMU_FLUSHF_added
2021-08-18 10:51 meaning and use of IOMMU_FLUSHF_added Jan Beulich
@ 2021-08-18 12:09 ` Jan Beulich
2021-08-18 14:15 ` Paul Durrant
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2021-08-18 12:09 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: meaning and use of IOMMU_FLUSHF_added
2021-08-18 12:09 ` Jan Beulich
@ 2021-08-18 14:15 ` Paul Durrant
2021-08-19 11:10 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Paul Durrant @ 2021-08-18 14:15 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: meaning and use of IOMMU_FLUSHF_added
2021-08-18 14:15 ` Paul Durrant
@ 2021-08-19 11:10 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2021-08-19 11:10 UTC (permalink / raw)
To: paul; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-19 11:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18 10:51 meaning and use of IOMMU_FLUSHF_added Jan Beulich
2021-08-18 12:09 ` Jan Beulich
2021-08-18 14:15 ` Paul Durrant
2021-08-19 11:10 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.