Re: [Xen-devel] [rfc 0/2] ioemu: Allow any unused PCI device to be used for pass-through

[Simon Horman <horms@xxxxxxxxxxxx>]

On Thu, Mar 19, 2009 at 05:30:00PM +1100, Simon Horman wrote:
> On Fri, Mar 13, 2009 at 06:07:34PM +1100, Simon Horman wrote:
> > The current limit of 2 pass-through devices seems to be rather small.
> > These patches remove that limit and allow any unused PCI device to
> > be used for pass-through, including hotplug.
> > 
> > It seems that the first few (4) devices will be used for other purposes by
> > HVM domains. However, it seems to me to be clean to make this code generic
> > and cope with any device on the bus - saving the need for subsequent
> > changes if the number of devices HVM domains use by default ever changes.
> > 
> > There are also xend and hvmloader patches required for this change.
> > I will submit them separately.
> 
> Hi Ian,
> 
> please consider applying these patches as they are required
> for hotplug to work since the firmware and xend portions where
> applied to xen-unstable.hg as:
> 
> changeset:   19362:986d0ed147ff
> user:        Keir Fraser <keir.fraser@xxxxxxxxxx>
> date:        Tue Mar 17 10:48:52 2009 +0000
> summary:     hvmloader: Allow any unused PCI device to be used for PCI
> hotplug
> 
> changeset:   19361:6e3e841f0259
> user:        Keir Fraser <keir.fraser@xxxxxxxxxx>
> date:        Tue Mar 17 10:42:32 2009 +0000
> summary:     xend: Use AUTO_PHP_SLOT_STR=0xff rather than 0x0 for auto pci
> allocation
> 
> 
> Thanks

Hi,

I found a regression introduced by the 2nd patch, in the
way that it treats vslots in pt_init.

pt_init() iterates through the PHP slots independantly of
the assignment that occurs inside __insert_to_pci_slot
which is called by register_real_device(). It assumes
that vslots are assigned in order sarting at PHP_SLOT_START.

This was valid before my change, although why it didn't take
the simpler option of just checking what value had been
assigned to pt_dev->dev.devfn in register_real_device()
is a mystery to me. Its also a mystery to what valid circumstance
could lead to pt_init() using 0 (a.k.a. unknown?) as the vslot.


My patch made one the assumptions that pt_init() made about slot numbers
invalid. That is, they don't start at PHP_SLOT_START, they start
wherever there is a free device.

A simple solution seems to be to use the value assigned to
pt_dev->dev.devfn in register_real_device().

The fix is as per the patch below. I will fold it into my previous
patch series and repost.

Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>

Index: ioemu-remote/hw/pass-through.c
===================================================================
--- ioemu-remote.orig/hw/pass-through.c 2009-03-20 11:05:58.000000000 +1100
+++ ioemu-remote/hw/pass-through.c      2009-03-20 11:22:06.000000000 +1100
@@ -3804,7 +3804,7 @@ int power_off_php_slot(int slot)
 
 int pt_init(PCIBus *e_bus, const char *direct_pci)
 {
-    int seg, b, d, f, slot, status = -1;
+    int seg, b, d, f, status = -1;
     struct pt_dev *pt_dev;
     struct pci_access *pci_access;
     char *vslots;
@@ -3840,9 +3840,7 @@ int pt_init(PCIBus *e_bus, const char *d
     vslots = qemu_mallocz ( strlen(direct_pci) / 3 );
 
     /* Assign given devices to guest */
-    for ( slot = 0;
-          slot < NR_PCI_DEV && next_bdf(&direct_pci_p, &seg, &b, &d, &f, &opt);
-          slot++ )
+    while ( next_bdf(&direct_pci_p, &seg, &b, &d, &f, &opt) )
     {
         /* Register real device with the emulated bus */
         pt_dev = register_real_device(e_bus, "DIRECT PCI", PT_VIRT_DEVFN_AUTO,
@@ -3854,9 +3852,7 @@ int pt_init(PCIBus *e_bus, const char *d
         }
 
         /* Record the virtual slot info */
-        sprintf(slot_str, "0x%02x;",
-                dpci_infos.php_devs[slot].pt_dev == pt_dev ? slot :
-                AUTO_PHP_SLOT);
+        sprintf(slot_str, "0x%02x;", PCI_SLOT(pt_dev->dev.devfn));
 
         strcat(vslots, slot_str);
     }


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel