Re: [PATCH v2 08/18] IOMMU/x86: support freeing of pagetables

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Dec 03, 2021 at 09:30:00AM +0100, Roger Pau Monné wrote:
> On Thu, Dec 02, 2021 at 05:10:38PM +0100, Jan Beulich wrote:
> > On 02.12.2021 17:03, Roger Pau Monné wrote:
> > > On Fri, Sep 24, 2021 at 11:48:21AM +0200, Jan Beulich wrote:
> > >> For vendor specific code to support superpages we need to be able to
> > >> deal with a superpage mapping replacing an intermediate page table (or
> > >> hierarchy thereof). Consequently an iommu_alloc_pgtable() counterpart is
> > >> needed to free individual page tables while a domain is still alive.
> > >> Since the freeing needs to be deferred until after a suitable IOTLB
> > >> flush was performed, released page tables get queued for processing by a
> > >> tasklet.
> > >>
> > >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > >> ---
> > >> I was considering whether to use a softirq-taklet instead. This would
> > >> have the benefit of avoiding extra scheduling operations, but come with
> > >> the risk of the freeing happening prematurely because of a
> > >> process_pending_softirqs() somewhere.
> > > 
> > > Another approach that comes to mind (maybe you already thought of it
> > > and discarded) would be to perform the freeing after the flush in
> > > iommu_iotlb_flush{_all} while keeping the per pPCU lists.
> > 
> > This was my initial plan, but I couldn't convince myself that the first
> > flush to happen would be _the_ one associated with the to-be-freed
> > page tables. ISTR (vaguely though) actually having found an example to
> > the contrary.
> 
> I see. If we keep the list per pCPU I'm not sure we could have an
> IOMMU  flush not related to the to-be-freed pages, as we cannot have
> interleaved IOMMU operations on the same pCPU.
> 
> Also, if we strictly add the pages to the freeing list once unhooked
> from the IOMMU page tables it should be safe to flush and then free
> them, as they would be no references remaining anymore.

Replying to my last paragraph: there are different types of flushes,
and they have different scopes, so just adding the pages to be freed
to a random list and expecting any flush to remove them from the IOMMU
cache is not correct.

I still think the first paragraph is accurate, as we shouldn't have
interleaving IOMMU operations on the same pCPU, so a flush on a pCPU
should always clear the entries that have been freed as a result of
the ongoing operation on that pCPU, and those operations should be
sequential.

Thanks, Roger.