On 2025-03-18 10:46 am, Mostafa Saleh wrote:
On Mon, Mar 17, 2025 at 10:35:00AM -0300, Jason Gunthorpe wrote:
On Wed, Mar 12, 2025 at 12:59:00PM +0000, Mostafa Saleh wrote:
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -263,14 +263,13 @@ static void *__arm_lpae_alloc_pages(size_t size, gfp_t gfp,
void *cookie)
{
struct device *dev = cfg->iommu_dev;
- int order = get_order(size);
dma_addr_t dma;
void *pages;
if (cfg->alloc)
pages = cfg->alloc(cookie, size, gfp);
else
- pages = iommu_alloc_pages_node(dev_to_node(dev), gfp, order);
+ pages = iommu_alloc_pages_node_sz(dev_to_node(dev), gfp, size);
Although, the current implementation of iommu_alloc_pages_node_sz() would round
the size to order, but this is not correct according to the API definition
"The returned allocation is round_up_pow_two(size) big, and is physically aligned
to its size."
Yes.. The current implementation is limited to full PAGE_SIZE only,
the documentation imagines a future where it is not. Drivers should
ideally not assume the PAGE_SIZE limit during this conversion.
I'd say we can align the size or use min with 64 bytes before calling the
function would be enough (or change the API to state that allocations
are rounded to order)
OK, like this:
if (cfg->alloc) {
pages = cfg->alloc(cookie, size, gfp);
} else {
/*
* For very small starting-level translation tables the HW
* requires a minimum alignment of at least 64 to cover all
* cases.
*/
pages = iommu_alloc_pages_node_sz(dev_to_node(dev), gfp,
max(size, 64));
}
Yes, that looks good.
Although for completeness it really wants to cover both paths, so an
unconditional "size = max(size, 64);" further up would be even better.
Thanks,
Robin.
