On Tue, Mar 02, 2010 at 06:31:10PM +0000, Stefano Stabellini wrote:
> +config XEN_PLATFORM_PCI
> + bool "xen platform pci device driver"
Case? Why does this need to be built-in?
> + depends on XEN
> + select XEN_XENBUS_FRONTEND
> + help
> + Necessary to run Xen PV drivers in Xen HVM domains.
A little bit short.
> - gsv.version = 2;
> + if (xen_hvm_domain())
> + gsv.version = 1;
> + else
> + gsv.version = 2;
> rc = HYPERVISOR_grant_table_op(GNTTABOP_set_version, &gsv, 1);
> if (rc == 0)
> - grant_table_version = 2;
> + grant_table_version = xen_hvm_domain() ? 1 : 2;
Why is version 1 grant table the default on HVM?
> - grant_table_version = 1;
> + grant_table_version = xen_hvm_domain() ? 2 : 1;
But then, this makes no sense for me.
> -static int __devinit gnttab_init(void)
> +int gnttab_init(void)
I miss an export for this function.
> -core_initcall(gnttab_init);
> +static int __devinit __gnttab_init(void)
> +{
> + if (!xen_domain())
> + return -ENODEV;
Shouldn't this read xen_pv_domain?
> +/* 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?
> +#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.
> +#define XEN_IOPORT_PLATFLAGS (XEN_IOPORT_BASE + 0) /* 1 byte access (R/W) */
> +#define XEN_IOPORT_MAGIC (XEN_IOPORT_BASE + 0) /* 2 byte access (R) */
> +#define XEN_IOPORT_UNPLUG (XEN_IOPORT_BASE + 0) /* 2 byte access (W) */
> +#define XEN_IOPORT_DRVVER (XEN_IOPORT_BASE + 0) /* 4 byte access (W) */
> +
> +#define XEN_IOPORT_SYSLOG (XEN_IOPORT_BASE + 2) /* 1 byte access (W) */
> +#define XEN_IOPORT_PROTOVER (XEN_IOPORT_BASE + 2) /* 1 byte access (R) */
> +#define XEN_IOPORT_PRODNUM (XEN_IOPORT_BASE + 2) /* 2 byte access (W) */
> +
> +#define XEN_IOPORT_MAGIC_VAL 0x49d2
> +#define XEN_IOPORT_LINUX_PRODNUM 0xffff /* NB: register a proper one */
What does this "register a proper one" mean?
> +#define XEN_IOPORT_LINUX_DRVVER ((LINUX_VERSION_CODE << 8) + 0x0)
> +
> +#define UNPLUG_ALL_IDE_DISKS 1
> +#define UNPLUG_ALL_NICS 2
> +#define UNPLUG_AUX_IDE_DISKS 4
> +#define UNPLUG_ALL 7
> +
> +static int check_platform_magic(struct device *dev, long ioaddr, long iolen)
> +{
> + short magic, unplug = 0;
> + char protocol, *p, *q, *err;
> +
> + for (p = dev_unplug; p; p = q) {
> + q = strchr(dev_unplug, ',');
> + if (q)
> + *q++ = '\0';
> + if (!strcmp(p, "all"))
> + unplug |= UNPLUG_ALL;
> + else if (!strcmp(p, "ide-disks"))
> + unplug |= UNPLUG_ALL_IDE_DISKS;
> + else if (!strcmp(p, "aux-ide-disks"))
> + unplug |= UNPLUG_AUX_IDE_DISKS;
> + else if (!strcmp(p, "nics"))
> + unplug |= UNPLUG_ALL_NICS;
> + else
> + dev_warn(dev, "unrecognised option '%s' "
> + "in module parameter 'dev_unplug'\n", p);
> + }
Usually this is done during the parameter setup.
> + 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?
> + 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?
> +#define XEN_PLATFORM_VENDOR_ID 0x5853
> +#define XEN_PLATFORM_DEVICE_ID 0x0001
> +static struct pci_device_id platform_pci_tbl[] __devinitdata = {
> + {XEN_PLATFORM_VENDOR_ID, XEN_PLATFORM_DEVICE_ID,
> + PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
> + /* Continue to recognise the old ID for now */
> + {0xfffd, 0x0101, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
What is the reasoning for this?
> +static int pci_device_registered;
What is this for?
Bastian
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home