All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] r8169 DMA failure with iommu=off
@ 2015-04-29 23:58 Stephen Hemminger
  2015-05-01 12:12 ` Francois Romieu
  2015-05-02  1:46 ` David Miller
  0 siblings, 2 replies; 3+ messages in thread
From: Stephen Hemminger @ 2015-04-29 23:58 UTC (permalink / raw)
  To: Francois Romieu, Eric Dumazet; +Cc: netdev

This only fixes the Rx side, what about Tx?

I think maybe just removing the whole use_dac flag completely?

Subject: r8169: allocate rx memory in correct region

This fixes failure when using r8169 with IOMMU defined but not
enabled. Reproduced on a 16G machine with LOM r8169 and kernel
set to 'iommu=off'.

This driver has two dma modes. By default use_dac module parameter
is zero and therefore the driver uses only has a 32 bit DMA
mask. But since it calls kmalloc_node() without setting DMA32 flag
the receive buffer maybe above 4G and the dma_map_single will fail.

In either case since this is a receive DMA buffer, it should set
the appropriate GFP_DMA since that may matter on some platforms.

This an old bug was introduced by:

commit 6f0333b8fde44b8c04a53b2461504f0e8f1cebe6
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date:   Mon Oct 11 11:17:47 2010 +0000

    r8169: use 50% less ram for RX ring
    
    Using standard skb allocations in r8169 leads to order-3 allocations (if
    PAGE_SIZE=4096), because NIC needs 16383 bytes, and skb overhead makes
    this bigger than 16384 -> 32768 bytes per "skb"
    
    Using kmalloc() permits to reduce memory requirements of one r8169 nic
    by 4Mbytes. (256 frames * 16Kbytes). This is fine since a hardware bug
    requires us to copy incoming frames, so we build real skb when doing
    this copy.
    
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -5721,14 +5721,16 @@ static struct sk_buff *rtl8169_alloc_rx_
 	struct device *d = &tp->pci_dev->dev;
 	struct net_device *dev = tp->dev;
 	int node = dev->dev.parent ? dev_to_node(dev->dev.parent) : -1;
+	gfp_t flags = GFP_KERNEL;
 
-	data = kmalloc_node(rx_buf_sz, GFP_KERNEL, node);
+	flags |= (dev->features & NETIF_F_HIGHDMA) ? GFP_DMA : GFP_DMA32;
+	data = kmalloc_node(rx_buf_sz, flags, node);
 	if (!data)
 		return NULL;
 
 	if (rtl8169_align(data) != data) {
 		kfree(data);
-		data = kmalloc_node(rx_buf_sz + 15, GFP_KERNEL, node);
+		data = kmalloc_node(rx_buf_sz + 15, flags, node);
 		if (!data)
 			return NULL;
 	}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] r8169 DMA failure with iommu=off
  2015-04-29 23:58 [RFC] r8169 DMA failure with iommu=off Stephen Hemminger
@ 2015-05-01 12:12 ` Francois Romieu
  2015-05-02  1:46 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: Francois Romieu @ 2015-05-01 12:12 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Eric Dumazet, netdev

Stephen Hemminger <shemming@Brocade.com> :
> This only fixes the Rx side, what about Tx?
> 
> I think maybe just removing the whole use_dac flag completely ?

Something simple would be welcome but I doubt it should be that simple.

The problems that the use_dac comment (MODULE_PARM_DESC) relates to
are 2003 ~ 2004, plain old PCI 8169 stuff. I see no problem exchanging
the use_dac test for something else (Config2.BIT(3) test for bus width
or old PCI chipsets blacklist).

I lack explicit reports to tell if high dma is safe with the PCI-E
chipsets but it's worth testing. The DAC bit in the CPlusCmd register
is indirectly documented in my old 8168c datasheet through the 64 bit
address capable bit of the MSI message control word (it's missing in
the section dedicated to the CPlusCmd register :o/). Expect it to
initially mirror some eeprom value.

Other than that I am mostly unqualified when it comes to high DMA paths
in the upper parts of the transmit stack.

-- 
Ueimor

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [RFC] r8169 DMA failure with iommu=off
  2015-04-29 23:58 [RFC] r8169 DMA failure with iommu=off Stephen Hemminger
  2015-05-01 12:12 ` Francois Romieu
@ 2015-05-02  1:46 ` David Miller
  1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2015-05-02  1:46 UTC (permalink / raw)
  To: shemming; +Cc: romieu, eric.dumazet, netdev

From: Stephen Hemminger <shemming@brocade.com>
Date: Wed, 29 Apr 2015 16:58:13 -0700

> In either case since this is a receive DMA buffer, it should set
> the appropriate GFP_DMA since that may matter on some platforms.

Plain GFP_DMA is really only appropriate when used for ISA DMA
situations where we must have 24-bit addresses or whatever that
restriction was.

Anyways, it is dma_map_single()'s job to restrict DMA addressing if
necessary.  Otherwise, the transmit path would not work at all.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-02  1:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-29 23:58 [RFC] r8169 DMA failure with iommu=off Stephen Hemminger
2015-05-01 12:12 ` Francois Romieu
2015-05-02  1:46 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.