Re: [PATCH 1/2] iommu: Take group lock before attaching device in iommu_deferred_attach()

[Vasant Hegde <vasant.hegde@amd.com>]

Jason,


On 6/24/2024 8:55 PM, Jason Gunthorpe wrote:
> On Mon, Jun 24, 2024 at 07:50:46PM +0530, Vasant Hegde wrote:
> 
>> It may be good thing to add mutex here.. But if any of the caller of these
>> deferred functions hold spin lock again we will hit the similar issue that I hit
>> with AMD driver.
> 
> My point was that Intel doesn't support non-sleeping attach, it uses
> GFP_KERNEL, that support seems to be a uniquely AMD thing of the major
> drivers.

As I understand we don't have any hard requirement. We can sleep in attach
device path. I do have a patch to replace dev_data->spinlock to mutex in AMD
driver. I need to do some more testing before sending it out.

> 
> So, I don't see an issue with formalizing the lowest common
> denominator in the core code - the context must be sleepable.
> 
> Presumably things work because the kdump kernels only run drivers that
> reliably trigger first dma from sleepable contexts. Unless there is
> something AMD only that relies on this special behavior??

AFAIK AMD driver doesn't have any special behaviour in kdump path. When we jump
to crashing kernel (kexec) we don't disable IOMMU and try to init IOMMU with
existing info.

Having said that I have not done exhaustive kdump test with mutex lock. I will
run some more tests and get back to you.

-Vasant