lkml.org 
[lkml]   [2018]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 06/21] platform: goldfish: pipe: Add DMA support to goldfish pipe
On Fri, Sep 14, 2018 at 10:51:07AM -0700, rkir@google.com wrote:
> From: Roman Kiryanov <rkir@google.com>
>
> Goldfish DMA is an extension to the pipe device and is designed
> to facilitate high-speed RAM->RAM transfers from guest to host.
>
> See uapi/linux/goldfish/goldfish_dma.h for more details.
>
> Signed-off-by: Roman Kiryanov <rkir@google.com>
> ---
> drivers/platform/goldfish/goldfish_pipe.c | 312 +++++++++++++++++-
> .../platform/goldfish/goldfish_pipe_qemu.h | 2 +
> include/uapi/linux/goldfish/goldfish_dma.h | 84 +++++
> 3 files changed, 396 insertions(+), 2 deletions(-)
> create mode 100644 include/uapi/linux/goldfish/goldfish_dma.h

A whole new api needs some others to review it becides just me. Please
get some more signed off by on this.

Also, some minor comments:

> --- /dev/null
> +++ b/include/uapi/linux/goldfish/goldfish_dma.h
> @@ -0,0 +1,84 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2018 Google, Inc.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.

If you have a spdx line, you don't need the gpl boiler-plate text
either.

But, this is a uapi file, so gpl2 is not probably the license you want
here, right? That should be fixed before you end up doing something
foolish with a userspace program that includes this :)

> +/* GOLDFISH DMA
> + *
> + * Goldfish DMA is an extension to the pipe device
> + * and is designed to facilitate high-speed RAM->RAM
> + * transfers from guest to host.
> + *
> + * Interface (guest side):
> + *
> + * The guest user calls goldfish_dma_alloc (ioctls)
> + * and then mmap() on a goldfish pipe fd,
> + * which means that it wants high-speed access to
> + * host-visible memory.
> + *
> + * The guest can then write into the pointer
> + * returned by mmap(), and these writes
> + * become immediately visible on the host without BQL
> + * or otherweise context switching.
> + *
> + * dma_alloc_coherent() is used to obtain contiguous
> + * physical memory regions, and we allocate and interact
> + * with this region on both guest and host through
> + * the following ioctls:
> + *
> + * - LOCK: lock the region for data access.
> + * - UNLOCK: unlock the region. This may also be done from the host
> + * through the WAKE_ON_UNLOCK_DMA procedure.
> + * - CREATE_REGION: initialize size info for a dma region.
> + * - GETOFF: send physical address to guest drivers.
> + * - (UN)MAPHOST: uses goldfish_pipe_cmd to tell the host to
> + * (un)map to the guest physical address associated
> + * with the current dma context. This makes the physically
> + * contiguous memory (in)visible to the host.
> + *
> + * Guest userspace obtains a pointer to the DMA memory
> + * through mmap(), which also lazily allocates the memory
> + * with dma_alloc_coherent. (On last pipe close(), the region is freed).
> + * The mmaped() region can handle very high bandwidth
> + * transfers, and pipe operations can be used at the same
> + * time to handle synchronization and command communication.
> + */
> +
> +#define GOLDFISH_DMA_BUFFER_SIZE (32 * 1024 * 1024)
> +
> +struct goldfish_dma_ioctl_info {
> + __u64 phys_begin;
> + __u64 size;
> +};

Don't we have a dma userspace api? What does virtio use? What about
uio? Why can't one of the existing interfaces work for you?

> +/* There is an ioctl associated with goldfish dma driver.
> + * Make it conflict with ioctls that are not likely to be used
> + * in the emulator.
> + * 'G' 00-3F drivers/misc/sgi-gru/grulib.h conflict!
> + * 'G' 00-0F linux/gigaset_dev.h conflict!

Causing known conflicts is not wise.

thanks,

greg k-h

\
 
 \ /
  Last update: 2018-09-25 20:32    [W:0.104 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site