* [PATCH] Fix IOMMU overflow checking in s2io.c
@ 2008-06-18 11:58 Andi Kleen
2008-06-26 2:16 ` Ramkrishna Vepa
2008-06-27 5:32 ` Jeff Garzik
0 siblings, 2 replies; 3+ messages in thread
From: Andi Kleen @ 2008-06-18 11:58 UTC (permalink / raw)
To: jeff, netdev, ram.vepa, santosh.rastapur, sivakumar.subramani,
sreenivasa.honnur
Fix IOMMU overflow checking in s2io.c
s2io has IOMMU overflow checking, but unfortunately it is wrong.
It didn't use the standard macros, which meant that it only worked
on POWER and SPARC because only those define DMA_ERROR_CODE. Convert it to
use the standard macros instead.
I also commented two more bugs in the IOMMU handling. It assumes
that 0 DMA addresses cannot happen, but that's not true in all IOMMU setups.
The information if a buffer has been already mapped needs to be stored
elsewhere.
Didn't fix those because it needs careful checking of the buffer handling
by the maintainers.
Cc: ram.vepa@neterion.com
Cc: santosh.rastapur@neterion.com
Cc: sivakumar.subramani@neterion.com
Cc: sreenivasa.honnur@neterion.com
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Index: linux/drivers/net/s2io.c
===================================================================
--- linux.orig/drivers/net/s2io.c
+++ linux/drivers/net/s2io.c
@@ -2625,9 +2625,7 @@ static int fill_rx_buffers(struct ring_i
rxdp1->Buffer0_ptr = pci_map_single
(ring->pdev, skb->data, size - NET_IP_ALIGN,
PCI_DMA_FROMDEVICE);
- if( (rxdp1->Buffer0_ptr == 0) ||
- (rxdp1->Buffer0_ptr ==
- DMA_ERROR_CODE))
+ if(pci_dma_mapping_error(rxdp1->Buffer0_ptr))
goto pci_map_failed;
rxdp->Control_2 =
@@ -2657,6 +2655,7 @@ static int fill_rx_buffers(struct ring_i
skb->data = (void *) (unsigned long)tmp;
skb_reset_tail_pointer(skb);
+ /* AK: check is wrong. 0 can be valid dma address */
if (!(rxdp3->Buffer0_ptr))
rxdp3->Buffer0_ptr =
pci_map_single(ring->pdev, ba->ba_0,
@@ -2665,8 +2664,7 @@ static int fill_rx_buffers(struct ring_i
pci_dma_sync_single_for_device(ring->pdev,
(dma_addr_t) rxdp3->Buffer0_ptr,
BUF0_LEN, PCI_DMA_FROMDEVICE);
- if( (rxdp3->Buffer0_ptr == 0) ||
- (rxdp3->Buffer0_ptr == DMA_ERROR_CODE))
+ if (pci_dma_mapping_error(rxdp3->Buffer0_ptr))
goto pci_map_failed;
rxdp->Control_2 = SET_BUFFER0_SIZE_3(BUF0_LEN);
@@ -2681,18 +2679,17 @@ static int fill_rx_buffers(struct ring_i
(ring->pdev, skb->data, ring->mtu + 4,
PCI_DMA_FROMDEVICE);
- if( (rxdp3->Buffer2_ptr == 0) ||
- (rxdp3->Buffer2_ptr == DMA_ERROR_CODE))
+ if (pci_dma_mapping_error(rxdp3->Buffer2_ptr))
goto pci_map_failed;
+ /* AK: check is wrong */
if (!rxdp3->Buffer1_ptr)
rxdp3->Buffer1_ptr =
pci_map_single(ring->pdev,
ba->ba_1, BUF1_LEN,
PCI_DMA_FROMDEVICE);
- if( (rxdp3->Buffer1_ptr == 0) ||
- (rxdp3->Buffer1_ptr == DMA_ERROR_CODE)) {
+ if (pci_dma_mapping_error(rxdp3->Buffer1_ptr)) {
pci_unmap_single
(ring->pdev,
(dma_addr_t)(unsigned long)
@@ -4264,16 +4261,14 @@ static int s2io_xmit(struct sk_buff *skb
txdp->Buffer_Pointer = pci_map_single(sp->pdev,
fifo->ufo_in_band_v,
sizeof(u64), PCI_DMA_TODEVICE);
- if((txdp->Buffer_Pointer == 0) ||
- (txdp->Buffer_Pointer == DMA_ERROR_CODE))
+ if (pci_dma_mapping_error(txdp->Buffer_Pointer))
goto pci_map_failed;
txdp++;
}
txdp->Buffer_Pointer = pci_map_single
(sp->pdev, skb->data, frg_len, PCI_DMA_TODEVICE);
- if((txdp->Buffer_Pointer == 0) ||
- (txdp->Buffer_Pointer == DMA_ERROR_CODE))
+ if (pci_dma_mapping_error(txdp->Buffer_Pointer))
goto pci_map_failed;
txdp->Host_Control = (unsigned long) skb;
@@ -6884,10 +6879,8 @@ static int set_rxd_buffer_pointer(struct
pci_map_single( sp->pdev, (*skb)->data,
size - NET_IP_ALIGN,
PCI_DMA_FROMDEVICE);
- if( (rxdp1->Buffer0_ptr == 0) ||
- (rxdp1->Buffer0_ptr == DMA_ERROR_CODE)) {
+ if (pci_dma_mapping_error(rxdp1->Buffer0_ptr))
goto memalloc_failed;
- }
rxdp->Host_Control = (unsigned long) (*skb);
}
} else if ((sp->rxd_mode == RXD_MODE_3B) && (rxdp->Host_Control == 0)) {
@@ -6913,15 +6906,12 @@ static int set_rxd_buffer_pointer(struct
pci_map_single(sp->pdev, (*skb)->data,
dev->mtu + 4,
PCI_DMA_FROMDEVICE);
- if( (rxdp3->Buffer2_ptr == 0) ||
- (rxdp3->Buffer2_ptr == DMA_ERROR_CODE)) {
+ if (pci_dma_mapping_error(rxdp3->Buffer2_ptr))
goto memalloc_failed;
- }
rxdp3->Buffer0_ptr = *temp0 =
pci_map_single( sp->pdev, ba->ba_0, BUF0_LEN,
PCI_DMA_FROMDEVICE);
- if( (rxdp3->Buffer0_ptr == 0) ||
- (rxdp3->Buffer0_ptr == DMA_ERROR_CODE)) {
+ if (pci_dma_mapping_error(rxdp3->Buffer0_ptr)) {
pci_unmap_single (sp->pdev,
(dma_addr_t)rxdp3->Buffer2_ptr,
dev->mtu + 4, PCI_DMA_FROMDEVICE);
@@ -6933,8 +6923,7 @@ static int set_rxd_buffer_pointer(struct
rxdp3->Buffer1_ptr = *temp1 =
pci_map_single(sp->pdev, ba->ba_1, BUF1_LEN,
PCI_DMA_FROMDEVICE);
- if( (rxdp3->Buffer1_ptr == 0) ||
- (rxdp3->Buffer1_ptr == DMA_ERROR_CODE)) {
+ if (pci_dma_mapping_error(rxdp3->Buffer1_ptr)) {
pci_unmap_single (sp->pdev,
(dma_addr_t)rxdp3->Buffer0_ptr,
BUF0_LEN, PCI_DMA_FROMDEVICE);
Index: linux/drivers/net/s2io.h
===================================================================
--- linux.orig/drivers/net/s2io.h
+++ linux/drivers/net/s2io.h
@@ -75,10 +75,6 @@ static int debug_level = ERR_DBG;
/* DEBUG message print. */
#define DBG_PRINT(dbg_level, args...) if(!(debug_level<dbg_level)) printk(args)
-#ifndef DMA_ERROR_CODE
-#define DMA_ERROR_CODE (~(dma_addr_t)0x0)
-#endif
-
/* Protocol assist features of the NIC */
#define L3_CKSUM_OK 0xFFFF
#define L4_CKSUM_OK 0xFFFF
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [PATCH] Fix IOMMU overflow checking in s2io.c
2008-06-18 11:58 [PATCH] Fix IOMMU overflow checking in s2io.c Andi Kleen
@ 2008-06-26 2:16 ` Ramkrishna Vepa
2008-06-27 5:32 ` Jeff Garzik
1 sibling, 0 replies; 3+ messages in thread
From: Ramkrishna Vepa @ 2008-06-26 2:16 UTC (permalink / raw)
To: Andi Kleen, jeff, netdev, Rastapur Santosh, Sivakumar Subramani
Andi,
Thanks for this fix. We will recreate a new patch which includes the fix
mentioned in your comments and resubmit.
Ram
> -----Original Message-----
> From: Andi Kleen [mailto:andi@firstfloor.org]
> Sent: Wednesday, June 18, 2008 4:59 AM
> To: jeff@garzik.org; netdev@vger.kernel.org; ram.vepa@neterion.com;
> Rastapur Santosh; Sivakumar Subramani; Sreenivasa Honnur
> Subject: [PATCH] Fix IOMMU overflow checking in s2io.c
>
> Fix IOMMU overflow checking in s2io.c
>
> s2io has IOMMU overflow checking, but unfortunately it is wrong.
>
> It didn't use the standard macros, which meant that it only worked
> on POWER and SPARC because only those define DMA_ERROR_CODE. Convert
it to
> use the standard macros instead.
>
> I also commented two more bugs in the IOMMU handling. It assumes
> that 0 DMA addresses cannot happen, but that's not true in all IOMMU
> setups.
> The information if a buffer has been already mapped needs to be stored
> elsewhere.
>
> Didn't fix those because it needs careful checking of the buffer
handling
> by the maintainers.
>
> Cc: ram.vepa@neterion.com
> Cc: santosh.rastapur@neterion.com
> Cc: sivakumar.subramani@neterion.com
> Cc: sreenivasa.honnur@neterion.com
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
>
> Index: linux/drivers/net/s2io.c
> ===================================================================
> --- linux.orig/drivers/net/s2io.c
> +++ linux/drivers/net/s2io.c
> @@ -2625,9 +2625,7 @@ static int fill_rx_buffers(struct ring_i
> rxdp1->Buffer0_ptr = pci_map_single
> (ring->pdev, skb->data, size - NET_IP_ALIGN,
> PCI_DMA_FROMDEVICE);
> - if( (rxdp1->Buffer0_ptr == 0) ||
> - (rxdp1->Buffer0_ptr ==
> - DMA_ERROR_CODE))
> + if(pci_dma_mapping_error(rxdp1->Buffer0_ptr))
> goto pci_map_failed;
>
> rxdp->Control_2 =
> @@ -2657,6 +2655,7 @@ static int fill_rx_buffers(struct ring_i
> skb->data = (void *) (unsigned long)tmp;
> skb_reset_tail_pointer(skb);
>
> + /* AK: check is wrong. 0 can be valid dma
address */
> if (!(rxdp3->Buffer0_ptr))
> rxdp3->Buffer0_ptr =
> pci_map_single(ring->pdev, ba->ba_0,
> @@ -2665,8 +2664,7 @@ static int fill_rx_buffers(struct ring_i
>
pci_dma_sync_single_for_device(ring->pdev,
> (dma_addr_t) rxdp3->Buffer0_ptr,
> BUF0_LEN, PCI_DMA_FROMDEVICE);
> - if( (rxdp3->Buffer0_ptr == 0) ||
> - (rxdp3->Buffer0_ptr == DMA_ERROR_CODE))
> + if (pci_dma_mapping_error(rxdp3->Buffer0_ptr))
> goto pci_map_failed;
>
> rxdp->Control_2 = SET_BUFFER0_SIZE_3(BUF0_LEN);
> @@ -2681,18 +2679,17 @@ static int fill_rx_buffers(struct ring_i
> (ring->pdev, skb->data, ring->mtu + 4,
> PCI_DMA_FROMDEVICE);
>
> - if( (rxdp3->Buffer2_ptr == 0) ||
> - (rxdp3->Buffer2_ptr ==
DMA_ERROR_CODE))
> + if
(pci_dma_mapping_error(rxdp3->Buffer2_ptr))
> goto pci_map_failed;
>
> + /* AK: check is wrong */
> if (!rxdp3->Buffer1_ptr)
> rxdp3->Buffer1_ptr =
>
pci_map_single(ring->pdev,
> ba->ba_1, BUF1_LEN,
> PCI_DMA_FROMDEVICE);
>
> - if( (rxdp3->Buffer1_ptr == 0) ||
> - (rxdp3->Buffer1_ptr ==
DMA_ERROR_CODE)) {
> + if
(pci_dma_mapping_error(rxdp3->Buffer1_ptr)) {
> pci_unmap_single
> (ring->pdev,
> (dma_addr_t)(unsigned
long)
> @@ -4264,16 +4261,14 @@ static int s2io_xmit(struct sk_buff *skb
> txdp->Buffer_Pointer = pci_map_single(sp->pdev,
> fifo->ufo_in_band_v,
> sizeof(u64), PCI_DMA_TODEVICE);
> - if((txdp->Buffer_Pointer == 0) ||
> - (txdp->Buffer_Pointer == DMA_ERROR_CODE))
> + if (pci_dma_mapping_error(txdp->Buffer_Pointer))
> goto pci_map_failed;
> txdp++;
> }
>
> txdp->Buffer_Pointer = pci_map_single
> (sp->pdev, skb->data, frg_len, PCI_DMA_TODEVICE);
> - if((txdp->Buffer_Pointer == 0) ||
> - (txdp->Buffer_Pointer == DMA_ERROR_CODE))
> + if (pci_dma_mapping_error(txdp->Buffer_Pointer))
> goto pci_map_failed;
>
> txdp->Host_Control = (unsigned long) skb;
> @@ -6884,10 +6879,8 @@ static int set_rxd_buffer_pointer(struct
> pci_map_single( sp->pdev, (*skb)->data,
> size - NET_IP_ALIGN,
> PCI_DMA_FROMDEVICE);
> - if( (rxdp1->Buffer0_ptr == 0) ||
> - (rxdp1->Buffer0_ptr == DMA_ERROR_CODE))
{
> + if (pci_dma_mapping_error(rxdp1->Buffer0_ptr))
> goto memalloc_failed;
> - }
> rxdp->Host_Control = (unsigned long) (*skb);
> }
> } else if ((sp->rxd_mode == RXD_MODE_3B) && (rxdp->Host_Control
==
> 0)) {
> @@ -6913,15 +6906,12 @@ static int set_rxd_buffer_pointer(struct
> pci_map_single(sp->pdev, (*skb)->data,
> dev->mtu + 4,
> PCI_DMA_FROMDEVICE);
> - if( (rxdp3->Buffer2_ptr == 0) ||
> - (rxdp3->Buffer2_ptr == DMA_ERROR_CODE))
{
> + if (pci_dma_mapping_error(rxdp3->Buffer2_ptr))
> goto memalloc_failed;
> - }
> rxdp3->Buffer0_ptr = *temp0 =
> pci_map_single( sp->pdev, ba->ba_0,
BUF0_LEN,
> PCI_DMA_FROMDEVICE);
> - if( (rxdp3->Buffer0_ptr == 0) ||
> - (rxdp3->Buffer0_ptr == DMA_ERROR_CODE))
{
> + if (pci_dma_mapping_error(rxdp3->Buffer0_ptr)) {
> pci_unmap_single (sp->pdev,
> (dma_addr_t)rxdp3->Buffer2_ptr,
> dev->mtu + 4,
PCI_DMA_FROMDEVICE);
> @@ -6933,8 +6923,7 @@ static int set_rxd_buffer_pointer(struct
> rxdp3->Buffer1_ptr = *temp1 =
> pci_map_single(sp->pdev, ba->ba_1,
BUF1_LEN,
> PCI_DMA_FROMDEVICE);
> - if( (rxdp3->Buffer1_ptr == 0) ||
> - (rxdp3->Buffer1_ptr == DMA_ERROR_CODE))
{
> + if (pci_dma_mapping_error(rxdp3->Buffer1_ptr)) {
> pci_unmap_single (sp->pdev,
> (dma_addr_t)rxdp3->Buffer0_ptr,
> BUF0_LEN, PCI_DMA_FROMDEVICE);
> Index: linux/drivers/net/s2io.h
> ===================================================================
> --- linux.orig/drivers/net/s2io.h
> +++ linux/drivers/net/s2io.h
> @@ -75,10 +75,6 @@ static int debug_level = ERR_DBG;
> /* DEBUG message print. */
> #define DBG_PRINT(dbg_level, args...) if(!(debug_level<dbg_level))
> printk(args)
>
> -#ifndef DMA_ERROR_CODE
> -#define DMA_ERROR_CODE (~(dma_addr_t)0x0)
> -#endif
> -
> /* Protocol assist features of the NIC */
> #define L3_CKSUM_OK 0xFFFF
> #define L4_CKSUM_OK 0xFFFF
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Fix IOMMU overflow checking in s2io.c
2008-06-18 11:58 [PATCH] Fix IOMMU overflow checking in s2io.c Andi Kleen
2008-06-26 2:16 ` Ramkrishna Vepa
@ 2008-06-27 5:32 ` Jeff Garzik
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Garzik @ 2008-06-27 5:32 UTC (permalink / raw)
To: Andi Kleen
Cc: netdev, ram.vepa, santosh.rastapur, sivakumar.subramani,
sreenivasa.honnur
Andi Kleen wrote:
> Fix IOMMU overflow checking in s2io.c
>
> s2io has IOMMU overflow checking, but unfortunately it is wrong.
>
> It didn't use the standard macros, which meant that it only worked
> on POWER and SPARC because only those define DMA_ERROR_CODE. Convert it to
> use the standard macros instead.
>
> I also commented two more bugs in the IOMMU handling. It assumes
> that 0 DMA addresses cannot happen, but that's not true in all IOMMU setups.
> The information if a buffer has been already mapped needs to be stored
> elsewhere.
>
> Didn't fix those because it needs careful checking of the buffer handling
> by the maintainers.
>
> Cc: ram.vepa@neterion.com
> Cc: santosh.rastapur@neterion.com
> Cc: sivakumar.subramani@neterion.com
> Cc: sreenivasa.honnur@neterion.com
>
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
applied
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-06-27 5:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-18 11:58 [PATCH] Fix IOMMU overflow checking in s2io.c Andi Kleen
2008-06-26 2:16 ` Ramkrishna Vepa
2008-06-27 5:32 ` Jeff Garzik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).