This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH 4 of 4] Linux pvops: xen pci platform device driv

To: Bastian Blank <waldi@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 4 of 4] Linux pvops: xen pci platform device driver
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Wed, 3 Mar 2010 15:52:07 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "linux-kernel@xxxxxxxxxxxxxxx" <linux-kernel@xxxxxxxxxxxxxxx>
Delivery-date: Wed, 03 Mar 2010 07:51:32 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20100303121726.GA26656@xxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <alpine.DEB.2.00.1003021748250.22259@kaball-desktop> <20100303121726.GA26656@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
On Wed, 3 Mar 2010, Bastian Blank wrote:
> Why is version 1 grant table the default on HVM?

Because version 2 is known not to work on HVM, fixing this is on my TODO

> > +/* NB. [aux-]ide-disks options do not unplug IDE CD-ROM drives. */
> > +/* NB. aux-ide-disks is equiv to ide-disks except ignores primary master. 
> > */
> > +static char *dev_unplug;
> > +module_param(dev_unplug, charp, 0644);
> > +MODULE_PARM_DESC(dev_unplug, "Emulated devices to unplug: "
> > +            "[all,][ide-disks,][aux-ide-disks,][nics]\n");
> What is this parameter about?

During the initialization of the xen pci device driver, we write to a
magic ioport that causes qemu to unplug some emulated devices, disc and
network in particular.
This parameter is meant to present the user a choice about what emulated
devices to unplug.

> > +#define XEN_IOPORT_BASE 0x10
> Why are this constants defined in the driver themself? They only make
> sense if there are two components making use of them.

Yep, the other one is qemu.

> > +   if (request_mem_region(mmio_addr, mmio_len, DRV_NAME) == NULL) {
> > +           printk(KERN_ERR ":MEM I/O resource 0x%lx @ 0x%lx busy\n",
> > +                  mmio_addr, mmio_len);
> > +           return -EBUSY;
> Why is this a sign of busy?

I think we were just trying to follow the behaviour of the other drivers
of real pci devices.
I am not sure how this condition could happen on Xen.

> > +   if ((ret = gnttab_init()))
> > +           goto out;
> > +
> > +   if ((ret = xenbus_probe_init()))
> > +           goto out;
> > +
> > + out:
> > +   if (ret) {
> > +           release_mem_region(mmio_addr, mmio_len);
> > +           release_region(ioaddr, iolen);
> If xenbus inits, this does not undo the gnttab init?

Do you mean "if xenbus DOESN'T init, this does not undo the gnttab
Strictly speaking the grant table can work without xenbus even if no Xen
pv driver is able to use it without xenstore support at the moment.

All the other comments indicate issues that need to be solved,
I'll address them in the next version of the series.
Most issues derive from the the fact that I did a straight port of the
platform-pci driver we have in xen-unstable/unmodified_drivers.

Thank you very much for the detailed review.



Xen-devel mailing list