netdev - Re: [PATCH 3/3] net: hisilicon: new hip04 ethernet driver

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] [day] [month] [year] [list]

Message-ID: <CAMj5Bkg1_a=tAw1sxOTzSG6eWUHT8QVPXOzx2C+oCyf+dPr7Uw@mail.gmail.com>
Date:	Wed, 26 Mar 2014 22:22:35 +0800
From:	Zhangfei Gao <zhangfei.gao@...il.com>
To:	Rob Herring <robherring2@...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

Dear Rob

On Mon, Mar 24, 2014 at 10:17 PM, Rob Herring <robherring2@...il.com> wrote:

>
>>>> +             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.

The warning is solved via adding dma_mapping_error check after every
dma_map_single.

>
>
>>>> +             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.

That would be very helpful, still not sure how to enable this feature.
Could you help clarify which file.
Not find highbank and mvebu under drivers/iommu/.

Thanks
--
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