On Tue, Feb 04, 2020 at 11:30:11AM +0800, Jason Wang wrote: > On 2020/1/31 上午11:36, Tiwei Bie wrote: > > This patch introduces a vDPA based vhost backend. This > > backend is built on top of the same interface defined > > in virtio-vDPA and provides a generic vhost interface > > for userspace to accelerate the virtio devices in guest. > > > > This backend is implemented as a vDPA device driver on > > top of the same ops used in virtio-vDPA. It will create > > char device entry named vhost-vdpa/$vdpa_device_index > > for userspace to use. Userspace can use vhost ioctls on > > top of this char device to setup the backend. > > > > Signed-off-by: Tiwei Bie <tiwei.bie@xxxxxxxxx> > > --- > > This patch depends on below series: > > https://lkml.org/lkml/2020/1/16/353 > > > > Please note that _SET_MEM_TABLE isn't fully supported yet. > > Comments would be appreciated! > > > > Changes since last patch (https://lkml.org/lkml/2019/11/18/1068) > > - Switch to the vDPA bus; > > - Switch to vhost's own chardev; > > > > drivers/vhost/Kconfig | 12 + > > drivers/vhost/Makefile | 3 + > > drivers/vhost/vdpa.c | 705 +++++++++++++++++++++++++++++++ > > include/uapi/linux/vhost.h | 21 + > > include/uapi/linux/vhost_types.h | 8 + > > 5 files changed, 749 insertions(+) > > create mode 100644 drivers/vhost/vdpa.c > > > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > > index f21c45aa5e07..13e6a94d0243 100644 > > --- a/drivers/vhost/Kconfig > > +++ b/drivers/vhost/Kconfig > > @@ -34,6 +34,18 @@ config VHOST_VSOCK > > To compile this driver as a module, choose M here: the module will be called > > vhost_vsock. > > +config VHOST_VDPA > > + tristate "Vhost driver for vDPA based backend" > > + depends on EVENTFD && VDPA > > + select VHOST > > + default n > > + ---help--- > > + This kernel module can be loaded in host kernel to accelerate > > + guest virtio devices with the vDPA based backends. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called vhost_vdpa. > > + > > config VHOST > > tristate > > depends on VHOST_IOTLB > > diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile > > index df99756fbb26..a65e9f4a2c0a 100644 > > --- a/drivers/vhost/Makefile > > +++ b/drivers/vhost/Makefile > > @@ -10,6 +10,9 @@ vhost_vsock-y := vsock.o > > obj-$(CONFIG_VHOST_RING) += vringh.o > > +obj-$(CONFIG_VHOST_VDPA) += vhost_vdpa.o > > +vhost_vdpa-y := vdpa.o > > + > > obj-$(CONFIG_VHOST) += vhost.o > > obj-$(CONFIG_VHOST_IOTLB) += vhost_iotlb.o > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > new file mode 100644 > > index 000000000000..631d994d37ac > > --- /dev/null > > +++ b/drivers/vhost/vdpa.c > > @@ -0,0 +1,705 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2018-2020 Intel Corporation. > > + * > > + * Author: Tiwei Bie <tiwei.bie@xxxxxxxxx> > > + * > > + * Thanks to Jason Wang and Michael S. Tsirkin for the valuable > > + * comments and suggestions. And thanks to Cunming Liang and > > + * Zhihong Wang for all their supports. > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/cdev.h> > > +#include <linux/device.h> > > +#include <linux/uuid.h> > > +#include <linux/vdpa.h> > > +#include <linux/nospec.h> > > +#include <linux/vhost.h> > > +#include <linux/virtio_net.h> > > + > > +#include "vhost.h" > > + > > +enum { > > + VHOST_VDPA_FEATURES = > > + (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) | > > + (1ULL << VIRTIO_F_ANY_LAYOUT) | > > + (1ULL << VIRTIO_F_VERSION_1) | > > + (1ULL << VIRTIO_F_IOMMU_PLATFORM) | > > + (1ULL << VIRTIO_F_RING_PACKED) | > > + (1ULL << VIRTIO_F_ORDER_PLATFORM) | > > + (1ULL << VIRTIO_RING_F_INDIRECT_DESC) | > > + (1ULL << VIRTIO_RING_F_EVENT_IDX), > > + > > + VHOST_VDPA_NET_FEATURES = VHOST_VDPA_FEATURES | > > + (1ULL << VIRTIO_NET_F_CSUM) | > > + (1ULL << VIRTIO_NET_F_GUEST_CSUM) | > > + (1ULL << VIRTIO_NET_F_MTU) | > > + (1ULL << VIRTIO_NET_F_MAC) | > > + (1ULL << VIRTIO_NET_F_GUEST_TSO4) | > > + (1ULL << VIRTIO_NET_F_GUEST_TSO6) | > > + (1ULL << VIRTIO_NET_F_GUEST_ECN) | > > + (1ULL << VIRTIO_NET_F_GUEST_UFO) | > > + (1ULL << VIRTIO_NET_F_HOST_TSO4) | > > + (1ULL << VIRTIO_NET_F_HOST_TSO6) | > > + (1ULL << VIRTIO_NET_F_HOST_ECN) | > > + (1ULL << VIRTIO_NET_F_HOST_UFO) | > > + (1ULL << VIRTIO_NET_F_MRG_RXBUF) | > > + (1ULL << VIRTIO_NET_F_STATUS) | > > + (1ULL << VIRTIO_NET_F_SPEED_DUPLEX), > > +}; > > + > > +/* Currently, only network backend w/o multiqueue is supported. */ > > +#define VHOST_VDPA_VQ_MAX 2 > > + > > +struct vhost_vdpa { > > + /* The lock is to protect this structure. */ > > + struct mutex mutex; > > + struct vhost_dev vdev; > > + struct vhost_virtqueue *vqs; > > + struct vdpa_device *vdpa; > > + struct device *dev; > > + atomic_t opened; > > + int nvqs; > > + int virtio_id; > > + int minor; > > +}; > > + > > +static struct { > > + /* The lock is to protect this structure. */ > > + struct mutex mutex; > > + struct class *class; > > + struct idr idr; > > + struct cdev cdev; > > + dev_t devt; > > + wait_queue_head_t release_q; > > +} vhost_vdpa; > > + > > +static const u64 vhost_vdpa_features[] = { > > + [VIRTIO_ID_NET] = VHOST_VDPA_NET_FEATURES, > > +}; > > + > > +static int vhost_vdpa_alloc_minor(struct vhost_vdpa *v) > > +{ > > + return idr_alloc(&vhost_vdpa.idr, v, 0, MINORMASK + 1, > > + GFP_KERNEL); > > +} > > + > > +static void vhost_vdpa_free_minor(int minor) > > +{ > > + idr_remove(&vhost_vdpa.idr, minor); > > +} > > + > > +static struct vhost_vdpa *vhost_vdpa_get_from_minor(int minor) > > +{ > > + struct vhost_vdpa *v; > > + > > + mutex_lock(&vhost_vdpa.mutex); > > + v = idr_find(&vhost_vdpa.idr, minor); > > + mutex_unlock(&vhost_vdpa.mutex); > > + > > + return v; > > +} > > + > > +static void handle_vq_kick(struct vhost_work *work) > > +{ > > + struct vhost_virtqueue *vq = container_of(work, struct vhost_virtqueue, > > + poll.work); > > + struct vhost_vdpa *v = container_of(vq->dev, struct vhost_vdpa, vdev); > > + const struct vdpa_config_ops *ops = v->vdpa->config; > > + > > + ops->kick_vq(v->vdpa, vq - v->vqs); > > +} > > + > > +static irqreturn_t vhost_vdpa_virtqueue_cb(void *private) > > +{ > > + struct vhost_virtqueue *vq = private; > > + struct eventfd_ctx *call_ctx = vq->call_ctx; > > + > > + if (call_ctx) > > + eventfd_signal(call_ctx, 1); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static void vhost_vdpa_reset(struct vhost_vdpa *v) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + > > + ops->set_status(vdpa, 0); > > +} > > + > > +static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user *argp) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u32 device_id; > > + > > + device_id = ops->get_device_id(vdpa); > > + > > + if (copy_to_user(argp, &device_id, sizeof(device_id))) > > + return -EFAULT; > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_get_status(struct vhost_vdpa *v, u8 __user *statusp) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u8 status; > > + > > + status = ops->get_status(vdpa); > > + > > + if (copy_to_user(statusp, &status, sizeof(status))) > > + return -EFAULT; > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_set_status(struct vhost_vdpa *v, u8 __user *statusp) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u8 status; > > + > > + if (copy_from_user(&status, statusp, sizeof(status))) > > + return -EFAULT; > > + > > + /* > > + * Userspace shouldn't remove status bits unless reset the > > + * status to 0. > > + */ > > + if (status != 0 && (ops->get_status(vdpa) & ~status) != 0) > > + return -EINVAL; > > + > > + ops->set_status(vdpa, status); > > + > > + return 0; > > +} > > + > > +static int vhost_vdpa_config_validate(struct vhost_vdpa *v, > > + struct vhost_vdpa_config *c) > > +{ > > + long size = 0; > > + > > + switch (v->virtio_id) { > > + case VIRTIO_ID_NET: > > + size = sizeof(struct virtio_net_config); > > + break; > > + } > > + > > + if (c->len == 0) > > + return -EINVAL; > > + > > + if (c->len > size - c->off) > > + return -E2BIG; > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_get_config(struct vhost_vdpa *v, > > + struct vhost_vdpa_config __user *c) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + struct vhost_vdpa_config config; > > + unsigned long size = offsetof(struct vhost_vdpa_config, buf); > > + u8 *buf; > > + > > + if (copy_from_user(&config, c, size)) > > + return -EFAULT; > > + if (vhost_vdpa_config_validate(v, &config)) > > + return -EINVAL; > > + buf = kvzalloc(config.len, GFP_KERNEL); > > + if (!buf) > > + return -ENOMEM; > > + > > + ops->get_config(vdpa, config.off, buf, config.len); > > + > > + if (copy_to_user(c->buf, buf, config.len)) { > > + kvfree(buf); > > + return -EFAULT; > > + } > > + > > + kvfree(buf); > > + return 0; > > +} > > + > > +static long vhost_vdpa_set_config(struct vhost_vdpa *v, > > + struct vhost_vdpa_config __user *c) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + struct vhost_vdpa_config config; > > + unsigned long size = offsetof(struct vhost_vdpa_config, buf); > > + u8 *buf; > > + > > + if (copy_from_user(&config, c, size)) > > + return -EFAULT; > > + if (vhost_vdpa_config_validate(v, &config)) > > + return -EINVAL; > > + buf = kvzalloc(config.len, GFP_KERNEL); > > + if (!buf) > > + return -ENOMEM; > > + > > + if (copy_from_user(buf, c->buf, config.len)) { > > + kvfree(buf); > > + return -EFAULT; > > + } > > + > > + ops->set_config(vdpa, config.off, buf, config.len); > > + > > + kvfree(buf); > > + return 0; > > +} > > + > > +static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u64 features; > > + > > + features = ops->get_features(vdpa); > > + features &= vhost_vdpa_features[v->virtio_id]; > > + > > + if (copy_to_user(featurep, &features, sizeof(features))) > > + return -EFAULT; > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_set_features(struct vhost_vdpa *v, u64 __user *featurep) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u64 features; > > + > > + /* > > + * It's not allowed to change the features after they have > > + * been negotiated. > > + */ > > + if (ops->get_status(vdpa) & VIRTIO_CONFIG_S_FEATURES_OK) > > + return -EBUSY; > > + > > + if (copy_from_user(&features, featurep, sizeof(features))) > > + return -EFAULT; > > + > > + if (features & ~vhost_vdpa_features[v->virtio_id]) > > + return -EINVAL; > > + > > + if (ops->set_features(vdpa, features)) > > + return -EINVAL; > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + u16 num; > > + > > + num = ops->get_vq_num_max(vdpa); > > + > > + if (copy_to_user(argp, &num, sizeof(num))) > > + return -EFAULT; > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd, > > + void __user *argp) > > +{ > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + struct vdpa_callback cb; > > + struct vhost_virtqueue *vq; > > + struct vhost_vring_state s; > > + u8 status; > > + u32 idx; > > + long r; > > + > > + r = get_user(idx, (u32 __user *)argp); > > + if (r < 0) > > + return r; > > + if (idx >= v->nvqs) > > + return -ENOBUFS; > > + > > + idx = array_index_nospec(idx, v->nvqs); > > + vq = &v->vqs[idx]; > > + > > + status = ops->get_status(vdpa); > > + > > + /* > > + * It's not allowed to detect and program vqs before > > + * features negotiation or after enabling driver. > > + */ > > + if (!(status & VIRTIO_CONFIG_S_FEATURES_OK) || > > + (status & VIRTIO_CONFIG_S_DRIVER_OK)) > > + return -EBUSY; > > + > > + if (cmd == VHOST_VDPA_SET_VRING_ENABLE) { > > + if (copy_from_user(&s, argp, sizeof(s))) > > + return -EFAULT; > > + ops->set_vq_ready(vdpa, idx, s.num); > > + return 0; > > + } > > + > > + /* > > + * It's not allowed to detect and program vqs with > > + * vqs enabled. > > + */ > > + if (ops->get_vq_ready(vdpa, idx)) > > + return -EBUSY; > > + > > + if (cmd == VHOST_GET_VRING_BASE) > > + vq->last_avail_idx = ops->get_vq_state(v->vdpa, idx); > > + > > + r = vhost_vring_ioctl(&v->vdev, cmd, argp); > > + if (r) > > + return r; > > + > > + switch (cmd) { > > + case VHOST_SET_VRING_ADDR: > > + if (ops->set_vq_address(vdpa, idx, (u64)vq->desc, > > + (u64)vq->avail, (u64)vq->used)) > > + r = -EINVAL; > > + break; > > + > > + case VHOST_SET_VRING_BASE: > > + if (ops->set_vq_state(vdpa, idx, vq->last_avail_idx)) > > + r = -EINVAL; > > + break; > > + > > + case VHOST_SET_VRING_CALL: > > + if (vq->call_ctx) { > > + cb.callback = vhost_vdpa_virtqueue_cb; > > + cb.private = vq; > > + } else { > > + cb.callback = NULL; > > + cb.private = NULL; > > + } > > + ops->set_vq_cb(vdpa, idx, &cb); > > + break; > > + > > + case VHOST_SET_VRING_NUM: > > + ops->set_vq_num(vdpa, idx, vq->num); > > + break; > > + } > > + > > + return r; > > +} > > + > > +static long vhost_vdpa_do_dma_mapping(struct vhost_vdpa *v) > > +{ > > + /* TODO: fix this */ > > > Before trying to do this it looks to me we need the following during the > probe > > 1) if set_map() is not supported by the vDPA device probe the IOMMU that is > supported by the vDPA device > 2) allocate IOMMU domain > > And then: > > 3) pin pages through GUP and do proper accounting > 4) store GPA->HPA mapping in the umem > 5) generate diffs of memory table and using IOMMU API to setup the dma > mapping in this method > > For 1), I'm not sure parent is sufficient for to doing this or need to > introduce new API like iommu_device in mdev. Agree. We may also need to introduce something like the iommu_device. > > > > + return -ENOTSUPP; > > +} > > + > > +static int vhost_vdpa_open(struct inode *inode, struct file *filep) > > +{ > > + struct vhost_vdpa *v; > > + struct vhost_dev *dev; > > + struct vhost_virtqueue **vqs; > > + int nvqs, i, r, opened; > > + > > + v = vhost_vdpa_get_from_minor(iminor(inode)); > > + if (!v) > > + return -ENODEV; > > + > > + opened = atomic_cmpxchg(&v->opened, 0, 1); > > + if (opened) { > > + r = -EBUSY; > > + goto err; > > + } > > + > > + nvqs = v->nvqs; > > + vhost_vdpa_reset(v); > > + > > + vqs = kmalloc_array(nvqs, sizeof(*vqs), GFP_KERNEL); > > + if (!vqs) { > > + r = -ENOMEM; > > + goto err; > > + } > > + > > + dev = &v->vdev; > > + for (i = 0; i < nvqs; i++) { > > + vqs[i] = &v->vqs[i]; > > + vqs[i]->handle_kick = handle_vq_kick; > > + } > > + vhost_dev_init(dev, vqs, nvqs, 0, 0, 0); > > + > > + filep->private_data = v; > > + > > + return 0; > > + > > +err: > > + atomic_dec(&v->opened); > > + return r; > > +} > > + > > +static int vhost_vdpa_release(struct inode *inode, struct file *filep) > > +{ > > + struct vhost_vdpa *v = filep->private_data; > > + > > + mutex_lock(&v->mutex); > > + filep->private_data = NULL; > > + vhost_vdpa_reset(v); > > + vhost_dev_stop(&v->vdev); > > + vhost_dev_cleanup(&v->vdev); > > + kfree(v->vdev.vqs); > > + mutex_unlock(&v->mutex); > > + > > + atomic_dec(&v->opened); > > + wake_up(&vhost_vdpa.release_q); > > + > > + return 0; > > +} > > + > > +static long vhost_vdpa_unlocked_ioctl(struct file *filep, > > + unsigned int cmd, unsigned long arg) > > +{ > > + struct vhost_vdpa *v = filep->private_data; > > + struct vdpa_device *vdpa = v->vdpa; > > + const struct vdpa_config_ops *ops = vdpa->config; > > + void __user *argp = (void __user *)arg; > > + long r; > > + > > + mutex_lock(&v->mutex); > > + > > + switch (cmd) { > > + case VHOST_VDPA_GET_DEVICE_ID: > > + r = vhost_vdpa_get_device_id(v, argp); > > + break; > > + case VHOST_VDPA_GET_STATUS: > > + r = vhost_vdpa_get_status(v, argp); > > + break; > > + case VHOST_VDPA_SET_STATUS: > > + r = vhost_vdpa_set_status(v, argp); > > + break; > > + case VHOST_VDPA_GET_CONFIG: > > + r = vhost_vdpa_get_config(v, argp); > > + break; > > + case VHOST_VDPA_SET_CONFIG: > > + r = vhost_vdpa_set_config(v, argp); > > + break; > > + case VHOST_GET_FEATURES: > > + r = vhost_vdpa_get_features(v, argp); > > + break; > > + case VHOST_SET_FEATURES: > > + r = vhost_vdpa_set_features(v, argp); > > + break; > > + case VHOST_VDPA_GET_VRING_NUM: > > + r = vhost_vdpa_get_vring_num(v, argp); > > + break; > > + case VHOST_SET_LOG_BASE: > > + case VHOST_SET_LOG_FD: > > + r = -ENOIOCTLCMD; > > + break; > > + default: > > + mutex_lock(&v->vdev.mutex); > > + r = vhost_dev_ioctl(&v->vdev, cmd, argp); > > + if (r == -ENOIOCTLCMD) > > + r = vhost_vdpa_vring_ioctl(v, cmd, argp); > > + mutex_unlock(&v->vdev.mutex); > > + break; > > + } > > + > > + if (r) > > + goto out; > > + > > + switch (cmd) { > > + case VHOST_SET_MEM_TABLE: > > + if (ops->set_map) > > + r = ops->set_map(vdpa, v->vdev.umem); > > > I think we need do the following to make sure set_map() works: > > 1) pin pages through GUP and do proper accounting for e.g ulimit > 2) using GPA->HPA mapping for dev->umem > 3) then there's no need to deal VHOST_SET_MEM_TABLE in vhost_dev_ioctl(), or > add some specific callbacks in vhost_set_memory(). OK. Thanks! Tiwei > > Thanks > >
