Message ID | 20200802131107.15857-1-baijiaju@tsinghua.edu.cn |
---|---|
State | Rejected |
Delegated to: | David Miller |
Headers | show |
Series | net: vmxnet3: avoid accessing the data mapped to streaming DMA | expand |
From: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> Date: Sun, 2 Aug 2020 21:11:07 +0800 > In vmxnet3_probe_device(), "adapter" is mapped to streaming DMA: > adapter->adapter_pa = dma_map_single(..., adapter, ...); > > Then "adapter" is accessed at many places in this function. > > Theses accesses may cause data inconsistency between CPU cache and > hardware. > > To fix this problem, dma_map_single() is called after these accesses. > > Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> 'adapter' is accessed everywhere, in the entire driver, not just here in the probe function.
On 2020/8/4 6:59, David Miller wrote: > From: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> > Date: Sun, 2 Aug 2020 21:11:07 +0800 > >> In vmxnet3_probe_device(), "adapter" is mapped to streaming DMA: >> adapter->adapter_pa = dma_map_single(..., adapter, ...); >> >> Then "adapter" is accessed at many places in this function. >> >> Theses accesses may cause data inconsistency between CPU cache and >> hardware. >> >> To fix this problem, dma_map_single() is called after these accesses. >> >> Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> > 'adapter' is accessed everywhere, in the entire driver, not just here > in the probe function. Okay, replacing dma_map_single() with dma_alloc_coherent() may be better. If you think this solution is okay, I can submit a new patch. Best wishes, Jia-Ju Bai
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c index ca395f9679d0..96a4c28ba429 100644 --- a/drivers/net/vmxnet3/vmxnet3_drv.c +++ b/drivers/net/vmxnet3/vmxnet3_drv.c @@ -3445,14 +3445,6 @@ vmxnet3_probe_device(struct pci_dev *pdev, } spin_lock_init(&adapter->cmd_lock); - adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter, - sizeof(struct vmxnet3_adapter), - PCI_DMA_TODEVICE); - if (dma_mapping_error(&adapter->pdev->dev, adapter->adapter_pa)) { - dev_err(&pdev->dev, "Failed to map dma\n"); - err = -EFAULT; - goto err_set_mask; - } adapter->shared = dma_alloc_coherent( &adapter->pdev->dev, sizeof(struct Vmxnet3_DriverShared), @@ -3628,6 +3620,16 @@ vmxnet3_probe_device(struct pci_dev *pdev, } vmxnet3_check_link(adapter, false); + + adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter, + sizeof(struct vmxnet3_adapter), + PCI_DMA_TODEVICE); + if (dma_mapping_error(&adapter->pdev->dev, adapter->adapter_pa)) { + dev_err(&pdev->dev, "Failed to map dma\n"); + err = -EFAULT; + goto err_register; + } + return 0; err_register: @@ -3655,8 +3657,6 @@ vmxnet3_probe_device(struct pci_dev *pdev, sizeof(struct Vmxnet3_DriverShared), adapter->shared, adapter->shared_pa); err_alloc_shared: - dma_unmap_single(&adapter->pdev->dev, adapter->adapter_pa, - sizeof(struct vmxnet3_adapter), PCI_DMA_TODEVICE); err_set_mask: free_netdev(netdev); return err;
In vmxnet3_probe_device(), "adapter" is mapped to streaming DMA: adapter->adapter_pa = dma_map_single(..., adapter, ...); Then "adapter" is accessed at many places in this function. Theses accesses may cause data inconsistency between CPU cache and hardware. To fix this problem, dma_map_single() is called after these accesses. Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> --- drivers/net/vmxnet3/vmxnet3_drv.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)