On Fri, Sep 25, 2020 at 03:26:26PM +0800, Sherry Sun wrote:
> For noncoherent platform, we should allocate vring through
> dma_alloc_coherent api to ensure timely synchronization of vring.
> The orginal way which used __get_free_pages and dma_map_single only
> apply to coherent platforms.
>
> Signed-off-by: Joakim Zhang <[email protected]>
> Signed-off-by: Sherry Sun <[email protected]>
> ---
> drivers/misc/mic/vop/vop_vringh.c | 101 ++++++++++++++++++++----------
> 1 file changed, 67 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/misc/mic/vop/vop_vringh.c
> b/drivers/misc/mic/vop/vop_vringh.c
> index f344209ac386..fc8d8ff9ded3 100644
> --- a/drivers/misc/mic/vop/vop_vringh.c
> +++ b/drivers/misc/mic/vop/vop_vringh.c
> @@ -9,6 +9,7 @@
> #include <linux/sched.h>
> #include <linux/poll.h>
> #include <linux/dma-mapping.h>
> +#include <linux/dma-noncoherent.h>
>
> #include <linux/mic_common.h>
> #include "../common/mic_dev.h"
> @@ -67,11 +68,12 @@ static void vop_virtio_init_post(struct vop_vdev *vdev)
> dev_warn(vop_dev(vdev), "used_address zero??\n");
> continue;
> }
> - vdev->vvr[i].vrh.vring.used =
> - (void __force *)vpdev->hw_ops->remap(
> - vpdev,
> - le64_to_cpu(vqconfig[i].used_address),
> - used_size);
> + if (dev_is_dma_coherent(vop_dev(vdev)))
> + vdev->vvr[i].vrh.vring.used =
> + (void __force *)vpdev->hw_ops->remap(
> + vpdev,
> + le64_to_cpu(vqconfig[i].used_address),
> + used_size);
That's really hard to read, don't you agree? We can go longer than 80
columns now, and why the __force?
> }
>
> vdev->dc->used_address_updated = 0;
> @@ -298,9 +300,14 @@ static int vop_virtio_add_device(struct vop_vdev *vdev,
> mutex_init(&vvr->vr_mutex);
> vr_size = PAGE_ALIGN(round_up(vring_size(num,
> MIC_VIRTIO_RING_ALIGN), 4) +
> sizeof(struct _mic_vring_info));
> - vr->va = (void *)
> - __get_free_pages(GFP_KERNEL | __GFP_ZERO,
> - get_order(vr_size));
> +
> + if (!dev_is_dma_coherent(vop_dev(vdev)))
> + vr->va = dma_alloc_coherent(vop_dev(vdev), vr_size,
> + &vr_addr, GFP_KERNEL);
Are you sure this is correct? If dev_is_dma_coherent is NOT true, then
you call dma_alloc_coherent()?
> + else
> + vr->va = (void *)
> + __get_free_pages(GFP_KERNEL | __GFP_ZERO,
> + get_order(vr_size));
> if (!vr->va) {
> ret = -ENOMEM;
> dev_err(vop_dev(vdev), "%s %d err %d\n",
> @@ -310,14 +317,17 @@ static int vop_virtio_add_device(struct vop_vdev *vdev,
> vr->len = vr_size;
> vr->info = vr->va + round_up(vring_size(num,
> MIC_VIRTIO_RING_ALIGN), 4);
> vr->info->magic = cpu_to_le32(MIC_MAGIC + vdev->virtio_id + i);
> - vr_addr = dma_map_single(&vpdev->dev, vr->va, vr_size,
> - DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(&vpdev->dev, vr_addr)) {
> - free_pages((unsigned long)vr->va, get_order(vr_size));
> - ret = -ENOMEM;
> - dev_err(vop_dev(vdev), "%s %d err %d\n",
> - __func__, __LINE__, ret);
> - goto err;
> +
> + if (dev_is_dma_coherent(vop_dev(vdev))) {
> + vr_addr = dma_map_single(&vpdev->dev, vr->va, vr_size,
> + DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(&vpdev->dev, vr_addr)) {
> + free_pages((unsigned long)vr->va,
> get_order(vr_size));
> + ret = -ENOMEM;
> + dev_err(vop_dev(vdev), "%s %d err %d\n",
> + __func__, __LINE__, ret);
> + goto err;
> + }
What about if this is not true?
Same for other places in this patch.
thanks,
greg k-h
