Re: [PATCH] IOMMU: avoid double flushing in shared page table case

[Julien Grall <julien@xxxxxxx>]

Hi Jan,

On 20/10/2020 16:05, Jan Beulich wrote:
> On 20.10.2020 17:00, Julien Grall wrote:
> > On 20/10/2020 14:52, Jan Beulich wrote:
> > > While the flush coalescing optimization has been helping the non-shared
> > > case, it has actually lead to double flushes in the shared case (which
> > > ought to be the more common one nowadays at least): Once from
> > > *_set_entry() and a second time up the call tree from wherever the
> > > overriding flag gets played with. In alignment with XSA-346 suppress
> > > flushing in this case.
> > >
> > > Similarly avoid excessive setting of IOMMU_FLUSHF_added on the batched
> > > flushes: "idx" hasn't been added a new mapping for.
> > >
> > > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > ---
> > > TBD: The Arm part really is just for completeness (and hence could also
> > >        be dropped) - the affected mapping spaces aren't currently
> > >        supported there.
> > As I may I have pointed out in the past, there are many ways to screw
> > things up when using iommu_dont_flush_iotlb.
> >
> > So I would rather not introduce any usage on Arm until we see a use-case.
> "Usage" to me would mean a path actually setting the flag.
> What I'm adding here, basically as a precautionary measure,
> is a check of the flag.
The code would always be safe without checking the safe (albeit doing 
pointless flush). I wouldn't say the same if we check the flag because 
it is not correct to set it everywhere.
> Does your use of "usage" imply you
> don't want that either? Just to be sure; I'm okay to drop
> the Arm part.
That's correct, I don't want this check to be present until there is a 
user. Only then, we can assess whether this is the right approach for Arm.
Cheers,

--
Julien Grall