| Message ID | 20191017104836.32464-5-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mdev based hardware virtio offloading support | expand |
On Thu, 17 Oct 2019 18:48:34 +0800 Jason Wang <jasowang@redhat.com> wrote: > This patch implements basic support for mdev driver that supports > virtio transport for kernel virtio driver. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vfio/mdev/mdev_core.c | 12 +++ > include/linux/mdev.h | 4 + > include/linux/virtio_mdev.h | 151 ++++++++++++++++++++++++++++++++++ > 3 files changed, 167 insertions(+) > create mode 100644 include/linux/virtio_mdev.h > > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c > index d0f3113c8071..5834f6b7c7a5 100644 > --- a/drivers/vfio/mdev/mdev_core.c > +++ b/drivers/vfio/mdev/mdev_core.c > @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, > } > EXPORT_SYMBOL(mdev_set_vfio_ops); > > +/* Specify the virtio device ops for the mdev device, this > + * must be called during create() callback for virtio mdev device. > + */ > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops) > +{ > + BUG_ON(mdev->class_id); Nit, this one is a BUG_ON, but the vfio one is a WARN_ON. Thanks, Alex > + mdev->class_id = MDEV_CLASS_ID_VIRTIO; > + mdev->device_ops = virtio_ops; > +} > +EXPORT_SYMBOL(mdev_set_virtio_ops); > + > const void *mdev_get_dev_ops(struct mdev_device *mdev) > { > return mdev->device_ops; > diff --git a/include/linux/mdev.h b/include/linux/mdev.h > index 3d29e09e20c9..13e045e09d3b 100644 > --- a/include/linux/mdev.h > +++ b/include/linux/mdev.h > @@ -17,6 +17,7 @@ > > struct mdev_device; > struct vfio_mdev_device_ops; > +struct virtio_mdev_device_ops; > > /* > * Called by the parent device driver to set the device which represents > @@ -111,6 +112,8 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data); > const guid_t *mdev_uuid(struct mdev_device *mdev); > void mdev_set_vfio_ops(struct mdev_device *mdev, > const struct vfio_mdev_device_ops *vfio_ops); > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops); > const void *mdev_get_dev_ops(struct mdev_device *mdev); > > extern struct bus_type mdev_bus_type; > @@ -127,6 +130,7 @@ struct mdev_device *mdev_from_dev(struct device *dev); > > enum { > MDEV_CLASS_ID_VFIO = 1, > + MDEV_CLASS_ID_VIRTIO = 2, > /* New entries must be added here */ > }; > > diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h > new file mode 100644 > index 000000000000..b965b50f9b24 > --- /dev/null > +++ b/include/linux/virtio_mdev.h > @@ -0,0 +1,151 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Virtio mediated device driver > + * > + * Copyright 2019, Red Hat Corp. > + * Author: Jason Wang <jasowang@redhat.com> > + */ > +#ifndef _LINUX_VIRTIO_MDEV_H > +#define _LINUX_VIRTIO_MDEV_H > + > +#include <linux/interrupt.h> > +#include <linux/mdev.h> > +#include <uapi/linux/vhost.h> > + > +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev" > +#define VIRTIO_MDEV_F_VERSION_1 0x1 > + > +struct virtio_mdev_callback { > + irqreturn_t (*callback)(void *data); > + void *private; > +}; > + > +/** > + * struct vfio_mdev_device_ops - Structure to be registered for each > + * mdev device to register the device to virtio-mdev module. > + * > + * @set_vq_address: Set the address of virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @desc_area: address of desc area > + * @driver_area: address of driver area > + * @device_area: address of device area > + * Returns integer: success (0) or error (< 0) > + * @set_vq_num: Set the size of virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @num: the size of virtqueue > + * @kick_vq: Kick the virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @set_vq_cb: Set the interrupt callback function for > + * a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @cb: virtio-mdev interrupt callback structure > + * @set_vq_ready: Set ready status for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @ready: ready (true) not ready(false) > + * @get_vq_ready: Get ready status for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * Returns boolean: ready (true) or not (false) > + * @set_vq_state: Set the state for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @state: virtqueue state (last_avail_idx) > + * Returns integer: success (0) or error (< 0) > + * @get_vq_state: Get the state for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * Returns virtqueue state (last_avail_idx) > + * @get_vq_align: Get the virtqueue align requirement > + * for the device > + * @mdev: mediated device > + * Returns virtqueue algin requirement > + * @get_features: Get virtio features supported by the device > + * @mdev: mediated device > + * Returns the virtio features support by the > + * device > + * @get_features: Set virtio features supported by the driver > + * @mdev: mediated device > + * @features: feature support by the driver > + * Returns integer: success (0) or error (< 0) > + * @set_config_cb: Set the config interrupt callback > + * @mdev: mediated device > + * @cb: virtio-mdev interrupt callback structure > + * @get_vq_num_max: Get the max size of virtqueue > + * @mdev: mediated device > + * Returns u16: max size of virtqueue > + * @get_device_id: Get virtio device id > + * @mdev: mediated device > + * Returns u32: virtio device id > + * @get_vendor_id: Get virtio vendor id > + * @mdev: mediated device > + * Returns u32: virtio vendor id > + * @get_status: Get the device status > + * @mdev: mediated device > + * Returns u8: virtio device status > + * @set_status: Set the device status > + * @mdev: mediated device > + * @status: virtio device status > + * @get_config: Read from device specific configuration space > + * @mdev: mediated device > + * @offset: offset from the beginning of > + * configuration space > + * @buf: buffer used to read to > + * @len: the length to read from > + * configration space > + * @set_config: Write to device specific configuration space > + * @mdev: mediated device > + * @offset: offset from the beginning of > + * configuration space > + * @buf: buffer used to write from > + * @len: the length to write to > + * configration space > + * @get_mdev_features: Get the feature of virtio mdev device > + * @mdev: mediated device > + * Returns the mdev features (API) support by > + * the device. > + * @get_generation: Get device generaton > + * @mdev: mediated device > + * Returns u32: device generation > + */ > +struct virtio_mdev_device_ops { > + /* Virtqueue ops */ > + int (*set_vq_address)(struct mdev_device *mdev, > + u16 idx, u64 desc_area, u64 driver_area, > + u64 device_area); > + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num); > + void (*kick_vq)(struct mdev_device *mdev, u16 idx); > + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx, > + struct virtio_mdev_callback *cb); > + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready); > + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx); > + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state); > + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx); > + > + /* Device ops */ > + u16 (*get_vq_align)(struct mdev_device *mdev); > + u64 (*get_features)(struct mdev_device *mdev); > + int (*set_features)(struct mdev_device *mdev, u64 features); > + void (*set_config_cb)(struct mdev_device *mdev, > + struct virtio_mdev_callback *cb); > + u16 (*get_vq_num_max)(struct mdev_device *mdev); > + u32 (*get_device_id)(struct mdev_device *mdev); > + u32 (*get_vendor_id)(struct mdev_device *mdev); > + u8 (*get_status)(struct mdev_device *mdev); > + void (*set_status)(struct mdev_device *mdev, u8 status); > + void (*get_config)(struct mdev_device *mdev, unsigned int offset, > + void *buf, unsigned int len); > + void (*set_config)(struct mdev_device *mdev, unsigned int offset, > + const void *buf, unsigned int len); > + u64 (*get_mdev_features)(struct mdev_device *mdev); > + u32 (*get_generation)(struct mdev_device *mdev); > +}; > + > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops); > + > +#endif
On 2019/10/18 上午1:53, Alex Williamson wrote: > On Thu, 17 Oct 2019 18:48:34 +0800 > Jason Wang<jasowang@redhat.com> wrote: > >> This patch implements basic support for mdev driver that supports >> virtio transport for kernel virtio driver. >> >> Signed-off-by: Jason Wang<jasowang@redhat.com> >> --- >> drivers/vfio/mdev/mdev_core.c | 12 +++ >> include/linux/mdev.h | 4 + >> include/linux/virtio_mdev.h | 151 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 167 insertions(+) >> create mode 100644 include/linux/virtio_mdev.h >> >> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c >> index d0f3113c8071..5834f6b7c7a5 100644 >> --- a/drivers/vfio/mdev/mdev_core.c >> +++ b/drivers/vfio/mdev/mdev_core.c >> @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, >> } >> EXPORT_SYMBOL(mdev_set_vfio_ops); >> >> +/* Specify the virtio device ops for the mdev device, this >> + * must be called during create() callback for virtio mdev device. >> + */ >> +void mdev_set_virtio_ops(struct mdev_device *mdev, >> + const struct virtio_mdev_device_ops *virtio_ops) >> +{ >> + BUG_ON(mdev->class_id); > Nit, this one is a BUG_ON, but the vfio one is a WARN_ON. Thanks, > > Alex Let me fix in next version. Thanks
On Thu, 17 Oct 2019 18:48:34 +0800 Jason Wang <jasowang@redhat.com> wrote: > This patch implements basic support for mdev driver that supports > virtio transport for kernel virtio driver. > > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > drivers/vfio/mdev/mdev_core.c | 12 +++ > include/linux/mdev.h | 4 + > include/linux/virtio_mdev.h | 151 ++++++++++++++++++++++++++++++++++ > 3 files changed, 167 insertions(+) > create mode 100644 include/linux/virtio_mdev.h > > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c > index d0f3113c8071..5834f6b7c7a5 100644 > --- a/drivers/vfio/mdev/mdev_core.c > +++ b/drivers/vfio/mdev/mdev_core.c > @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, > } > EXPORT_SYMBOL(mdev_set_vfio_ops); > > +/* Specify the virtio device ops for the mdev device, this > + * must be called during create() callback for virtio mdev device. > + */ Change this as for the vfio comment (last patch?) > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops) > +{ > + BUG_ON(mdev->class_id); s/BUG_ON/WARN_ON/ > + mdev->class_id = MDEV_CLASS_ID_VIRTIO; > + mdev->device_ops = virtio_ops; > +} > +EXPORT_SYMBOL(mdev_set_virtio_ops); > + > const void *mdev_get_dev_ops(struct mdev_device *mdev) > { > return mdev->device_ops; (...) > diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h > new file mode 100644 > index 000000000000..b965b50f9b24 > --- /dev/null > +++ b/include/linux/virtio_mdev.h > @@ -0,0 +1,151 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Virtio mediated device driver > + * > + * Copyright 2019, Red Hat Corp. > + * Author: Jason Wang <jasowang@redhat.com> > + */ > +#ifndef _LINUX_VIRTIO_MDEV_H > +#define _LINUX_VIRTIO_MDEV_H > + > +#include <linux/interrupt.h> > +#include <linux/mdev.h> > +#include <uapi/linux/vhost.h> > + > +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev" > +#define VIRTIO_MDEV_F_VERSION_1 0x1 > + > +struct virtio_mdev_callback { > + irqreturn_t (*callback)(void *data); > + void *private; > +}; > + > +/** > + * struct vfio_mdev_device_ops - Structure to be registered for each > + * mdev device to register the device to virtio-mdev module. > + * > + * @set_vq_address: Set the address of virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @desc_area: address of desc area > + * @driver_area: address of driver area > + * @device_area: address of device area > + * Returns integer: success (0) or error (< 0) Probably dumb question (have not read the next patches yet): Is this agnostic regarding split or packed queues? > + * @set_vq_num: Set the size of virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @num: the size of virtqueue > + * @kick_vq: Kick the virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @set_vq_cb: Set the interrupt callback function for > + * a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @cb: virtio-mdev interrupt callback structure > + * @set_vq_ready: Set ready status for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @ready: ready (true) not ready(false) > + * @get_vq_ready: Get ready status for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * Returns boolean: ready (true) or not (false) > + * @set_vq_state: Set the state for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * @state: virtqueue state (last_avail_idx) > + * Returns integer: success (0) or error (< 0) > + * @get_vq_state: Get the state for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * Returns virtqueue state (last_avail_idx) > + * @get_vq_align: Get the virtqueue align requirement > + * for the device > + * @mdev: mediated device > + * Returns virtqueue algin requirement > + * @get_features: Get virtio features supported by the device > + * @mdev: mediated device > + * Returns the virtio features support by the > + * device > + * @get_features: Set virtio features supported by the driver s/get_features/set_features/ > + * @mdev: mediated device > + * @features: feature support by the driver > + * Returns integer: success (0) or error (< 0) > + * @set_config_cb: Set the config interrupt callback > + * @mdev: mediated device > + * @cb: virtio-mdev interrupt callback structure > + * @get_vq_num_max: Get the max size of virtqueue > + * @mdev: mediated device > + * Returns u16: max size of virtqueue > + * @get_device_id: Get virtio device id > + * @mdev: mediated device > + * Returns u32: virtio device id > + * @get_vendor_id: Get virtio vendor id > + * @mdev: mediated device > + * Returns u32: virtio vendor id How is the vendor id defined? As for normal virtio-pci devices? > + * @get_status: Get the device status > + * @mdev: mediated device > + * Returns u8: virtio device status > + * @set_status: Set the device status > + * @mdev: mediated device > + * @status: virtio device status > + * @get_config: Read from device specific configuration space > + * @mdev: mediated device > + * @offset: offset from the beginning of > + * configuration space > + * @buf: buffer used to read to > + * @len: the length to read from > + * configration space > + * @set_config: Write to device specific configuration space > + * @mdev: mediated device > + * @offset: offset from the beginning of > + * configuration space > + * @buf: buffer used to write from > + * @len: the length to write to > + * configration space > + * @get_mdev_features: Get the feature of virtio mdev device > + * @mdev: mediated device > + * Returns the mdev features (API) support by > + * the device. What kind of 'features' are supposed to go in there? Are these bits, like you defined for VIRTIO_MDEV_F_VERSION_1 above? > + * @get_generation: Get device generaton > + * @mdev: mediated device > + * Returns u32: device generation Is that callback mandatory? > + */ > +struct virtio_mdev_device_ops { > + /* Virtqueue ops */ > + int (*set_vq_address)(struct mdev_device *mdev, > + u16 idx, u64 desc_area, u64 driver_area, > + u64 device_area); > + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num); > + void (*kick_vq)(struct mdev_device *mdev, u16 idx); > + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx, > + struct virtio_mdev_callback *cb); > + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready); > + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx); > + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state); > + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx); > + > + /* Device ops */ > + u16 (*get_vq_align)(struct mdev_device *mdev); > + u64 (*get_features)(struct mdev_device *mdev); > + int (*set_features)(struct mdev_device *mdev, u64 features); > + void (*set_config_cb)(struct mdev_device *mdev, > + struct virtio_mdev_callback *cb); > + u16 (*get_vq_num_max)(struct mdev_device *mdev); > + u32 (*get_device_id)(struct mdev_device *mdev); > + u32 (*get_vendor_id)(struct mdev_device *mdev); > + u8 (*get_status)(struct mdev_device *mdev); > + void (*set_status)(struct mdev_device *mdev, u8 status); > + void (*get_config)(struct mdev_device *mdev, unsigned int offset, > + void *buf, unsigned int len); > + void (*set_config)(struct mdev_device *mdev, unsigned int offset, > + const void *buf, unsigned int len); > + u64 (*get_mdev_features)(struct mdev_device *mdev); > + u32 (*get_generation)(struct mdev_device *mdev); > +}; > + > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops); > + > +#endif
On Thu, Oct 17, 2019 at 06:48:34PM +0800, Jason Wang wrote: > + * @get_vq_state: Get the state for a virtqueue > + * @mdev: mediated device > + * @idx: virtqueue index > + * Returns virtqueue state (last_avail_idx) > + * @get_vq_align: Get the virtqueue align requirement > + * for the device > + * @mdev: mediated device > + * Returns virtqueue algin requirement > + * @get_features: Get virtio features supported by the device > + * @mdev: mediated device > + * Returns the virtio features support by the > + * device > + * @get_features: Set virtio features supported by the driver s/get_features/set_features/ > + * configration space > + * @get_mdev_features: Get the feature of virtio mdev device > + * @mdev: mediated device > + * Returns the mdev features (API) support by > + * the device. > + * @get_generation: Get device generaton > + * @mdev: mediated device > + * Returns u32: device generation > + */ > +struct virtio_mdev_device_ops { > + /* Virtqueue ops */ > + int (*set_vq_address)(struct mdev_device *mdev, > + u16 idx, u64 desc_area, u64 driver_area, > + u64 device_area); > + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num); > + void (*kick_vq)(struct mdev_device *mdev, u16 idx); > + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx, > + struct virtio_mdev_callback *cb); > + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready); > + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx); > + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state); > + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx); > + > + /* Device ops */ > + u16 (*get_vq_align)(struct mdev_device *mdev); > + u64 (*get_features)(struct mdev_device *mdev); > + int (*set_features)(struct mdev_device *mdev, u64 features); > + void (*set_config_cb)(struct mdev_device *mdev, > + struct virtio_mdev_callback *cb); > + u16 (*get_vq_num_max)(struct mdev_device *mdev); > + u32 (*get_device_id)(struct mdev_device *mdev); > + u32 (*get_vendor_id)(struct mdev_device *mdev); > + u8 (*get_status)(struct mdev_device *mdev); > + void (*set_status)(struct mdev_device *mdev, u8 status); > + void (*get_config)(struct mdev_device *mdev, unsigned int offset, > + void *buf, unsigned int len); > + void (*set_config)(struct mdev_device *mdev, unsigned int offset, > + const void *buf, unsigned int len); > + u64 (*get_mdev_features)(struct mdev_device *mdev); Do we need a .set_mdev_features method as well? It's not very clear what does mdev_features mean. Does it mean the vhost backend features? https://github.com/torvalds/linux/blob/0e2adab6cf285c41e825b6c74a3aa61324d1132c/include/uapi/linux/vhost.h#L93-L94 > + u32 (*get_generation)(struct mdev_device *mdev); > +}; > + > +void mdev_set_virtio_ops(struct mdev_device *mdev, > + const struct virtio_mdev_device_ops *virtio_ops); > + > +#endif > -- > 2.19.1 >
On 2019/10/18 下午5:46, Cornelia Huck wrote: > On Thu, 17 Oct 2019 18:48:34 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> This patch implements basic support for mdev driver that supports >> virtio transport for kernel virtio driver. >> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> drivers/vfio/mdev/mdev_core.c | 12 +++ >> include/linux/mdev.h | 4 + >> include/linux/virtio_mdev.h | 151 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 167 insertions(+) >> create mode 100644 include/linux/virtio_mdev.h >> >> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c >> index d0f3113c8071..5834f6b7c7a5 100644 >> --- a/drivers/vfio/mdev/mdev_core.c >> +++ b/drivers/vfio/mdev/mdev_core.c >> @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, >> } >> EXPORT_SYMBOL(mdev_set_vfio_ops); >> >> +/* Specify the virtio device ops for the mdev device, this >> + * must be called during create() callback for virtio mdev device. >> + */ > Change this as for the vfio comment (last patch?) Ok. > >> +void mdev_set_virtio_ops(struct mdev_device *mdev, >> + const struct virtio_mdev_device_ops *virtio_ops) >> +{ >> + BUG_ON(mdev->class_id); > s/BUG_ON/WARN_ON/ Yes. > >> + mdev->class_id = MDEV_CLASS_ID_VIRTIO; >> + mdev->device_ops = virtio_ops; >> +} >> +EXPORT_SYMBOL(mdev_set_virtio_ops); >> + >> const void *mdev_get_dev_ops(struct mdev_device *mdev) >> { >> return mdev->device_ops; > (...) > >> diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h >> new file mode 100644 >> index 000000000000..b965b50f9b24 >> --- /dev/null >> +++ b/include/linux/virtio_mdev.h >> @@ -0,0 +1,151 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Virtio mediated device driver >> + * >> + * Copyright 2019, Red Hat Corp. >> + * Author: Jason Wang <jasowang@redhat.com> >> + */ >> +#ifndef _LINUX_VIRTIO_MDEV_H >> +#define _LINUX_VIRTIO_MDEV_H >> + >> +#include <linux/interrupt.h> >> +#include <linux/mdev.h> >> +#include <uapi/linux/vhost.h> >> + >> +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev" >> +#define VIRTIO_MDEV_F_VERSION_1 0x1 >> + >> +struct virtio_mdev_callback { >> + irqreturn_t (*callback)(void *data); >> + void *private; >> +}; >> + >> +/** >> + * struct vfio_mdev_device_ops - Structure to be registered for each >> + * mdev device to register the device to virtio-mdev module. >> + * >> + * @set_vq_address: Set the address of virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * @desc_area: address of desc area >> + * @driver_area: address of driver area >> + * @device_area: address of device area >> + * Returns integer: success (0) or error (< 0) > Probably dumb question (have not read the next patches yet): Is this > agnostic regarding split or packed queues? Yes, it is to and to be more obvious, I use the terminology from recent spec. > >> + * @set_vq_num: Set the size of virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * @num: the size of virtqueue >> + * @kick_vq: Kick the virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * @set_vq_cb: Set the interrupt callback function for >> + * a virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * @cb: virtio-mdev interrupt callback structure >> + * @set_vq_ready: Set ready status for a virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * @ready: ready (true) not ready(false) >> + * @get_vq_ready: Get ready status for a virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * Returns boolean: ready (true) or not (false) >> + * @set_vq_state: Set the state for a virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * @state: virtqueue state (last_avail_idx) >> + * Returns integer: success (0) or error (< 0) >> + * @get_vq_state: Get the state for a virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * Returns virtqueue state (last_avail_idx) >> + * @get_vq_align: Get the virtqueue align requirement >> + * for the device >> + * @mdev: mediated device >> + * Returns virtqueue algin requirement >> + * @get_features: Get virtio features supported by the device >> + * @mdev: mediated device >> + * Returns the virtio features support by the >> + * device >> + * @get_features: Set virtio features supported by the driver > s/get_features/set_features/ Will fix. > >> + * @mdev: mediated device >> + * @features: feature support by the driver >> + * Returns integer: success (0) or error (< 0) >> + * @set_config_cb: Set the config interrupt callback >> + * @mdev: mediated device >> + * @cb: virtio-mdev interrupt callback structure >> + * @get_vq_num_max: Get the max size of virtqueue >> + * @mdev: mediated device >> + * Returns u16: max size of virtqueue >> + * @get_device_id: Get virtio device id >> + * @mdev: mediated device >> + * Returns u32: virtio device id >> + * @get_vendor_id: Get virtio vendor id >> + * @mdev: mediated device >> + * Returns u32: virtio vendor id > How is the vendor id defined? As for normal virtio-pci devices? The vendor that provides this device. So something like this I notice that MMIO also had this so it looks to me it's not pci specific. > >> + * @get_status: Get the device status >> + * @mdev: mediated device >> + * Returns u8: virtio device status >> + * @set_status: Set the device status >> + * @mdev: mediated device >> + * @status: virtio device status >> + * @get_config: Read from device specific configuration space >> + * @mdev: mediated device >> + * @offset: offset from the beginning of >> + * configuration space >> + * @buf: buffer used to read to >> + * @len: the length to read from >> + * configration space >> + * @set_config: Write to device specific configuration space >> + * @mdev: mediated device >> + * @offset: offset from the beginning of >> + * configuration space >> + * @buf: buffer used to write from >> + * @len: the length to write to >> + * configration space >> + * @get_mdev_features: Get the feature of virtio mdev device >> + * @mdev: mediated device >> + * Returns the mdev features (API) support by >> + * the device. > What kind of 'features' are supposed to go in there? Are these bits, > like you defined for VIRTIO_MDEV_F_VERSION_1 above? It's the API or mdev features other than virtio features. It could be used by driver to determine the capability of the mdev device. Besides _F_VERSION_1, we may add dirty page tracking etc which means we need new device ops. > >> + * @get_generation: Get device generaton >> + * @mdev: mediated device >> + * Returns u32: device generation > Is that callback mandatory? I think so, it's hard to emulate that completely in virtio-mdev transport. Thanks > >> + */ >> +struct virtio_mdev_device_ops { >> + /* Virtqueue ops */ >> + int (*set_vq_address)(struct mdev_device *mdev, >> + u16 idx, u64 desc_area, u64 driver_area, >> + u64 device_area); >> + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num); >> + void (*kick_vq)(struct mdev_device *mdev, u16 idx); >> + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx, >> + struct virtio_mdev_callback *cb); >> + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready); >> + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx); >> + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state); >> + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx); >> + >> + /* Device ops */ >> + u16 (*get_vq_align)(struct mdev_device *mdev); >> + u64 (*get_features)(struct mdev_device *mdev); >> + int (*set_features)(struct mdev_device *mdev, u64 features); >> + void (*set_config_cb)(struct mdev_device *mdev, >> + struct virtio_mdev_callback *cb); >> + u16 (*get_vq_num_max)(struct mdev_device *mdev); >> + u32 (*get_device_id)(struct mdev_device *mdev); >> + u32 (*get_vendor_id)(struct mdev_device *mdev); >> + u8 (*get_status)(struct mdev_device *mdev); >> + void (*set_status)(struct mdev_device *mdev, u8 status); >> + void (*get_config)(struct mdev_device *mdev, unsigned int offset, >> + void *buf, unsigned int len); >> + void (*set_config)(struct mdev_device *mdev, unsigned int offset, >> + const void *buf, unsigned int len); >> + u64 (*get_mdev_features)(struct mdev_device *mdev); >> + u32 (*get_generation)(struct mdev_device *mdev); >> +}; >> + >> +void mdev_set_virtio_ops(struct mdev_device *mdev, >> + const struct virtio_mdev_device_ops *virtio_ops); >> + >> +#endif
On 2019/10/18 下午5:46, Tiwei Bie wrote: > On Thu, Oct 17, 2019 at 06:48:34PM +0800, Jason Wang wrote: >> + * @get_vq_state: Get the state for a virtqueue >> + * @mdev: mediated device >> + * @idx: virtqueue index >> + * Returns virtqueue state (last_avail_idx) >> + * @get_vq_align: Get the virtqueue align requirement >> + * for the device >> + * @mdev: mediated device >> + * Returns virtqueue algin requirement >> + * @get_features: Get virtio features supported by the device >> + * @mdev: mediated device >> + * Returns the virtio features support by the >> + * device >> + * @get_features: Set virtio features supported by the driver > s/get_features/set_features/ Will fix. > >> + * configration space >> + * @get_mdev_features: Get the feature of virtio mdev device >> + * @mdev: mediated device >> + * Returns the mdev features (API) support by >> + * the device. >> + * @get_generation: Get device generaton >> + * @mdev: mediated device >> + * Returns u32: device generation >> + */ >> +struct virtio_mdev_device_ops { >> + /* Virtqueue ops */ >> + int (*set_vq_address)(struct mdev_device *mdev, >> + u16 idx, u64 desc_area, u64 driver_area, >> + u64 device_area); >> + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num); >> + void (*kick_vq)(struct mdev_device *mdev, u16 idx); >> + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx, >> + struct virtio_mdev_callback *cb); >> + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready); >> + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx); >> + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state); >> + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx); >> + >> + /* Device ops */ >> + u16 (*get_vq_align)(struct mdev_device *mdev); >> + u64 (*get_features)(struct mdev_device *mdev); >> + int (*set_features)(struct mdev_device *mdev, u64 features); >> + void (*set_config_cb)(struct mdev_device *mdev, >> + struct virtio_mdev_callback *cb); >> + u16 (*get_vq_num_max)(struct mdev_device *mdev); >> + u32 (*get_device_id)(struct mdev_device *mdev); >> + u32 (*get_vendor_id)(struct mdev_device *mdev); >> + u8 (*get_status)(struct mdev_device *mdev); >> + void (*set_status)(struct mdev_device *mdev, u8 status); >> + void (*get_config)(struct mdev_device *mdev, unsigned int offset, >> + void *buf, unsigned int len); >> + void (*set_config)(struct mdev_device *mdev, unsigned int offset, >> + const void *buf, unsigned int len); >> + u64 (*get_mdev_features)(struct mdev_device *mdev); > Do we need a .set_mdev_features method as well? Good question. To me I think we may document that the API provides backward compatibility, so there's no need for set_mdev_features. Or is there any other chance that we need that? > > It's not very clear what does mdev_features mean. > Does it mean the vhost backend features? > > https://github.com/torvalds/linux/blob/0e2adab6cf285c41e825b6c74a3aa61324d1132c/include/uapi/linux/vhost.h#L93-L94 Something like this, it's kind of the version of the API, except for the _F_VERSION_1, the first user should be _F_LOG_ALL. I will add more docs for this API. Thanks > > >> + u32 (*get_generation)(struct mdev_device *mdev); >> +}; >> + >> +void mdev_set_virtio_ops(struct mdev_device *mdev, >> + const struct virtio_mdev_device_ops *virtio_ops); >> + >> +#endif >> -- >> 2.19.1 >>
On Fri, 18 Oct 2019 18:55:02 +0800 Jason Wang <jasowang@redhat.com> wrote: > On 2019/10/18 下午5:46, Cornelia Huck wrote: > > On Thu, 17 Oct 2019 18:48:34 +0800 > > Jason Wang <jasowang@redhat.com> wrote: > >> + * @get_vendor_id: Get virtio vendor id > >> + * @mdev: mediated device > >> + * Returns u32: virtio vendor id > > How is the vendor id defined? As for normal virtio-pci devices? > > > The vendor that provides this device. So something like this > > I notice that MMIO also had this so it looks to me it's not pci specific. Ok. Would be good to specify this more explicitly. > > > > > >> + * @get_status: Get the device status > >> + * @mdev: mediated device > >> + * Returns u8: virtio device status > >> + * @set_status: Set the device status > >> + * @mdev: mediated device > >> + * @status: virtio device status > >> + * @get_config: Read from device specific configuration space > >> + * @mdev: mediated device > >> + * @offset: offset from the beginning of > >> + * configuration space > >> + * @buf: buffer used to read to > >> + * @len: the length to read from > >> + * configration space > >> + * @set_config: Write to device specific configuration space > >> + * @mdev: mediated device > >> + * @offset: offset from the beginning of > >> + * configuration space > >> + * @buf: buffer used to write from > >> + * @len: the length to write to > >> + * configration space > >> + * @get_mdev_features: Get the feature of virtio mdev device > >> + * @mdev: mediated device > >> + * Returns the mdev features (API) support by > >> + * the device. > > What kind of 'features' are supposed to go in there? Are these bits, > > like you defined for VIRTIO_MDEV_F_VERSION_1 above? > > > It's the API or mdev features other than virtio features. It could be > used by driver to determine the capability of the mdev device. Besides > _F_VERSION_1, we may add dirty page tracking etc which means we need new > device ops. Ok, so that's supposed to be distinct bits that can be or'ed together? Makes sense, but probably needs some more documentation somewhere. > > > > > >> + * @get_generation: Get device generaton > >> + * @mdev: mediated device > >> + * Returns u32: device generation > > Is that callback mandatory? > > > I think so, it's hard to emulate that completely in virtio-mdev transport. IIRC, the generation stuff is not mandatory in the current version of virtio, as not all transports have that concept. Generally, are any of the callbacks optional, or are all of them mandatory? From what I understand, you plan to add new things that depend on features... would that mean non-mandatory callbacks?
On 2019/10/18 下午9:30, Cornelia Huck wrote: > On Fri, 18 Oct 2019 18:55:02 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> On 2019/10/18 下午5:46, Cornelia Huck wrote: >>> On Thu, 17 Oct 2019 18:48:34 +0800 >>> Jason Wang <jasowang@redhat.com> wrote: >>>> + * @get_vendor_id: Get virtio vendor id >>>> + * @mdev: mediated device >>>> + * Returns u32: virtio vendor id >>> How is the vendor id defined? As for normal virtio-pci devices? >> >> The vendor that provides this device. So something like this >> >> I notice that MMIO also had this so it looks to me it's not pci specific. > Ok. Would be good to specify this more explicitly. Ok. > >> >>> >>>> + * @get_status: Get the device status >>>> + * @mdev: mediated device >>>> + * Returns u8: virtio device status >>>> + * @set_status: Set the device status >>>> + * @mdev: mediated device >>>> + * @status: virtio device status >>>> + * @get_config: Read from device specific configuration space >>>> + * @mdev: mediated device >>>> + * @offset: offset from the beginning of >>>> + * configuration space >>>> + * @buf: buffer used to read to >>>> + * @len: the length to read from >>>> + * configration space >>>> + * @set_config: Write to device specific configuration space >>>> + * @mdev: mediated device >>>> + * @offset: offset from the beginning of >>>> + * configuration space >>>> + * @buf: buffer used to write from >>>> + * @len: the length to write to >>>> + * configration space >>>> + * @get_mdev_features: Get the feature of virtio mdev device >>>> + * @mdev: mediated device >>>> + * Returns the mdev features (API) support by >>>> + * the device. >>> What kind of 'features' are supposed to go in there? Are these bits, >>> like you defined for VIRTIO_MDEV_F_VERSION_1 above? >> >> It's the API or mdev features other than virtio features. It could be >> used by driver to determine the capability of the mdev device. Besides >> _F_VERSION_1, we may add dirty page tracking etc which means we need new >> device ops. > Ok, so that's supposed to be distinct bits that can be or'ed together? Yes. > Makes sense, but probably needs some more documentation somewhere. Let me add some doc above this helper. > >> >>> >>>> + * @get_generation: Get device generaton >>>> + * @mdev: mediated device >>>> + * Returns u32: device generation >>> Is that callback mandatory? >> >> I think so, it's hard to emulate that completely in virtio-mdev transport. > IIRC, the generation stuff is not mandatory in the current version of > virtio, as not all transports have that concept. It looks to me we can have workaround as what has been done by virtio.c. Return 0 if this helper is not provided. > > Generally, are any of the callbacks optional, or are all of them > mandatory? From what I understand, you plan to add new things that > depend on features... would that mean non-mandatory callbacks? Yes, not all of them were mandatory, I will explicit explain which are mandatory and which are not. Thanks
diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c index d0f3113c8071..5834f6b7c7a5 100644 --- a/drivers/vfio/mdev/mdev_core.c +++ b/drivers/vfio/mdev/mdev_core.c @@ -57,6 +57,18 @@ void mdev_set_vfio_ops(struct mdev_device *mdev, } EXPORT_SYMBOL(mdev_set_vfio_ops); +/* Specify the virtio device ops for the mdev device, this + * must be called during create() callback for virtio mdev device. + */ +void mdev_set_virtio_ops(struct mdev_device *mdev, + const struct virtio_mdev_device_ops *virtio_ops) +{ + BUG_ON(mdev->class_id); + mdev->class_id = MDEV_CLASS_ID_VIRTIO; + mdev->device_ops = virtio_ops; +} +EXPORT_SYMBOL(mdev_set_virtio_ops); + const void *mdev_get_dev_ops(struct mdev_device *mdev) { return mdev->device_ops; diff --git a/include/linux/mdev.h b/include/linux/mdev.h index 3d29e09e20c9..13e045e09d3b 100644 --- a/include/linux/mdev.h +++ b/include/linux/mdev.h @@ -17,6 +17,7 @@ struct mdev_device; struct vfio_mdev_device_ops; +struct virtio_mdev_device_ops; /* * Called by the parent device driver to set the device which represents @@ -111,6 +112,8 @@ void mdev_set_drvdata(struct mdev_device *mdev, void *data); const guid_t *mdev_uuid(struct mdev_device *mdev); void mdev_set_vfio_ops(struct mdev_device *mdev, const struct vfio_mdev_device_ops *vfio_ops); +void mdev_set_virtio_ops(struct mdev_device *mdev, + const struct virtio_mdev_device_ops *virtio_ops); const void *mdev_get_dev_ops(struct mdev_device *mdev); extern struct bus_type mdev_bus_type; @@ -127,6 +130,7 @@ struct mdev_device *mdev_from_dev(struct device *dev); enum { MDEV_CLASS_ID_VFIO = 1, + MDEV_CLASS_ID_VIRTIO = 2, /* New entries must be added here */ }; diff --git a/include/linux/virtio_mdev.h b/include/linux/virtio_mdev.h new file mode 100644 index 000000000000..b965b50f9b24 --- /dev/null +++ b/include/linux/virtio_mdev.h @@ -0,0 +1,151 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Virtio mediated device driver + * + * Copyright 2019, Red Hat Corp. + * Author: Jason Wang <jasowang@redhat.com> + */ +#ifndef _LINUX_VIRTIO_MDEV_H +#define _LINUX_VIRTIO_MDEV_H + +#include <linux/interrupt.h> +#include <linux/mdev.h> +#include <uapi/linux/vhost.h> + +#define VIRTIO_MDEV_DEVICE_API_STRING "virtio-mdev" +#define VIRTIO_MDEV_F_VERSION_1 0x1 + +struct virtio_mdev_callback { + irqreturn_t (*callback)(void *data); + void *private; +}; + +/** + * struct vfio_mdev_device_ops - Structure to be registered for each + * mdev device to register the device to virtio-mdev module. + * + * @set_vq_address: Set the address of virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @desc_area: address of desc area + * @driver_area: address of driver area + * @device_area: address of device area + * Returns integer: success (0) or error (< 0) + * @set_vq_num: Set the size of virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @num: the size of virtqueue + * @kick_vq: Kick the virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @set_vq_cb: Set the interrupt callback function for + * a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @cb: virtio-mdev interrupt callback structure + * @set_vq_ready: Set ready status for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @ready: ready (true) not ready(false) + * @get_vq_ready: Get ready status for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * Returns boolean: ready (true) or not (false) + * @set_vq_state: Set the state for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * @state: virtqueue state (last_avail_idx) + * Returns integer: success (0) or error (< 0) + * @get_vq_state: Get the state for a virtqueue + * @mdev: mediated device + * @idx: virtqueue index + * Returns virtqueue state (last_avail_idx) + * @get_vq_align: Get the virtqueue align requirement + * for the device + * @mdev: mediated device + * Returns virtqueue algin requirement + * @get_features: Get virtio features supported by the device + * @mdev: mediated device + * Returns the virtio features support by the + * device + * @get_features: Set virtio features supported by the driver + * @mdev: mediated device + * @features: feature support by the driver + * Returns integer: success (0) or error (< 0) + * @set_config_cb: Set the config interrupt callback + * @mdev: mediated device + * @cb: virtio-mdev interrupt callback structure + * @get_vq_num_max: Get the max size of virtqueue + * @mdev: mediated device + * Returns u16: max size of virtqueue + * @get_device_id: Get virtio device id + * @mdev: mediated device + * Returns u32: virtio device id + * @get_vendor_id: Get virtio vendor id + * @mdev: mediated device + * Returns u32: virtio vendor id + * @get_status: Get the device status + * @mdev: mediated device + * Returns u8: virtio device status + * @set_status: Set the device status + * @mdev: mediated device + * @status: virtio device status + * @get_config: Read from device specific configuration space + * @mdev: mediated device + * @offset: offset from the beginning of + * configuration space + * @buf: buffer used to read to + * @len: the length to read from + * configration space + * @set_config: Write to device specific configuration space + * @mdev: mediated device + * @offset: offset from the beginning of + * configuration space + * @buf: buffer used to write from + * @len: the length to write to + * configration space + * @get_mdev_features: Get the feature of virtio mdev device + * @mdev: mediated device + * Returns the mdev features (API) support by + * the device. + * @get_generation: Get device generaton + * @mdev: mediated device + * Returns u32: device generation + */ +struct virtio_mdev_device_ops { + /* Virtqueue ops */ + int (*set_vq_address)(struct mdev_device *mdev, + u16 idx, u64 desc_area, u64 driver_area, + u64 device_area); + void (*set_vq_num)(struct mdev_device *mdev, u16 idx, u32 num); + void (*kick_vq)(struct mdev_device *mdev, u16 idx); + void (*set_vq_cb)(struct mdev_device *mdev, u16 idx, + struct virtio_mdev_callback *cb); + void (*set_vq_ready)(struct mdev_device *mdev, u16 idx, bool ready); + bool (*get_vq_ready)(struct mdev_device *mdev, u16 idx); + int (*set_vq_state)(struct mdev_device *mdev, u16 idx, u64 state); + u64 (*get_vq_state)(struct mdev_device *mdev, u16 idx); + + /* Device ops */ + u16 (*get_vq_align)(struct mdev_device *mdev); + u64 (*get_features)(struct mdev_device *mdev); + int (*set_features)(struct mdev_device *mdev, u64 features); + void (*set_config_cb)(struct mdev_device *mdev, + struct virtio_mdev_callback *cb); + u16 (*get_vq_num_max)(struct mdev_device *mdev); + u32 (*get_device_id)(struct mdev_device *mdev); + u32 (*get_vendor_id)(struct mdev_device *mdev); + u8 (*get_status)(struct mdev_device *mdev); + void (*set_status)(struct mdev_device *mdev, u8 status); + void (*get_config)(struct mdev_device *mdev, unsigned int offset, + void *buf, unsigned int len); + void (*set_config)(struct mdev_device *mdev, unsigned int offset, + const void *buf, unsigned int len); + u64 (*get_mdev_features)(struct mdev_device *mdev); + u32 (*get_generation)(struct mdev_device *mdev); +}; + +void mdev_set_virtio_ops(struct mdev_device *mdev, + const struct virtio_mdev_device_ops *virtio_ops); + +#endif
This patch implements basic support for mdev driver that supports virtio transport for kernel virtio driver. Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/vfio/mdev/mdev_core.c | 12 +++ include/linux/mdev.h | 4 + include/linux/virtio_mdev.h | 151 ++++++++++++++++++++++++++++++++++ 3 files changed, 167 insertions(+) create mode 100644 include/linux/virtio_mdev.h