RDMA/cxgb4: Support on-chip SQs

Hello Steve Wise,

The patch c6d7b26791a2: "RDMA/cxgb4: Support on-chip SQs" from Sep 13, 2010, leads to the following warning:
"drivers/infiniband/hw/cxgb4/qp.c:98 alloc_host_sq()

	error: 'sq->queue' came from dma_alloc_coherent() so we can't
	do virt_to_phys()"

drivers/infiniband/hw/cxgb4/qp.c

     92  static int alloc_host_sq(struct c4iw_rdev *rdev, struct t4_sq *sq)
     93  {
     94          sq->queue = dma_alloc_coherent(&(rdev->lldi.pdev->dev), sq->memsize,
     95                                         &(sq->dma_addr), GFP_KERNEL);
     96          if (!sq->queue)
     97                  return -ENOMEM;
     98          sq->phys_addr = virt_to_phys(sq->queue);
     99          pci_unmap_addr_set(sq, mapping, sq->dma_addr);
    100          return 0;
    101  }

This is a new Smatch check. I don't know the rules on virt_to_phys yet but here is the relevant comment.

/**
* virt_to_phys - map virtual addresses to physical
* @address: address to remap
*

  *      The returned physical address is the physical (CPU) mapping for
  *      the memory address given. It is only valid to use this function on
  *      addresses directly mapped or allocated via kmalloc.
  *
  *      This function does not give bus mappings for DMA transfers. In
  *      almost all conceivable cases a device driver should not be using
  *      this function

*/

regards,
dan carpenter

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

Quoted 5 lines · Steve Wise

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

DMA-API.txt says:

This routine allocates a region of <size> bytes of consistent memory. It also returns a <dma_handle> which may be cast to an unsigned integer the same width as the bus and used as the physical address base of the region.

So instead of virt_to_phys you should use dma_addr??

I think the note about 'directly mapped' refers to things the arch setups during early boot, not dma_alloc stuff.

Jason

Quoted 13 lines · Jason Gunthorpe

Quoted 5 lines · Steve Wise

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

DMA-API.txt says:

This routine allocates a region of <size> bytes of consistent memory. It also returns a <dma_handle> which may be cast to an unsigned integer the same width as the bus and used as the physical address base of the region.

Quoted 1 line

So instead of virt_to_phys you should use dma_addr??

I don't think so. The result of virt_to_phys() is used in remap_pfn_range() to map this memory into user space... The dma_addr is give to HW for DMA operations.

So instead of virt_to_phys you should use dma_addr??

Quoted 1 line · Jason Gunthorpe

So instead of virt_to_phys you should use dma_addr??

No, you clearly can't use the bus address. That has no relationship to the CPU view, even in cases as common as VT-d on x86.

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

Quoted 6 lines · Steve Wise

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

Here is another relevant comment I should have included.

commit ae6a5d37725853325a2b3460165fbc5613ce2916

Author: Russell King <rmk+kernel-lFZ/[email protected]>
Date:   Tue Nov 20 12:17:51 2012 +0000

ASoC: kirkwood-dma: fix use of virt_to_phys()

This is part of a patch found in Rabeeh Khoury's git tree for the cubox.

You can not use virt_to_phys() on the address returned from dma_alloc_coherent(); it may not be part of the kernel direct-mapped memory. Fix this to use the DMA address instead.

Signed-off-by Russell King <rmk+kernel-lFZ/[email protected]>

Signed-off-by Mark Brown <broonie-yzvPICuk2AATkU/[email protected]>

I don't know the details though, and I certainly don't know how someone would fix this. ;)

Quoted 23 lines · Dan Carpenter

Quoted 6 lines · Steve Wise

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

Here is another relevant comment I should have included.

commit ae6a5d37725853325a2b3460165fbc5613ce2916

Author: Russell King <rmk+kernel-lFZ/[email protected]>
Date:   Tue Nov 20 12:17:51 2012 +0000

ASoC: kirkwood-dma: fix use of virt_to_phys()

This is part of a patch found in Rabeeh Khoury's git tree for the cubox.

You can not use virt_to_phys() on the address returned from dma_alloc_coherent(); it may not be part of the kernel
direct-mapped
memory. Fix this to use the DMA address instead.

Quoted 6 lines

Signed-off-by Russell King <rmk+kernel-lFZ/[email protected]>

Signed-off-by Mark Brown <broonie-yzvPICuk2AATkU/[email protected]>

I don't know the details though, and I certainly don't know how someone would fix this. ;)

How do I map the dma address into a physical address suitable for passing to remap_pfn_range()?

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

What do you think Roland?

Quoted 7 lines · Steve Wise

I wonder, then, what the correct service is to get the cpu physical address from a kernel virtual address returned from dma_alloc_coherent()? I think this is correct as-is, since I think dma_alloc_coherent() falls under the "directly mapped" addresses in the virt_to_phys() prototype comment.

What do you think Roland?

In the end, we decided this was correct as-is?

I'm about to complain to the caif network people about the dma_alloc_coherent() -> virt_to_phys() thing, but the truth is I don't understand this stuff very well and I'm not sure when it's ok or not.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

In the end, we decided this was correct as-is?

I'm about to complain to the caif network people about the dma_alloc_coherent() -> virt_to_phys() thing, but the truth is I don't understand this stuff very well and I'm not sure when it's ok or not.