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
| ||
|
Date: Mon, 24 Mar 2014 09:17:31 -0500 From: Rob Herring <robherring2@...il.com> To: Zhangfei Gao <zhangfei.gao@...il.com> Cc: Russell King - ARM Linux <linux@....linux.org.uk>, Zhangfei Gao <zhangfei.gao@...aro.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "David S. Miller" <davem@...emloft.net>, linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, netdev <netdev@...r.kernel.org> Subject: Re: [PATCH 3/3] net: hisilicon: new hip04 ethernet driver On Thu, Mar 20, 2014 at 4:51 AM, Zhangfei Gao <zhangfei.gao@...il.com> wrote: > Dear Russell > > Thanks for sparing time and giving so many perfect suggestion, really helpful. > > On Tue, Mar 18, 2014 at 6:46 PM, Russell King - ARM Linux > <linux@....linux.org.uk> wrote: >> I was just browsing this patch when I noticed some of these issues - I >> haven't done a full review of this driver, I'm just commenting on the >> things I've spotted. [snip] >>> + dma_map_single(&ndev->dev, skb->data, >>> + RX_BUF_SIZE, DMA_FROM_DEVICE); >> >> This is incorrect. >> >> buf = buffer alloc() >> /* CPU owns buffer and can read/write it, device does not */ >> dev_addr = dma_map_single(dev, buf, ..., DMA_FROM_DEVICE); >> /* Device owns buffer and can write it, CPU does not access it */ >> dma_unmap_single(dev, dev_addr, ..., DMA_FROM_DEVICE); >> /* CPU owns buffer again and can read/write it, device does not */ >> >> Please turn on DMA API debugging in the kernel debug options and verify >> whether your driver causes it to complain (it will.) > > Yes, you are right. > After change to dma_map/unmap_single, however, still get warning like > "DMA-API: device driver failed to check map error", not sure whether > it can be ignored? If it could be ignored, there would be no warning. So yes you should check the error. I guess correct error handling would be throwing away the packet. >>> + dma_map_single(&ndev->dev, buf, RX_BUF_SIZE, DMA_TO_DEVICE); >>> + hip04_set_recv_desc(priv, virt_to_phys(buf)); >> >> No need for virt_to_phys() here - dma_map_single() returns the device >> address. > Got it. > Use virt_to_phys since find same result come out, it should be > different for iommu case. > > In fact, the hardware can help to do the cache flushing, the function > still not be enabled now. > Then dma_map/unmap_single may be ignored. If you don't need cache flushing, you should setup different dma_map_ops for the device such as arm_coherent_dma_ops. The driver should always have the dma_map calls. See highbank and mvebu for examples. Rob -- 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
Powered by blists - more mailing lists