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
