Messages in this thread |  | | From | Arnd Bergmann <> | | Date | Fri, 1 Apr 2022 08:40:00 +0200 | | Subject | Re: [PATCH] habanalabs: fix build warning |
| |
On Fri, Apr 1, 2022 at 6:16 AM Max Filippov <jcmvbkbc@gmail.com> wrote: > > allmodconfig build fails on ARCH=xtensa with the following message: > > drivers/misc/habanalabs/common/memory.c:153:49: error: cast from pointer > to integer of different size [-Werror=pointer-to-int-cast] > (u64) gen_pool_dma_alloc_align(vm->dram_pg_pool, > > Fix it by adding intermediate conversion to uintptr_t as in other places > in that driver. > > Fixes: e8458e20e0a3 ("habanalabs: make sure device mem alloc is page aligned") > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > drivers/misc/habanalabs/common/memory.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/misc/habanalabs/common/memory.c b/drivers/misc/habanalabs/common/memory.c > index e008d82e4ba3..f0d373171d2a 100644 > --- a/drivers/misc/habanalabs/common/memory.c > +++ b/drivers/misc/habanalabs/common/memory.c > @@ -150,9 +150,9 @@ static int alloc_device_memory(struct hl_ctx *ctx, struct hl_mem_in *args, > for (i = 0 ; i < num_pgs ; i++) { > if (is_power_of_2(page_size)) > phys_pg_pack->pages[i] = > - (u64) gen_pool_dma_alloc_align(vm->dram_pg_pool, > - page_size, NULL, > - page_size); > + (u64)(uintptr_t)gen_pool_dma_alloc_align(vm->dram_pg_pool, > + page_size, NULL, > + page_size); > else > phys_pg_pack->pages[i] = (u64) gen_pool_alloc(vm->dram_pg_pool, > page_size);
This addresses the warning, but I suspect there is still a problem in the code: The description of that member lists it as '@pages: the physical page array', but it is actually a kernel virtual address that gets passed to it. Since this is a 'u64' member, it is hard to tell what type it actually is.
gen_pool_dma_alloc_align() returns both a virtual address and a dma (bus) address. The dma address is ignored here, which makes me wonder why this interface is used in the first place.
I can see four possible things that may be going on here:
- if the pages[] array is meant to be a kernel virtual address, it should be changed from a 'u64' to a normal pointer, with the cast removed.
- if the pages[] array is meant to be a physical address, as documented, it should be assigned using virt_to_phys() on the pointer, with a warning that this must not be used a as a dma address (which can easily get confused with a phys address as the binary representation is often the same in the absence of an iommu). In this case, it should also be changed to a phys_addr_t.
- if the pages[] array is meant to be a dma address, it should be changed to a dma_addr_t, and passed as the third argument to gen_pool_dma_alloc_align() in order to return the correct address.
- if there is a 'u64' member that is used for two (or all three) of the above depending on context, it should be replaced with either multiple struct members or a union.
Looking at other uses of the pages[] array, I see a dma_addr_t assigned to it in init_phys_pg_pack_from_userptr(), but map_phys_pg_pack() and alloc_sgt_from_device_pages appear to treat it as a cpu-physical phys_addr_t rather than a device address again.
Arnd
|  |