Re: [PATCH 5/5] iommu/dart: Assert !locked when configuring

From: Robin Murphy
Date: Tue Feb 11 2025 - 16:13:25 EST


On 2025-02-11 7:21 pm, Janne Grunau wrote:
On Tue, Feb 11, 2025 at 06:41:00PM +0000, Robin Murphy wrote:
On 2025-02-10 7:39 pm, Alyssa Rosenzweig wrote:
Configuration is only possible and needed for non-locked DARTs and will
fail for locked DARTs. We cannot try -- assert that we do not.

Except now we absolutely will - if a locked DART and its client device
are advertised to Linux, instead of gracefully refusing to touch it,
we'll now attach the client to a DMA domain, firing a barrage of
multiple WARNs in the process, and give it DMA ops which still cannot
work. I'm not really convinced this series on its own leaves us in a
better position than we're already in now... :/

How hideous is the rest of what's required to actually make this usable?

The TTBR can not be changed but the preset first level table can
modified at will. The driver keeps a shadow first label table and syncs
that to the preset 1st level table on flush_tbl().
It gets more complicated by the fact that the iommu for the display
coprocessor is locked and mappings for its firmware and boot framebuffer
are preinstalled and have to be maintained or restored on
initialization.
This is handled via reserved memory with translation.

Downstream change to handle this is in
https://github.com/AsahiLinux/linux/commit/d90cc3590ea460e1c574b4b7c47fdafb2794af6a
not including the change to parse / handle reserved memory with
translation in iommu core.

Oh, if we handwave away the reserved region stuff for now, it doesn't seem *too* terrible, so definitely worth trying to land the bones of it along with this prep work, I reckon. From a quick look I think it might possibly be even cleaner as an io-pgtable quirk, to essentially skip allocating/freeing L1 and have some mechanism to fill in data->pgd with the remap afterwards (possible super cheeky version - also prepopulate cfg->apple_dart_cfg.ttbr and have alloc/free handle the remapping/unmapping themselves...). I'm not 100% sure off-hand, but since you avoid the DMA API and don't seem to have any other dependency on data->pgd having a linear map VA (other than the virt_to_phys() in the normal alloc path which you'd skip anyway), it feels like it ought to work out.

I guess to support multiple domains you do still end up having to save/restore the L1 contents at the driver level when attaching, so some kind of shadow table notion isn't entirely unavoidable... oh well, it's a thought, at least.

Thanks,
Robin.