Re: [kvm-devel] [PATCH/RFC 4/9] Basic guest virtual devices infrastructure

2007-05-14 Thread Avi Kivity
Carsten Otte wrote:
 From: Carsten Otte [EMAIL PROTECTED]

 This patch adds support for a new bus type that manages paravirtualized 
 devices. The bus uses the s390 diagnose instruction to query devices, and
 match them with the corresponding drivers.
 Future enhancements should include hotplug and hotremoval of virtual devices
 triggered by the host, and supend/resume of virtual devices for migration.

   

Interesting.  We could use a variation this for x86 as well, but I'm not 
sure how easy it is to integrate it into closed source OSes (Windows).  
The diag instruction could be replaced by a hypercall which would make 
the code generic.

-- 
error compiling committee.c: too many arguments to function


-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel


Re: [kvm-devel] [PATCH/RFC 4/9] Basic guest virtual devices infrastructure

2007-05-11 Thread Arnd Bergmann
On Friday 11 May 2007, Carsten Otte wrote:

 This patch adds support for a new bus type that manages paravirtualized
 devices. The bus uses the s390 diagnose instruction to query devices, and
 match them with the corresponding drivers.

It seems that the diagnose instruction is really the only s390 specific
thing in here, right? I guess this part of your series is the first one
that we should have in an architecture independent way.

There may also be the chance of merging this with existing virtual
buses like the one for the ps3, which also just exists using
hypercalls.

 +int vdev_match(struct device * dev, struct device_driver *drv)
 +{
 + struct vdev *vdev = to_vdev(dev);
 + struct vdev_driver *vdrv = to_vdrv(drv);
 +
 + if (vdev-vdev_type == vdrv-vdev_type)
 + return 1;
 +
 + return 0;
 +}

Why invent device type numbers? On open firmware, we just do a string compare,
which more intuitive, and means you don't need any further 

 +int vdev_probe(struct device * dev)
 +{
 + struct vdev *vdev = to_vdev(dev);
 + struct vdev_driver *vdrv = to_vdrv(dev-driver);
 +
 + return vdrv-probe(vdev);
 +}

This abstraction is unnecessary, just do the do_vdev() conversion inside
of the individual drivers.

 +
 +struct device vdev_bus = {
 + .bus_id  = vdev0,
 + .release = vdev_bus_release
 +};
 
 +static void vdev_bus_release (struct device *device)
 +{
 + /* noop, static bus object */
 +}

Just make the root of your devices a platform_device, then you don't need
to do dirty tricks like this.

 +static int vdev_scan_coldplug(void)
 +{
 + int rc;
 + struct vdev *device;
 +
 + do {
 + device = kzalloc(sizeof(struct vdev), GFP_ATOMIC);
 + if (!device) {
 + rc = -ENOMEM;
 + goto out;
 + }
 + rc = vdev_diag_hotplug(device-symname, device-hostid);
 + if (rc == -ENODEV)
 + break;
 + if (rc  0) {
 + printk (KERN_WARNING vdev: error %d detecting \
 +  initial devices\n, rc);
 + break;
 + }
 + device-vdev_type = rc;
 +
 + //sanity: are strings terminated?
 + if ((strnlen(device-symname, 128) == 128) ||
 + (strnlen(device-hostid, 128) == 128)) {
 + // warn and discard device
 + printk (vdev: illegal device entry received\n);
 + break;
 + }
 +
 + rc = vdevice_register(device);
 + if (rc) {
 + kfree(device);
 + } else
 + switch (device-vdev_type) {
 + case VDEV_TYPE_DISK:
 + printk (KERN_INFO vdev: storage device  \
 + detected: %s\n, device-symname);
 + break;
 + case VDEV_TYPE_NET:
 + printk (KERN_INFO vdev: network device  \
 + detected: %s\n, device-symname);
 + break;
 + default:
 + printk (KERN_INFO vdev: unknown device  \
 + detected: %s\n, device-symname);
 + }
 + } while(1);
 + kfree (device);
 + out:
 + return 0;
 +}

Interesting concept of probing the bus -- so you just ask if there are
any new devices, right?

 +#define VDEV_TYPE_DISK 0
 +#define VDEV_TYPE_NET  1
 +
 +struct vdev {
 + unsigned intvdev_type;
 + charsymname[128];
 + charhostid[128];
 + struct vdev_driver *driver;
 + struct device   dev;
 + void*drv_private;
 +};

You shouldn't need the driver and drv_private fields -- they are already
present in struct device.

Arnd 

-
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
___
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel