Re: [RFC PATCH v8 08/14] swiotlb: Align shared IO TLB pools to the shared granule size

[Aneesh Kumar K.V <aneesh.kumar@kernel.org>]

sashiko-bot@kernel.org writes:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] mm: infinite loop in swiotlb_init_late() when remap fails
> --
>
> commit 9e7f1b72520615870ee21508117e4c5c7e0eb684
> Author: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
>
> swiotlb: Align shared IO TLB pools to the shared granule size
>     
> This commit aligns and sizes the early memblock pool to the common CoCo
> shared granule, leaving the slot count unchanged. It replaces the default
> and dynamic pool set_memory calls with helpers to keep alignment validation
> and architecture dispatch centralized. Pages are intentionally leaked if
> their private state cannot be restored safely.
>
>> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
>> index ded7016a46a71..394db93b7eb96 100644
>> --- a/kernel/dma/swiotlb.c
>> +++ b/kernel/dma/swiotlb.c
>
> [ ... ]
>
>> @@ -578,7 +578,7 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
>>  		swiotlb_adjust_nareas(num_possible_cpus());
>>  
>>  retry:
>> -	order = get_order(nslabs << IO_TLB_SHIFT);
>> +	order = get_order(ALIGN(nslabs << IO_TLB_SHIFT, cc_shared_granule_size()));
>>  	nslabs = SLABS_PER_PAGE << order;
>
> [Severity: Medium]
> If remap() fails later in this function, nslabs is halved and the code jumps
> back to the retry label to try a smaller allocation:
>
> kernel/dma/swiotlb.c:swiotlb_init_late() {
>     ...
> 	if (remap)
> 		rc = remap(vstart, nslabs);
> 	if (rc) {
> 		free_pages((unsigned long)vstart, order);
>
> 		nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
> 		if (nslabs < IO_TLB_MIN_SLABS)
> 			return rc;
> 		retried = true;
> 		goto retry;
> 	}
>     ...
> }
>
> Does the new ALIGN macro at the retry label risk inflating the halved size
> back to its original value if cc_shared_granule_size() is >= 2MB?
>
> If so, this would restore nslabs to its original failing value, overriding
> the shrinking mechanism and creating an infinite loop.
>

We don't expect cc_shared_granule_size() > 2MB. But i can also add a
check. 

modified    kernel/dma/swiotlb.c
@@ -719,8 +719,13 @@ int swiotlb_init_late(size_t size, gfp_t gfp_mask,
 		free_pages((unsigned long)vstart, order);
 
 		nslabs = ALIGN(nslabs >> 1, IO_TLB_SEGSIZE);
-		if (nslabs < IO_TLB_MIN_SLABS)
-			return rc;
+		if (nslabs <= IO_TLB_MIN_SLABS)
+			return rc;
+
+		/* Avoid infinite loop */
+		if (WARN_ON(nslabs < (cc_shared_granule_size() >> IO_TLB_SHIFT)))
+			return rc;
+
 		retried = true;
 		goto retry;
 	}