Fix virtual and physical address types
Subsystems (11)
cover Fix virtual and physical address types
This series fixes compiler warnings on some architectures about implicit conversions and narrowing conversions between pointer and integer types.
Please apply these to the appropriate trees.
Ben.
Ben Hutchings (8):
IB/cxgb4: Fix formatting of physical address
farsync: Fix confusion about DMA address and buffer offset types
drm: Do not include page offset in argument to virt_to_page()
drm: Pass pointers to virt_to_page()
[SCSI] tgt: Pass pointers to virt_to_page(), not integers
uio: Pass pointers to virt_to_page(), not integers
rds: Pass pointers to virt_to_page(), not integers
pmcraid: Pass pointers to access_ok(), not integers
drivers/gpu/drm/drm_pci.c | 4 ++-- drivers/gpu/drm/drm_vm.c | 2 +- drivers/infiniband/hw/cxgb4/device.c | 4 ++-- drivers/net/wan/farsync.c | 19 +++++++------------ drivers/scsi/pmcraid.c | 3 ++- drivers/scsi/scsi_tgt_if.c | 2 +- drivers/uio/uio.c | 6 ++++-- net/rds/message.c | 2 +- 8 files changed, 20 insertions(+), 22 deletions(-)
Signature · 1 line
1.7.7.3
PATCH 1/8 IB/cxgb4: Fix formatting of physical address
Physical addresses may be wider than virtual addresses (e.g. on i386 with PAE) and must not be formatted with %p.
---
The resource could alternately be formatted with %Pr.
Ben.
drivers/infiniband/hw/cxgb4/device.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/infiniband/hw/cxgb4/device.c +2 -2
@@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev) rdev->lldi.vr->qp.size, rdev->lldi.vr->cq.start, rdev->lldi.vr->cq.size); - PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu " + PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu " "qpmask 0x%x cqshift %lu cqmask 0x%x\n", (unsigned)pci_resource_len(rdev->lldi.pdev, 2), - (void *)pci_resource_start(rdev->lldi.pdev, 2), + (u64)pci_resource_start(rdev->lldi.pdev, 2), rdev->lldi.db_reg, rdev->lldi.gts_reg, rdev->qpshift, rdev->qpmask,
Signature · 8 lines
1.7.7.3 -- 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
Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address
Quoted 2 lines · Ben Hutchings
Physical addresses may be wider than virtual addresses (e.g. on i386 with PAE) and must not be formatted with %p.
[]
drivers/infiniband/hw/cxgb4/device.c
Quoted 2 lines
The resource could alternately be formatted with %Pr.
drivers/infiniband/hw/cxgb4/device.c +0
-0
[]
drivers/infiniband/hw/cxgb4/device.c static int c4iw_rdev_open(struct c4iw_rdev *rdev) · line 324
Quoted 10 lines
drivers/infiniband/hw/cxgb4/device.c +2
-2@@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev) rdev->lldi.vr->qp.size, rdev->lldi.vr->cq.start, rdev->lldi.vr->cq.size); - PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu " + PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu " "qpmask 0x%x cqshift %lu cqmask 0x%x\n", (unsigned)pci_resource_len(rdev->lldi.pdev, 2), - (void *)pci_resource_start(rdev->lldi.pdev, 2), + (u64)pci_resource_start(rdev->lldi.pdev, 2),
Probably should be unsigned long long
--
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
Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address
Quoted 7 lines · Joe Perches
Quoted 2 lines · Ben Hutchings
Physical addresses may be wider than virtual addresses (e.g. on i386 with PAE) and must not be formatted with %p.
[]
drivers/infiniband/hw/cxgb4/device.c
Quoted 2 lines
The resource could alternately be formatted with %Pr.
drivers/infiniband/hw/cxgb4/device.c +0
-0[]
Quoted 12 lines
drivers/infiniband/hw/cxgb4/device.c static int c4iw_rdev_open(struct c4iw_rdev *rdev) · line 324
Quoted 10 lines
drivers/infiniband/hw/cxgb4/device.c +2
-2@@ -318,10 +318,10 @@ static int c4iw_rdev_open(struct c4iw_rdev *rdev) rdev->lldi.vr->qp.size, rdev->lldi.vr->cq.start, rdev->lldi.vr->cq.size); - PDBG("udb len 0x%x udb base %p db_reg %p gts_reg %p qpshift %lu " + PDBG("udb len 0x%x udb base %llx db_reg %p gts_reg %p qpshift %lu " "qpmask 0x%x cqshift %lu cqmask 0x%x\n", (unsigned)pci_resource_len(rdev->lldi.pdev, 2), - (void *)pci_resource_start(rdev->lldi.pdev, 2), + (u64)pci_resource_start(rdev->lldi.pdev, 2),Probably should be unsigned long long
unsigned long long is so long long long to type. And u64 is now defined as unsigned long long on all architectures, partly because of that.
Ben.
Signature · 2 lines
Ben Hutchings Humans are not rational beings; they are rationalising beings.
Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address
In reply to Joe PerchesQuoted 7 lines · Joe Perches
Quoted 2 lines · Ben Hutchings
Physical addresses may be wider than virtual addresses (e.g. on i386 with PAE) and must not be formatted with %p.
[]
Quoted 1 line
The resource could alternately be formatted with %Pr.
Probably should be unsigned long long
I don't have a dog in this fight, but the %Pr sounds better to me.
Re: [PATCH 1/8] IB/cxgb4: Fix formatting of physical address
In reply to Ben Hutchings--
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
PATCH 2/8 farsync: Fix confusion about DMA address and buffer offset types
In reply to Ben HutchingsConsistently use dma_addr_t for DMA addresses, except in fst_{rx,tx}_dma where we truncate them to 32 bits. (We know this is OK because that's the default DMA mask for PCI devices.)
Consistently use u32 for buffer offsets within shared memory.
Compile-tested only.
--- drivers/net/wan/farsync.c | 19 +++++++------------ 1 files changed, 7 insertions(+), 12 deletions(-)
drivers/net/wan/farsync.c +7 -12
@@ -886,15 +886,13 @@ fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port,* Receive a frame through the DMA */ static inline void -fst_rx_dma (struct fst_card_info *card,dma_addr_t skb, -dma_addr_t mem,int len) +fst_rx_dma (struct fst_card_info *card,dma_addr_t skb,u32 mem,int len) {/* * This routine will setup the DMA and start it */ -dbg (DBG_RX ,"In fst_rx_dma %lx %lx %d\n" , - (unsigned long ) skb, (unsigned long ) mem, len); +dbg (DBG_RX ,"In fst_rx_dma %x %x %d\n" , (u32 )skb, mem, len);if (card->dmarx_in_progress) {dbg (DBG_ASS ,"In fst_rx_dma while dma in progress\n" ); }
@@ -915,20 +913,19 @@ fst_rx_dma(struct fst_card_info *card, dma_addr_t skb,* Send a frame through the DMA */ static inline void -fst_tx_dma (struct fst_card_info *card,unsigned char *skb, -unsigned char *mem,int len) +fst_tx_dma (struct fst_card_info *card,dma_addr_t skb,u32 mem,int len) {/* * This routine will setup the DMA and start it. */ -dbg (DBG_TX ,"In fst_tx_dma %p %p %d\n" , skb, mem, len); +dbg (DBG_TX ,"In fst_tx_dma %x %x %d\n" , (u32 )skb, mem, len);if (card->dmatx_in_progress) {dbg (DBG_ASS ,"In fst_tx_dma while dma in progress\n" ); } -outl ((unsigned long ) skb, card->pci_conf +DMAPADR1 );/* Copy from here */ -outl ((unsigned long ) mem, card->pci_conf +DMALADR1 );/* to here */ +outl (skb, card->pci_conf +DMAPADR1 );/* Copy from here */ +outl (mem, card->pci_conf +DMALADR1 );/* to here */ outl (len, card->pci_conf +DMASIZ1 );/* for this length */ outl (0x000000004 , card->pci_conf +DMADPR1 );/* In this direction */
@@ -1405,9 +1402,7 @@ do_bottom_half_tx(struct fst_card_info *card) card->dma_len_tx = skb->len; card->dma_txpos = port->txpos;fst_tx_dma (card, - (char *) card-> - tx_dma_handle_card, - (char *) + card->tx_dma_handle_card,BUF_OFFSET (txBuffer[pi] [port->txpos][0 ]), skb->len);
Signature · 1 line
1.7.7.3
Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
I accidentally left signing enabled for this one, which I had intended to disable since you've said that patchwork doesn't deal with signatures. Except this looks OK on patchwork:
<http://patchwork.ozlabs.org/patch/132548/>.
If that also looks OK to you, is there any reason to avoid signing networking patches?
Ben.
Signature · 2 lines
Ben Hutchings Humans are not rational beings; they are rationalising beings.
David Miller
maintainerRe: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
From: Ben Hutchings <[email protected]>
Date: Wed, 21 Dec 2011 01:42:26 +0000
Quoted 7 lines
I accidentally left signing enabled for this one, which I had intended to disable since you've said that patchwork doesn't deal with signatures. Except this looks OK on patchwork:
<http://patchwork.ozlabs.org/patch/132548/>.If that also looks OK to you, is there any reason to avoid signing networking patches?
What do you mean exactly by "signing".
Do you mean simply the "Signed-off-by: " tag, which you should always add to your own patches, or something else?
Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
Quoted 8 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Wed, 21 Dec 2011 01:42:26 +0000Quoted 5 lines
I accidentally left signing enabled for this one, which I had intended to disable since you've said that patchwork doesn't deal with signatures. Except this looks OK on patchwork:
<http://patchwork.ozlabs.org/patch/132548/>.
Quoted 7 lines
Quoted 2 lines
If that also looks OK to you, is there any reason to avoid signing networking patches?
What do you mean exactly by "signing".
Do you mean simply the "Signed-off-by: " tag, which you should always add to your own patches, or something else?
GPG signing.
Ben.
Signature · 2 lines
Ben Hutchings Humans are not rational beings; they are rationalising beings.
David Miller
maintainerRe: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
In reply to Ben HutchingsFrom: Ben Hutchings <[email protected]>
Date: Wed, 21 Dec 2011 01:32:52 +0000
drivers/net/wan/farsync.c fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port, · line 895
Quoted 3 lines
drivers/net/wan/farsync.c +1
-2-dbg (DBG_RX ,"In fst_rx_dma %lx %lx %d\n" , - (unsigned long ) skb, (unsigned long ) mem, len); +dbg (DBG_RX ,"In fst_rx_dma %x %x %d\n" , (u32 )skb, mem, len);
This is more appropriately fixed by using "%p" instead of casting to a 32-bit int.
Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
Quoted 9 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Wed, 21 Dec 2011 01:32:52 +0000drivers/net/wan/farsync.c fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port, · line 895
Quoted 3 lines
drivers/net/wan/farsync.c +1
-2-dbg (DBG_RX ,"In fst_rx_dma %lx %lx %d\n" , - (unsigned long ) skb, (unsigned long ) mem, len); +dbg (DBG_RX ,"In fst_rx_dma %x %x %d\n" , (u32 )skb, mem, len);This is more appropriately fixed by using "%p" instead of casting to a 32-bit int.
'skb' is the physical address of the data in the skb. Whereas 'mem' is the offset of the corresponding buffer in shared memory.
All completely clear, right? ;-)
Ben.
Signature · 2 lines
Ben Hutchings Humans are not rational beings; they are rationalising beings.
RE: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
Hi,
Thanks for the patches. I will apply them to the master copy of the source code here.
Regards
Kevin Curtis
Linux Development
FarSite Communications Ltd http://www.farsite.com
Winner of The Queen's Award for Enterprise 2009
tel: +44 1256 330461
fax: +44 1256 854931
-----Original Message-----
From: Ben Hutchings [mailto:[email protected]]
Sent: 21 December 2011 05:41
To: David Miller
Cc: Kevin Curtis; [email protected]; [email protected]
Subject: Re: [PATCH 2/8] farsync: Fix confusion about DMA address and buffer offset types
Quoted 9 lines · David Miller
From: Ben Hutchings <[email protected]>
Date: Wed, 21 Dec 2011 01:32:52 +0000drivers/net/wan/farsync.c fst_rx_dma_complete(struct fst_card_info *card, struct fst_port_info *port, · line 895
Quoted 3 lines
drivers/net/wan/farsync.c +1
-2-dbg (DBG_RX ,"In fst_rx_dma %lx %lx %d\n" , - (unsigned long ) skb, (unsigned long ) mem, len); +dbg (DBG_RX ,"In fst_rx_dma %x %x %d\n" , (u32 )skb, mem, len);This is more appropriately fixed by using "%p" instead of casting to a 32-bit int.
'skb' is the physical address of the data in the skb. Whereas 'mem' is the offset of the corresponding buffer in shared memory.
All completely clear, right? ;-)
Ben.
--
Ben Hutchings
Humans are not rational beings; they are rationalising beings.
PATCH 3/8 drm: Do not include page offset in argument to virt_to_page()
In reply to Ben HutchingsBy definition, the page offset will not affect the result.
Compile-tested only.
--- drivers/gpu/drm/drm_vm.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/gpu/drm/drm_vm.c +1 -1
@@ -306,7 +306,7 @@ static int drm_do_vm_dma_fault(struct vm_area_struct *vma, struct vm_fault *vmf) offset = (unsigned long )vmf->virtual_address - vma->vm_start;/* vm_[pg]off[set] should be 0 */ page_nr = offset >>PAGE_SHIFT ;/* page_nr could just be vmf->pgoff */ - page =virt_to_page ((dma->pagelist[page_nr] + (offset & (~PAGE_MASK )))); + page =virt_to_page (dma->pagelist[page_nr]);get_page (page); vmf->page = page;
Signature · 1 line
1.7.7.3
PATCH 4/8 drm: Pass pointers to virt_to_page()
In reply to Ben HutchingsMost architectures define virt_to_page() as a macro that casts its argument such that an argument of type unsigned long will be accepted without complaint. However, the proper type is void *, and passing unsigned long results in a warning on MIPS.
--- drivers/gpu/drm/drm_pci.c | 4 ++-- drivers/gpu/drm/drm_vm.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/drm_pci.c +2 -2
@@ -82,7 +82,7 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali/* Reserve */ for (addr = (unsigned long )dmah->vaddr, sz = size; sz >0 ; addr +=PAGE_SIZE , sz -=PAGE_SIZE ) { -SetPageReserved (virt_to_page (addr)); +SetPageReserved (virt_to_page ((void *)addr)); }return dmah;
@@ -107,7 +107,7 @@ void __drm_pci_free(struct drm_device * dev, drm_dma_handle_t * dmah)/* Unreserve */ for (addr = (unsigned long )dmah->vaddr, sz = dmah->size; sz >0 ; addr +=PAGE_SIZE , sz -=PAGE_SIZE ) { -ClearPageReserved (virt_to_page (addr)); +ClearPageReserved (virt_to_page ((void *)addr)); }dma_free_coherent (&dev->pdev->dev, dmah->size, dmah->vaddr, dmah->busaddr);
drivers/gpu/drm/drm_vm.c +1 -1
@@ -306,7 +306,7 @@ static int drm_do_vm_dma_fault(struct vm_area_struct *vma, struct vm_fault *vmf) offset = (unsigned long )vmf->virtual_address - vma->vm_start;/* vm_[pg]off[set] should be 0 */ page_nr = offset >>PAGE_SHIFT ;/* page_nr could just be vmf->pgoff */ - page =virt_to_page (dma->pagelist[page_nr]); + page =virt_to_page ((void *)dma->pagelist[page_nr]);get_page (page); vmf->page = page;
Signature · 1 line
1.7.7.3
PATCH 5/8 tgt: Pass pointers to virt_to_page(), not integers
In reply to Ben HutchingsMost architectures define virt_to_page() as a macro that casts its argument such that an argument of type unsigned long will be accepted without complaint. However, the proper type is void *, and passing unsigned long results in a warning on MIPS.
--- drivers/scsi/scsi_tgt_if.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/scsi/scsi_tgt_if.c +1 -1
@@ -286,7 +286,7 @@ static int uspace_ring_map(struct vm_area_struct *vma, unsigned long addr,int i, err;for (i =0 ; i <TGT_RING_PAGES ; i++) { -struct page *page =virt_to_page (ring->tr_pages[i]); +struct page *page =virt_to_page ((void *)ring->tr_pages[i]); err =vm_insert_page (vma, addr, page);if (err)return err;
Signature · 1 line
1.7.7.3
PATCH 6/8 uio: Pass pointers to virt_to_page(), not integers
In reply to Ben HutchingsMost architectures define virt_to_page() as a macro that casts its argument such that an argument of type unsigned long will be accepted without complaint. However, the proper type is void *, and passing unsigned long results in a warning on MIPS.
---
It might be clearer to assign:
addr = (void *)((unsigned long)idev->info->mem[mi].addr + offset);
since arithmetic on void pointers is weird. Either way should work though.
Ben.
drivers/uio/uio.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
drivers/uio/uio.c +4 -2
@@ -620,6 +620,7 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)struct uio_device *idev = vma->vm_private_data;struct page *page;unsigned long offset; +void *addr;int mi =uio_find_mem_index (vma);if (mi <0 )
@@ -631,10 +632,11 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)*/ offset = (vmf->pgoff - mi) <<PAGE_SHIFT ; + addr = (void *)(unsigned long )idev->info->mem[mi].addr + offset;if (idev->info->mem[mi].memtype ==UIO_MEM_LOGICAL ) - page =virt_to_page (idev->info->mem[mi].addr + offset); + page =virt_to_page (addr);else - page =vmalloc_to_page ((void *)(unsigned long )idev->info->mem[mi].addr + offset); + page =vmalloc_to_page (addr);get_page (page); vmf->page = page;return 0 ;
Signature · 1 line
1.7.7.3
Re: [PATCH 6/8] uio: Pass pointers to virt_to_page(), not integers
Quoted 4 lines · Ben Hutchings
Most architectures define
virt_to_page()as a macro that casts its argument such that an argument of type unsigned long will be accepted without complaint. However, the proper type is void *, and passing unsigned long results in a warning on MIPS.
Not only on MIPS... This is already fixed in Greg's tree and will go to mainline in the next merge window.
Thanks,
Hans
Quoted 46 lines
Signed-off-by Ben Hutchings <[email protected]>---
It might be clearer to assign:addr = (void *)((unsigned long)idev->info->mem[mi].addr + offset);since arithmetic on void pointers is weird. Either way should work though.
Ben.
drivers/uio/uio.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)drivers/uio/uio.c +4
-2@@ -620,6 +620,7 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)struct uio_device *idev = vma->vm_private_data;struct page *page;unsigned long offset; +void *addr;int mi =uio_find_mem_index (vma);if (mi <0 )@@ -631,10 +632,11 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)*/ offset = (vmf->pgoff - mi) <<PAGE_SHIFT ; + addr = (void *)(unsigned long )idev->info->mem[mi].addr + offset;if (idev->info->mem[mi].memtype ==UIO_MEM_LOGICAL ) - page =virt_to_page (idev->info->mem[mi].addr + offset); + page =virt_to_page (addr);else - page =vmalloc_to_page ((void *)(unsigned long )idev->info->mem[mi].addr + offset); + page =vmalloc_to_page (addr);get_page (page); vmf->page = page;return 0 ;Signature · 1 line
1.7.7.3
PATCH 7/8 rds: Pass pointers to virt_to_page(), not integers
In reply to Ben HutchingsMost architectures define virt_to_page() as a macro that casts its argument such that an argument of type unsigned long will be accepted without complaint. However, the proper type is void *, and passing unsigned long results in a warning on MIPS.
--- net/rds/message.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
net/rds/message.c +1 -1
@@ -257,7 +257,7 @@ struct rds_message *rds_message_map_pages(unsigned long *page_addrs, unsigned infor (i =0 ; i < rm->data.op_nents; ++i) {sg_set_page (&rm->data.op_sg[i], -virt_to_page (page_addrs[i]), +virt_to_page ((void *)page_addrs[i]),PAGE_SIZE ,0 ); }
Signature · 1 line
1.7.7.3
PATCH 8/8 pmcraid: Pass pointers to access_ok(), not integers
In reply to Ben HutchingsMost architectures define access_ok() as a macro that casts its argument such that an argument of type unsigned long will be accepted without complaint. However, the proper type is void *, and passing unsigned long results in a warning on sparc64.
--- drivers/scsi/pmcraid.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)
drivers/scsi/pmcraid.c +2 -1
@@ -3807,7 +3807,8 @@ static long pmcraid_ioctl_passthrough( }if (request_size >0 ) { - rc =access_ok (access, arg, request_offset + request_size); + rc =access_ok (access, (void *)arg, + request_offset + request_size);if (!rc) { rc = -EFAULT ;
Signature · 1 line
1.7.7.3