Re: [PATCH V4 4/6] mdev: introduce virtio device and its device ops

Jason Wang
2019-10-18T10:57:22Z

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
>>