linux-kernel - Re: [PATCH v3 1/1] iommu/sva: Invalidate KVA range on kernel TLB flush

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aK3FsU1Dds4OG79o@casper.infradead.org>
Date: Tue, 26 Aug 2025 15:33:21 +0100
From: Matthew Wilcox <willy@...radead.org>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Baolu Lu <baolu.lu@...ux.intel.com>,
	"Tian, Kevin" <kevin.tian@...el.com>,
	Jason Gunthorpe <jgg@...dia.com>, Joerg Roedel <joro@...tes.org>,
	Will Deacon <will@...nel.org>, Robin Murphy <robin.murphy@....com>,
	Jann Horn <jannh@...gle.com>, Vasant Hegde <vasant.hegde@....com>,
	Alistair Popple <apopple@...dia.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Uladzislau Rezki <urezki@...il.com>,
	Jean-Philippe Brucker <jean-philippe@...aro.org>,
	Andy Lutomirski <luto@...nel.org>, "Lai, Yi1" <yi1.lai@...el.com>,
	"iommu@...ts.linux.dev" <iommu@...ts.linux.dev>,
	"security@...nel.org" <security@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"stable@...r.kernel.org" <stable@...r.kernel.org>,
	vishal.moola@...il.com
Subject: Re: [PATCH v3 1/1] iommu/sva: Invalidate KVA range on kernel TLB
 flush

On Tue, Aug 26, 2025 at 07:22:06AM -0700, Dave Hansen wrote:
> On 8/25/25 19:49, Baolu Lu wrote:
> >> The three separate lists are needed because we're handling three
> >> distinct types of page deallocation. Grouping the pages this way allows
> >> the workqueue handler to free each type using the correct function.
> > 
> > Please allow me to add more details.
> 
> Right, I know why it got added this way: it was the quickest way to hack
> together a patch that fixes the IOMMU issue without refactoring anything.
> 
> I agree that you have three cases:
> 1. A full on 'struct ptdesc' that needs its destructor run
> 2. An order-0 'struct page'
> 3. A higher-order 'struct page'
> 
> Long-term, #2 and #3 probably need to get converted over to 'struct
> ptdesc'. They don't look _that_ hard to convert to me. Willy, Vishal,
> any other mm folks: do you agree?

Uhh.  I'm still quite ignorant about iommu page tables.  Let me make
some general observations that may be helpful.

We are attempting to shrink struct page down to 8 bytes.  That's going
to proceed in stages over the next two to three years.  Depending how
much information you need to keep around, you may be able to keep using
struct page for a while, but eventually (unless you only need a few bits
of information), you're going to need a memdesc for your allocations.

One memdesc type already assigned is for page tables.  Maybe iommu page
tables are the same / similar enough to a CPU page table that we keep
the same data structure.  Maybe they'll need their own data structure.
I lack the knowledge to make that decision.

For more on memdescs, please see here:
https://kernelnewbies.org/MatthewWilcox/Memdescs

> Short-term, I'd just consolidate your issue down to a single list.
> 
> #1: For 'struct ptdesc', modify pte_free_kernel() to pass information in
>     to pagetable_dtor_free() to tell it to use the deferred page table
>     free list. Do this with a bit in the ptdesc or a new argument to
>     pagetable_dtor_free().

We should be able to reuse one of the flags in __page_flags or there
are 24 unused bits in __page_type.

> #2. Just append these to the deferred page table free list. Easy.
> #3. The biggest hacky way to do this is to just treat the higher-order
>     non-compound page and put the pages on the deferred page table
>     free list one at a time. The other way to do it is to track down how
>     this thing got allocated in the first place and make sure it's got
>     __GFP_COMP metadata. If so, you can just use __free_pages() for
>     everything.

Non-compound allocations are bad news.  Is there a good reason these
can't be made into compound allocations?

> Yeah, it'll take a couple patches up front to refactor some things. But
> that refactoring will make things more consistent instead of adding
> adding complexity to deal with the inconsistency.