linux-kernel - Re:Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit

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: <b7d9dff.663d.192994c6b14.Coremail.andyshrk@163.com>
Date: Thu, 17 Oct 2024 15:06:15 +0800 (CST)
From: "Andy Yan" <andyshrk@....com>
To: "Robin Murphy" <robin.murphy@....com>
Cc: heiko@...ech.de, hjc@...k-chips.com, krzk+dt@...nel.org, robh@...nel.org, 
	conor+dt@...nel.org, s.hauer@...gutronix.de, 
	devicetree@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	linux-rockchip@...ts.infradead.org, derek.foreman@...labora.com, 
	minhuadotchen@...il.com, detlev.casanova@...labora.com, 
	xxm@...k-chips.com, "Andy Yan" <andy.yan@...k-chips.com>
Subject: Re:Re: [PATCH v3 02/15] drm/rockchip: Set dma mask to 64 bit



Hi Robin,

 Thanks for your comment。

At 2024-10-17 01:38:23, "Robin Murphy" <robin.murphy@....com> wrote:
>On 2024-09-20 9:20 am, Andy Yan wrote:
>> From: Andy Yan <andy.yan@...k-chips.com>
>> 
>> The vop mmu support translate physical address upper 4 GB to iova
>> below 4 GB. So set dma mask to 64 bit to indicate we support address
>>> 4GB.
>> 
>> This can avoid warnging message like this on some boards with DDR
>>> 4 GB:
>> 
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 130 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 0 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 130 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 130 (slots)
>> rockchip-drm display-subsystem: swiotlb buffer is full (sz: 266240 bytes), total 32768 (slots), used 0 (slots)
>
>There are several things wrong with this...
>
>AFAICS the VOP itself still only supports 32-bit addresses, so the VOP 
>driver should only be setting a 32-bit DMA mask. The IOMMUs support 
>either 32-bit or 40-bit addresses, and the IOMMU driver does set its DMA 
Does that mean we can only use the dev of IOMMU ? If that is true， would you
please give some inspiration on how to implement this? Or is there any other
diver i can follow。Very sorry for that  I'm not familiar with memory management and the IOMMU。


>mask appropriately. None of those numbers is 64, so that's clearly 
>suspicious already. Plus it would seem the claim of the IOMMU being able 
>to address >4GB isn't strictly true for RK3288 (which does supposedly 
>support 8GB of RAM).

We can set DMA mask per device if we can find a right way to do it。

>
>Furthermore, the "display-subsystem" doesn't even exist - it does not 
>represent any actual DMA-capable hardware, so it should not have a DMA 
>mask, and it should not be used for DMA API operations. Buffers for the 
>VOP should be DMA-mapped for the VOP device itself. At the very least
>the rockchip_gem_alloc_dma() path is clearly broken otherwise (I guess 
>this patch possibly *would* make that brokenness apparent).
>
>> Signed-off-by: Andy Yan <andy.yan@...k-chips.com>
>> Tested-by: Derek Foreman <derek.foreman@...labora.com>
>> ---
>> 
>> (no changes since v1)
>> 
>>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> index 04ef7a2c3833..8bc2ff3b04bb 100644
>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
>> @@ -445,7 +445,9 @@ static int rockchip_drm_platform_probe(struct platform_device *pdev)
>>   		return ret;
>>   	}
>>   
>> -	return 0;
>> +	ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>
>Finally as a general thing, please don't misuse 
>dma_coerce_mask_and_coherent() in platform drivers, just use normal 
>dma_set_mask_and_coherent(). The platform bus code has been initialising 
>the dev->dma_mask pointer for years now, drivers should not be messing 
>with it any more.

Got it ， thanks again。

>
>Thanks,
>Robin.
>
>> +
>> +	return ret;
>>   }
>>   
>>   static void rockchip_drm_platform_remove(struct platform_device *pdev)