Re: [kvm-devel] [PATCH 1/6] virtio interace
On Sat, 2007-09-22 at 12:01 +0200, Arnd Bergmann wrote: On Saturday 22 September 2007, Rusty Russell wrote: But now each virtio device has two struct devices, not one. And you've made up a fictional bus to do it. Yet for PCI systems, it really is a PCI device; exposing a second bus to userspace just because we put a layer in our implementation is poor form. Perhaps this is the easiest way of doing it. But it's still wrong. I think it's just a matter of perspective. In the model I'm advocating, the PCI device is not the same as the virtio device but rather a virtio host bridge, very much like USB or SATA works. We could easily have multiple virtio devices behind one PCI device, but since virtual PCI devices are cheap, a one-to-one mapping makes sense for simplicity. This is still retro-justification. The simplest way for PCI systems to represent a virtio device as a PCI device; this makes life easy for any guest OS. I just know we're going to regret this... Rusty. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/6] virtio interace
Rusty Russell wrote: On Sat, 2007-09-22 at 12:01 +0200, Arnd Bergmann wrote: On Saturday 22 September 2007, Rusty Russell wrote: But now each virtio device has two struct devices, not one. And you've made up a fictional bus to do it. Yet for PCI systems, it really is a PCI device; exposing a second bus to userspace just because we put a layer in our implementation is poor form. Perhaps this is the easiest way of doing it. But it's still wrong. I think it's just a matter of perspective. In the model I'm advocating, the PCI device is not the same as the virtio device but rather a virtio host bridge, very much like USB or SATA works. We could easily have multiple virtio devices behind one PCI device, but since virtual PCI devices are cheap, a one-to-one mapping makes sense for simplicity. This is still retro-justification. The simplest way for PCI systems to represent a virtio device as a PCI device; this makes life easy for any guest OS. I just know we're going to regret this... Rusty. I'm not sure what's best, maybe this can help: With the virtio_find_driver you need to place all virtio drivers under the same pci device or suggest new mapping. Using Arnd's solution (actually I implemented it for KVM before the new patch set) and 1-1 mapping for pci and virtio devices makes life simpler. Dor. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/6] virtio interace
On Thursday 20 September 2007, Rusty Russell wrote:
+ * virtio_driver - operations for a virtio I/O driver
+ * @name: the name of the driver (KBUILD_MODNAME).
+ * @owner: the module which contains these routines (ie. THIS_MODULE).
+ * @id_table: the ids (we re-use PCI ids) serviced by this driver.
+ * @probe: the function to call when a device is found. Returns a token for
+ * remove, or PTR_ERR().
+ * @remove: the function when a device is removed.
+ */
+struct virtio_driver {
+ const char *name;
+ struct module *owner;
+ struct pci_device_id *id_table;
+ void *(*probe)(struct device *device,
+ struct virtio_config_space *config,
+ struct virtqueue_ops *vqops);
+ void (*remove)(void *dev);
+};
+
+int register_virtio_driver(struct virtio_driver *drv);
+void unregister_virtio_driver(struct virtio_driver *drv);
+
+/* The particular virtio backend supplies these. */
+struct virtio_backend_ops {
+ int (*register_driver)(struct virtio_driver *drv);
+ void (*unregister_driver)(struct virtio_driver *drv);
+};
+extern struct virtio_backend_ops virtio_backend_ops;
This still seems a little awkward. From what I understand, you register
a virtio_driver, which leads to a pci_driver (or whatever you are based on)
to be registered behind the covers, so that the pci_device can be used
directly as the virtio device.
I think there should instead be a pci_driver that automatically binds
to all PCI based virtio imlpementations and creates a child device for
the actual virtio_device. Then you can have the virtio_driver itself
be based on a device_driver, and you can get rid of the global
virtio_backend_ops. That will be useful when a virtual machine
has two ways to get at the virtio devices, e.g. a KVM guest that
has both hcall based probing for virtio devices and some other virtio
devices that are exported through PCI.
Arnd
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/6] virtio interace
On Fri, 2007-09-21 at 14:05 +0200, Arnd Bergmann wrote:
On Thursday 20 September 2007, Rusty Russell wrote:
+int register_virtio_driver(struct virtio_driver *drv);
+void unregister_virtio_driver(struct virtio_driver *drv);
+
+/* The particular virtio backend supplies these. */
+struct virtio_backend_ops {
+ int (*register_driver)(struct virtio_driver *drv);
+ void (*unregister_driver)(struct virtio_driver *drv);
+};
+extern struct virtio_backend_ops virtio_backend_ops;
This still seems a little awkward. From what I understand, you register
a virtio_driver, which leads to a pci_driver (or whatever you are based on)
to be registered behind the covers, so that the pci_device can be used
directly as the virtio device.
Hi Arnd,
Yes, and I dislike it too.
I think there should instead be a pci_driver that automatically binds
to all PCI based virtio imlpementations and creates a child device for
the actual virtio_device.
I'm not sure I understand. For PCI probing to work, you want to have
identified yourself as a driver for each PCI id claimed by a virtio
device.
Hmm, I guess we could have a PCI driver which claims all VIRTIO vendor
devices. Then it can call virtio_find_driver() (?) at the top of its
probe function to find if there's a matching virtio driver. This PCI
driver would have to be initialized after all the virtio drivers are
registered, but that's easy.
The virtio layer would simply maintain a linked list of drivers and
implement the virtio_find_driver() matching function.
And since we've suppressed normal PCI driver request_module (since it
always finds the driver) then we can implement that in
virtio_find_driver(), and not use a PCI MODULE_DEVICE_TABLE. Then we
don't need (full) PCI ids at all.
OK, I have some coding to do now...
Rusty.
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/6] virtio interace
On Friday 21 September 2007, Rusty Russell wrote:
Hmm, I guess we could have a PCI driver which claims all VIRTIO vendor
devices.
yes, that was the idea.
Then it can call virtio_find_driver() (?) at the top of its
probe function to find if there's a matching virtio driver.
This PCI driver would have to be initialized after all the virtio
drivers are registered, but that's easy.
No, just use the driver model, instead of working against it:
struct pci_virtio_device {
struct pci_dev *pdev;
char __iomem *mmio_space;
struct virtio_device vdev;
};
static int __devinit pci_virtio_probe(struct pci_dev *pdev,
const struct pci_device_id *ent)
{
struct pci_virtio_device *dev = kzalloc(sizeof (*dev), GFP_KERNEL);
dev-pdev = pdev;
dev-mmio_space = pcim_iomap(pdev, 0, PCI_VIRTIO_BUFSIZE);
dev-vdev-ops = pci_virtqueue_ops;
dev-vdev-config = pci_virtio_config_ops;
dev-vdev-type = ent-device;
dev-vdev-class = ent-class;
dev-vdev.dev.parent = pdev-dev;
return virtio_device_register(dev-vdev;
}
The virtio layer would simply maintain a linked list of drivers and
implement the virtio_find_driver() matching function.
nonono, just a virtio_bus that all virtio drivers register to:
static int virtio_net_probe(struct device *dev)
{
struct virtio_device *vdev = to_virtio_dev(dev);
struct virtqueue_ops *vq_ops = vdev-ops;
/* same as current code */
...
return 0;
}
static struct virtio_device_id virtio_net_ids[] = {
{ .type = VIRTIO_ID_NET, .class = PCI_CLASS_NETWORK_OTHER },
{ },
};
static struct virtio_driver virtio_net = {
.id_table = virtio_net_ids,
.driver = {
.name = virtionet,
.probe = virtio_net_probe,
.remove = virtionet_remove,
.bus = virtio_bus,/* - look here */
},
};
static int __init virtio_net_init(void)
{
return driver_register(virtio_net.driver);
}
module_init(virtio_net_init);
And since we've suppressed normal PCI driver request_module (since it
always finds the driver) then we can implement that in
virtio_find_driver(), and not use a PCI MODULE_DEVICE_TABLE. Then we
don't need (full) PCI ids at all.
right, as shown in my example above.
Arnd
-
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2005.
http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel
Re: [kvm-devel] [PATCH 1/6] virtio interace
Rusty Russell wrote: (Changes: - renamed sync to kick as Dor suggested - added new_vq and free_vq hooks to create virtqueues - define a simple virtio driver, which uses PCI ids - provide register/unregister_virtio_driver hooks) This attempts to implement a virtual I/O layer which should allow common drivers to be efficiently used across most virtual I/O mechanisms. It will no-doubt need further enhancement. The virtio drivers add and get I/O buffers; as the buffers are consumed the driver interrupt callbacks are invoked. It also provides driver register and unregister hooks, which are simply overridden at run time (eg. for a guest kernel which supports KVM paravirt and lguest). +++ b/drivers/virtio/virtio.c @@ -0,0 +1,20 @@ +#include linux/virtio.h + +struct virtio_backend_ops virtio_backend_ops; +EXPORT_SYMBOL_GPL(virtio_backend_ops); Suggest calling this virtio_transport_ops rather than the too-generic virtio_backend_ops. Especially since Xen uses backend for something completely different. + +/** + * virtqueue_ops - operations for virtqueue abstraction layer + * @new_vq: create a new virtqueue + * config: the virtio_config_space field describing the queue + * off: the offset in the config space of the queue configuration + * len: the length of the virtio_config_space field 'off, len' are really a magic cookie. Why does the interface care about their meaning? + * callback: the driver callback when the queue is used. Missing callback return value description. + * @kick: update after add_buf + * vq: the struct virtqueue + * After one or more add_buf calls, invoke this to kick the virtio layer. 'the other side' I'm not thrilled about reusing pci ids. Maybe the s390 can say whether this is a real issue. -- error compiling committee.c: too many arguments to function - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ ___ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel
