回复: Re: [PATCH] iommu/rockchip: prevent iommus dead loop when two masters share one IOMMU

From: "xxm@rock-chips.com" <xxm@rock-chips.com>
To: robin.murphy <robin.murphy@arm.com>,  joro <joro@8bytes.org>,
	 will <will@kernel.org>,  heiko <heiko@sntech.de>
Cc: iommu <iommu@lists.linux.dev>,
	 linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	 linux-rockchip <linux-rockchip@lists.infradead.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>
Subject: 回复: Re: [PATCH] iommu/rockchip: prevent iommus dead loop when two masters share one IOMMU
Date: Mon, 23 Jun 2025 08:41:44 +0800	[thread overview]
Message-ID: <202506230841445347043@rock-chips.com> (raw)
In-Reply-To: 340d76b8-c3a6-4116-ae51-ac4e4ee6a994@arm.com

Hi Robin,

>On 2025-06-20 8:39 am, Simon Xue wrote:

>> When two masters share an IOMMU, calling ops->of_xlate during

>> the second master's driver init may overwrite iommu->domain set

>> by the first. This causes the check if (iommu->domain == domain)

>> in rk_iommu_attach_device() to fail, resulting in the same

>> iommu->node being added twice to &rk_domain->iommus, which can

>> lead to an infinite loop in subsequent &rk_domain->iommus operations.

>

>Indeed this is a property of the IOMMU instance itself so it really 

>should be initialised before registration, irrespective of client 

>devices. FWIW, if it's possible to take an unexpected 

>RK_MMU_IRQ_PAGE_FAULT immediately after requesting the IRQ (e.g. in a 

>kdump kernel after a crash with the hardware still running) then I think 

>the current code could probably end up dereferencing NULL in 

>report_iommu_fault() as well.

Thanks  for your review and clear explanation, I will add the information as you suggested.

Simon Xue

>

>Reviewed-by: Robin Murphy <robin.murphy@arm.com>

>

>And probably also:

>

>Cc: stable@vger.kernel.org

>Fixes: 25c2325575cc ("iommu/rockchip: Add missing set_platform_dma_ops 

>callback")

>

>Thanks,

>Robin.

>

>> Signed-off-by: Simon Xue <xxm@rock-chips.com>

>> ---

>>   drivers/iommu/rockchip-iommu.c | 3 ++-

>>   1 file changed, 2 insertions(+), 1 deletion(-)

>> 

>> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c

>> index 22f74ba33a0e..e6bb3c784017 100644

>> --- a/drivers/iommu/rockchip-iommu.c

>> +++ b/drivers/iommu/rockchip-iommu.c

>> @@ -1157,7 +1157,6 @@ static int rk_iommu_of_xlate(struct device *dev,

>>   		return -ENOMEM;

>>   

>>   	data->iommu = platform_get_drvdata(iommu_dev);

>> -	data->iommu->domain = &rk_identity_domain;

>>   	dev_iommu_priv_set(dev, data);

>>   

>>   	platform_device_put(iommu_dev);

>> @@ -1195,6 +1194,8 @@ static int rk_iommu_probe(struct platform_device *pdev)

>>   	if (!iommu)

>>   		return -ENOMEM;

>>   

>> +	iommu->domain = &rk_identity_domain;

>> +

>>   	platform_set_drvdata(pdev, iommu);

>>   	iommu->dev = dev;

>>   	iommu->num_mmu = 0;

>

>