Re: [PATCH rc v2] iommu/arm-smmu-v3: Do not use GFP_KERNEL under as spinlock

From: Will Deacon <will@kernel.org>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: iommu@lists.linux.dev, Joerg Roedel <joro@8bytes.org>,
	linux-arm-kernel@lists.infradead.org,
	Robin Murphy <robin.murphy@arm.com>,
	Dan Carpenter <dan.carpenter@linaro.org>,
	Michael Shavit <mshavit@google.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	patches@lists.linux.dev
Subject: Re: [PATCH rc v2] iommu/arm-smmu-v3: Do not use GFP_KERNEL under as spinlock
Date: Fri, 16 Feb 2024 12:05:12 +0000	[thread overview]
Message-ID: <20240216120512.GA1841@willie-the-truck> (raw)
In-Reply-To: <0-v2-7988c2682634+261-smmu_cd_atomic_jgg@nvidia.com>

On Thu, Feb 15, 2024 at 10:56:57AM -0400, Jason Gunthorpe wrote:
> If the SMMU is configured to use a two level CD table then
> arm_smmu_write_ctx_desc() allocates a CD table leaf internally using
> GFP_KERNEL. Due to recent changes this is being done under a spinlock to
> iterate over the device list - thus it will trigger a sleeping while
> atomic warning:
> 
>   arm_smmu_sva_set_dev_pasid()
>     mutex_lock(&sva_lock);
>     __arm_smmu_sva_bind()
>      arm_smmu_mmu_notifier_get()
>       spin_lock_irqsave()
>       arm_smmu_write_ctx_desc()
> 	arm_smmu_get_cd_ptr()
>          arm_smmu_alloc_cd_leaf_table()
> 	  dmam_alloc_coherent(GFP_KERNEL)
> 
> This is a 64K high order allocation and really should not be done
> atomically.
> 
> At the moment the rework of the SVA to follow the new API is half
> finished. Recently the CD table memory was moved from the domain to the
> master, however we have the confusing situation where the SVA code is
> wrongly using the RID domains device's list to track which CD tables the
> SVA is installed in.
> 
> Remove the logic to replicate the CD across all the domain's masters
> during attach. We know which master and which CD table the PASID should be
> installed in.
> 
> At the moment SVA is only invoked when dma-iommu.c is in control of the
> RID translation, which means we have a single iommu_domain shared across
> the entire group and that iommu_domain is not shared outside the group.
> 
> For PCI cases the core code also insists on singleton groups so there is
> only ever one entry in the smmu_domain->domains list that is equal to the
> master being passed in to arm_smmu_sva_set_dev_pasid().
> 
> Only non-PCI cases may have multi-device groups. However, the core code it
> self will replicate the calls to arm_smmu_sva_set_dev_pasid() across the
> entire group so we will still correctly install the CD into each group
> members master.

Are you sure about this paragraph? arm_smmu_mmu_notifier_get() will return
early if it finds an existing notifier in the 'mmu_notifiers' list for the
domain, so I don't think we'll actually get as far as installing the CD,
will we?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel