netdev - Re: [PATCH 3/20][BNX2]: Add 40-bit DMA workaround for 5708.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <46383883.8070209@garzik.org>
Date:	Wed, 02 May 2007 03:06:43 -0400
From:	Jeff Garzik <jeff@...zik.org>
To:	Michael Chan <mchan@...adcom.com>
CC:	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH 3/20][BNX2]: Add 40-bit DMA workaround for 5708.

Michael Chan wrote:
> [BNX2]: Add 40-bit DMA workaround for 5708.
> 
> The internal PCIE-to-PCIX bridge of the 5708 has the same 40-bit DMA
> limitation as some of the tg3 chips.  Use the same workaround used in
> tg3.  On 64-bit systems without IOMMU, linearize the SKB if any
> address is > 40-bit.
> 
> Signed-off-by: Michael Chan <mchan@...adcom.com>
> 
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 6d05397..dba4088 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -4495,6 +4495,93 @@ bnx2_vlan_rx_kill_vid(struct net_device *dev, uint16_t vid)
>  }
>  #endif
>  
> +/* Test for DMA addresses > 40-bit.
> + * Only 64-bit systems without IOMMU require DMA address checking.
> + */
> +static inline int bnx2_40bit_overflow_test(struct bnx2 *bp, dma_addr_t mapping,
> +					   int len)
> +{
> +#if defined(CONFIG_HIGHMEM) && (BITS_PER_LONG == 64)
> +	if (CHIP_NUM(bp) == CHIP_NUM_5708)
> +		return (((u64) mapping + len) > DMA_40BIT_MASK);

If the mapping is at a very high address, adding len might overflow, yes?



> +#if defined(CONFIG_HIGHMEM) && (BITS_PER_LONG == 64)
> +	if (unlikely(would_hit_hwbug)) {
> +		/* If the workaround fails due to memory/mapping
> +		 * failure, silently drop this packet.
> +		 */
> +		if (bnx2_dma_hwbug_workaround(bp, &skb, &prod,
> +					      vlan_tag_flags, mss))
> +			return NETDEV_TX_OK;
> +
> +	}
> +#endif

You need to at least account for the drop, in packets-dropped stat.

/Completely/ silent intentional packet drops are something to be 
avoided.  The user should be given /some/ indication of what is going on.

Second, CONFIG_HIGHMEM is a bad test for IOMMU presence.  I don't think 
you /can/ test for IOMMU presence.  Maybe DaveM knows a way that I do not?



> +	/* 5708 cannot support DMA addresses > 40-bit.
> +	 * On 64-bit systems with IOMMU, use 40-bit dma_mask.
> +	 * On 64-bit systems without IOMMU, use 64-bit dma_mask and
> +	 * do DMA address check in bnx2_start_xmit().
> +	 */
> +	if (CHIP_NUM(bp) == CHIP_NUM_5708) {
> +		persist_dma_mask = dma_mask = DMA_40BIT_MASK;
> +#ifdef CONFIG_HIGHMEM
> +		dma_mask = DMA_64BIT_MASK;
> +#endif
> +	} else
> +		persist_dma_mask = dma_mask = DMA_64BIT_MASK;
> +
> +	/* Configure DMA attributes. */
> +	if (pci_set_dma_mask(pdev, dma_mask) == 0) {
> +		dev->features |= NETIF_F_HIGHDMA;
> +		rc = pci_set_consistent_dma_mask(pdev, persist_dma_mask);
> +		if (rc) {
> +			dev_err(&pdev->dev,
> +				"pci_set_consistent_dma_mask failed, aborting.\n");
> +			goto err_out_unmap;
> +		}
> +	} else if ((rc = pci_set_dma_mask(pdev, DMA_32BIT_MASK)) != 0) {
> +		dev_err(&pdev->dev, "System does not support DMA, aborting.\n");
> +		goto err_out_unmap;
> +	}
> +

Ditto.  CONFIG_HIGHMEM is not an effective test.

NAK.

-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html