Messages in this thread | | | Subject | Re: [RFC PATCH 3/4] virtio: introudce a mdev based transport | From | Jason Wang <> | Date | Wed, 11 Sep 2019 17:53:48 +0800 |
| |
On 2019/9/11 下午5:36, Michael S. Tsirkin wrote: > On Wed, Sep 11, 2019 at 10:38:39AM +0800, Jason Wang wrote: >> On 2019/9/10 下午9:52, Michael S. Tsirkin wrote: >>> On Tue, Sep 10, 2019 at 09:13:02PM +0800, Jason Wang wrote: >>>> On 2019/9/10 下午6:01, Michael S. Tsirkin wrote: >>>>>> +#ifndef _LINUX_VIRTIO_MDEV_H >>>>>> +#define _LINUX_VIRTIO_MDEV_H >>>>>> + >>>>>> +#include <linux/interrupt.h> >>>>>> +#include <linux/vringh.h> >>>>>> +#include <uapi/linux/virtio_net.h> >>>>>> + >>>>>> +/* >>>>>> + * Ioctls >>>>>> + */ >>>>> Pls add a bit more content here. It's redundant to state these >>>>> are ioctls. Much better to document what does each one do. >>>> Ok. >>>> >>>> >>>>>> + >>>>>> +struct virtio_mdev_callback { >>>>>> + irqreturn_t (*callback)(void *); >>>>>> + void *private; >>>>>> +}; >>>>>> + >>>>>> +#define VIRTIO_MDEV 0xAF >>>>>> +#define VIRTIO_MDEV_SET_VQ_CALLBACK _IOW(VIRTIO_MDEV, 0x00, \ >>>>>> + struct virtio_mdev_callback) >>>>>> +#define VIRTIO_MDEV_SET_CONFIG_CALLBACK _IOW(VIRTIO_MDEV, 0x01, \ >>>>>> + struct virtio_mdev_callback) >>>>> Function pointer in an ioctl parameter? How does this ever make sense? >>>> I admit this is hacky (casting). >>>> >>>> >>>>> And can't we use a couple of registers for this, and avoid ioctls? >>>> Yes, how about something like interrupt numbers for each virtqueue and >>>> config? >>> Should we just reuse VIRTIO_PCI_COMMON_Q_XXX then? >> >> You mean something like VIRTIO_PCI_COMMON_Q_MSIX? Then it becomes a PCI >> transport in fact. And using either MSIX or irq number is actually another >> layer of indirection. So I think we can just write callback function and >> parameter through registers. > I just realized, all these registers are just encoded so you > can pass stuff through read/write. But it can instead be > just a normal C function call with no messy encoding. > So why do we want to do this encoding?
Just because it was easier to start as a POC since mdev_parent_ops is the only way to communicate between mdev driver and mdev device right now. We can invent private ops besides mdev_parent_ops, e.g a private pointer in mdev_parent_ops. I can try this in next version.
Thanks
| |