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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.12.2021 09:30, 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.

"interleaved" is perhaps the wrong word. But can you easily exclude e.g.
a put_page() in the middle of some other operation? That could in turn
invoke one of the legacy map/unmap functions (see cleanup_page_mappings()
for an example), where the flushing happens immediately after the
map/unmap.

> 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.

Only if the flush is a full-address-space one.

Jan