Re: [PATCH v2 3/3] iommu: rockchip: Allocate tables from all available memory

[Robin Murphy <robin.murphy@arm.com>]

On 2023-06-15 23:26, Jonas Karlman wrote:
> On 2023-06-15 23:25, Robin Murphy wrote:
>> On 2023-06-15 21:10, Jonas Karlman wrote:
>>> Newer Rockchip SoCs, RK356x and RK3588, support more then 4GB of memory.
>>
>> Older ones supposedly did too, e.g. commit 79db45be2b8b ("ARM: dts:
>> rockchip: convert rk3288 device tree files to 64 bits"). Are we certain
>> that nobody actually has a system with IOMMUv1 and more than 4GB of RAM?
> 
> In IOMMU v1 bit 11:0 read back as 0 from MMU_DTE_ADDR reg, so I expect
> that the old limit for v1 is 4GB.

Right, that was my point, sorry if it was unclear - on at least RK3288, 
the *SoC* apparently supports RAM above 4GB, even if it's only the CPUs 
that can access it. GFP_DMA32 technically *is* correct and appropriate 
for IOMMUv1, so it's only safe to unconditionally remove it if we're 
sure that, on all relevant IOMMUv1 SoCs in practice, no RAM above 4GB 
exists such that ZONE_NORMAL is empty and all allocations come from 
ZONE_DMA32 by default anyway.

(in truth this only matters for 64-bit SoCs, since on 32-bit there is no 
ZONE_DMA32, but any >4GB RAM would already have to be in ZONE_HIGHMEM, 
so safely out of scope)

Thanks,
Robin.

> I will reword this to focus on IOMMU
> v1 vs v2 instead of SoCs in v3.
> 
>>
>>> However, the RK IOMMU driver is using the GFP_DMA32 flag to limit
>>> allocation of the discovery and page tables into memory below 4GB.
>>>
>>> Let's remove this limitation now that the discovery table address is
>>
>> Nit: s/discovery/directory/g again
> 
> Will fix in v3 :-)
> 
> Regards,
> Jonas
> 
>>
>> Thanks,
>> Robin.
>>
>>> correctly configured for addresses above 4GB.
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>> v2:
>>> - no change
>>>
>>>    drivers/iommu/rockchip-iommu.c | 4 ++--
>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
>>> index 62be9bf42390..46498fc382ee 100644
>>> --- a/drivers/iommu/rockchip-iommu.c
>>> +++ b/drivers/iommu/rockchip-iommu.c
>>> @@ -727,7 +727,7 @@ static u32 *rk_dte_get_page_table(struct rk_iommu_domain *rk_domain,
>>>    	if (rk_dte_is_pt_valid(dte))
>>>    		goto done;
>>>    
>>> -	page_table = (u32 *)get_zeroed_page(GFP_ATOMIC | GFP_DMA32);
>>> +	page_table = (u32 *)get_zeroed_page(GFP_ATOMIC);
>>>    	if (!page_table)
>>>    		return ERR_PTR(-ENOMEM);
>>>    
>>> @@ -1076,7 +1076,7 @@ static struct iommu_domain *rk_iommu_domain_alloc(unsigned type)
>>>    	 * Each level1 (dt) and level2 (pt) table has 1024 4-byte entries.
>>>    	 * Allocate one 4 KiB page for each table.
>>>    	 */
>>> -	rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL | GFP_DMA32);
>>> +	rk_domain->dt = (u32 *)get_zeroed_page(GFP_KERNEL);
>>>    	if (!rk_domain->dt)
>>>    		goto err_free_domain;
>>>    
>