xend: hot-plug PCI devices at boot-time

Currently there are two interfaces to pass-through PCI devices:
1. A method driven through per-device xenstore entries that is used at
boot-time
2. An event-based method used for hot-plug.

This seems somewhat redundant and makes extending the code cumbersome
and prone to error - often the change needs to be made twice, in
two different ways.

This patch unifies PCI pass-through by using the existing event-based
method at boot-time.

Signed-off-by: Simon Horman <horms@verge.net.au>

tools/python/xen/xend/XendConfig.py

@@ -1669,6 +1669,13 @@ class XendConfig(dict):
 
         return ''
 
+    def pci_convert_dict_to_sxp(self, dev, state, sub_state = None):
+        sxp =  ['pci', ['dev'] + map(lambda (x, y): [x, y], dev.items()),
+                ['state', state]]
+        if sub_state != None:
+            sxp.append(['sub_state', sub_state])
+        return sxp
+
     def pci_convert_sxp_to_dict(self, dev_sxp):
         """Convert pci device sxp to dict
         @param dev_sxp: device configuration
@@ -1723,9 +1730,10 @@ class XendConfig(dict):
                     pci_dev_info[opt] = val
                 except (TypeError, ValueError):
                     pass
-            # append uuid for each pci device.
-            dpci_uuid = pci_dev_info.get('uuid', uuid.createString())
-            pci_dev_info['uuid'] = dpci_uuid
+            # append uuid to each pci device that does't already have one.
+            if not pci_dev_info.has_key('uuid'):
+                dpci_uuid = pci_dev_info.get('uuid', uuid.createString())
+                pci_dev_info['uuid'] = dpci_uuid
             pci_devs.append(pci_dev_info)
         dev_config['devs'] = pci_devs 
 

tools/python/xen/xend/XendDomainInfo.py

@@ -601,7 +601,7 @@ class XendDomainInfo:
         asserts.isCharConvertible(key)
         self.storeDom("control/sysrq", '%c' % key)
 
-    def sync_pcidev_info(self):
+    def pci_device_configure_boot(self):
 
         if not self.info.is_hvm():
             return
@@ -615,33 +615,12 @@ class XendDomainInfo:
         dev_uuid = sxp.child_value(dev_info, 'uuid')
         pci_conf = self.info['devices'][dev_uuid][1]
         pci_devs = pci_conf['devs']
+        request = map(lambda x:
+                      self.info.pci_convert_dict_to_sxp(x, 'Initialising',
+                                                        'Booting'), pci_devs)
 
-        count = 0
-        vslots = None
-        while vslots is None and count < 20:
-            vslots = xstransact.Read("/local/domain/0/backend/pci/%u/%s/vslots"
-                              % (self.getDomid(), devid))
-            time.sleep(0.1)
-            count += 1
-        if vslots is None:
-            log.error("Device model didn't tell the vslots for PCI device")
-            return
-
-        #delete last delim
-        if vslots[-1] == ";":
-            vslots = vslots[:-1]
-
-        slot_list = vslots.split(';')
-        if len(slot_list) != len(pci_devs):
-            log.error("Device model's pci dev num dismatch")
-            return
-
-        #update the vslot info
-        count = 0;
-        for x in pci_devs:
-            x['vslot'] = slot_list[count]
-            count += 1
-
+        for i in request:
+                self.pci_device_configure(i)
 
     def hvm_pci_device_create(self, dev_config):
         log.debug("XendDomainInfo.hvm_pci_device_create: %s"
@@ -741,6 +720,23 @@ class XendDomainInfo:
                     " assigned to other domain." \
                     )% (pci_device.name, self.info['name_label'], pci_str))
 
+        return self.hvm_pci_device_insert_dev(new_dev)
+
+    def hvm_pci_device_insert(self, dev_config):
+        log.debug("XendDomainInfo.hvm_pci_device_insert: %s"
+                  % scrub_password(dev_config))
+
+        if not self.info.is_hvm():
+            raise VmError("hvm_pci_device_create called on non-HVM guest")
+
+        new_dev = dev_config['devs'][0]
+
+        return self.hvm_pci_device_insert_dev(new_dev)
+
+    def hvm_pci_device_insert_dev(self, new_dev):
+        log.debug("XendDomainInfo.hvm_pci_device_insert_dev: %s"
+                  % scrub_password(new_dev))
+
         if self.domid is not None:
             opts = ''
             if 'opts' in new_dev and len(new_dev['opts']) > 0:
@@ -752,7 +748,10 @@ class XendDomainInfo:
                 new_dev['bus'],
                 new_dev['slot'],
                 new_dev['func'],
-                new_dev['requested_vslot'],
+                # vslot will be used when activating a
+                # previously activated domain.
+                # Otherwise requested_vslot will be used.
+                assigned_or_requested_vslot(new_dev),
                 opts)
             self.image.signalDeviceModel('pci-ins', 'pci-inserted', bdf_str)
 
@@ -827,6 +826,7 @@ class XendDomainInfo:
             return False
 
         pci_state = sxp.child_value(dev_sxp, 'state')
+        pci_sub_state = sxp.child_value(dev_sxp, 'sub_state')
         existing_dev_info = self._getDeviceInfo_pci(devid)
 
         if existing_dev_info is None and pci_state != 'Initialising':
@@ -840,7 +840,10 @@ class XendDomainInfo:
         if self.info.is_hvm():
             if pci_state == 'Initialising':
                 # HVM PCI device attachment
-                vslot = self.hvm_pci_device_create(dev_config)
+                if pci_sub_state == 'Booting':
+                    vslot = self.hvm_pci_device_insert(dev_config)
+                else:
+                    vslot = self.hvm_pci_device_create(dev_config)
                 # Update vslot
                 dev['vslot'] = vslot
                 for n in sxp.children(pci_dev):
@@ -907,7 +910,7 @@ class XendDomainInfo:
                         continue
                 new_dev_sxp.append(cur_dev)
 
-            if pci_state == 'Initialising':
+            if pci_state == 'Initialising' and pci_sub_state != 'Booting':
                 for new_dev in sxp.children(dev_sxp, 'dev'):
                     new_dev_sxp.append(new_dev)
 
@@ -2246,7 +2249,7 @@ class XendDomainInfo:
             self.image.createDeviceModel()
 
         #if have pass-through devs, need the virtual pci slots info from qemu
-        self.sync_pcidev_info()
+        self.pci_device_configure_boot()
 
     def _releaseDevices(self, suspend = False):
         """Release all domain's devices.  Nothrow guarantee."""

tools/python/xen/xend/image.py

@@ -454,8 +454,22 @@ class ImageHandler:
         if cmd is '' or ret is '':
             raise VmError('need valid command and result when signal device model')
 
-        orig_state = xstransact.Read("/local/domain/0/device-model/%i/state"
+        count = 0
+        while True:
+            orig_state = xstransact.Read("/local/domain/0/device-model/%i/state"
                                 % self.vm.getDomid())
+            # This can occur right after start-up
+            if orig_state != None:
+                break
+
+            log.debug('signalDeviceModel: orig_state is None, retrying')
+
+            time.sleep(0.1)
+            count += 1
+            if count < 100:
+                continue
+
+            VmError('Device model isn\'t ready for commands')
 
         if par is not None:
             xstransact.Store("/local/domain/0/device-model/%i"

tools/python/xen/xend/server/pciif.py

@@ -69,12 +69,7 @@ class PciController(DevController):
         """@see DevController.getDeviceDetails"""
         back = {}
         pcidevid = 0
-        vslots = ""
         for pci_config in config.get('devs', []):
-            attached_vslot = pci_config.get('vslot')
-            if attached_vslot is not None:
-                vslots = vslots + attached_vslot + ";"
-
             domain = parse_hex(pci_config.get('domain', 0))
             bus = parse_hex(pci_config.get('bus', 0))
             slot = parse_hex(pci_config.get('slot', 0))
@@ -93,9 +88,6 @@ class PciController(DevController):
             back['vslot-%i' % pcidevid] = "%02x" % vslot
             pcidevid += 1
 
-        if vslots != "":
-            back['vslots'] = vslots
-
         back['num_devs']=str(pcidevid)
         back['uuid'] = config.get('uuid','')
         if 'pci_msitranslate' in self.vm.info['platform']:
@@ -105,16 +97,17 @@ class PciController(DevController):
 
         return (0, back, {})
 
+    def reconfigureDevice_find(self, devid, nsearch_dev, match_dev):
+        for j in range(nsearch_dev):
+            if match_dev == self.readBackend(devid, 'dev-%i' % j):
+                return j
+        return None
 
     def reconfigureDevice(self, _, config):
         """@see DevController.reconfigureDevice"""
         (devid, back, front) = self.getDeviceDetails(config)
         num_devs = int(back['num_devs'])
         states = config.get('states', [])
-
-        old_vslots = self.readBackend(devid, 'vslots')
-        if old_vslots is None:
-            old_vslots = ''
         num_olddevs = int(self.readBackend(devid, 'num_devs'))
 
         for i in range(num_devs):
@@ -129,11 +122,15 @@ class PciController(DevController):
                 raise XendError('Error reading config')
 
             if state == 'Initialising':
-                # PCI device attachment
-                for j in range(num_olddevs):
-                    if dev == self.readBackend(devid, 'dev-%i' % j):
-                        raise XendError('Device %s is already connected.' % dev)
-                log.debug('Attaching PCI device %s.' % dev)
+                devno = self.reconfigureDevice_find(devid, num_olddevs, dev)
+                if devno == None:
+                    devno = num_olddevs + i
+                    log.debug('Attaching PCI device %s.' % dev)
+                    attaching = True
+                else:
+                    log.debug('Reconfiguring PCI device %s.' % dev)
+                    attaching = False
+
                 (domain, bus, slotfunc) = dev.split(':')
                 (slot, func) = slotfunc.split('.')
                 domain = parse_hex(domain)
@@ -142,41 +139,28 @@ class PciController(DevController):
                 func = parse_hex(func)
                 self.setupOneDevice(domain, bus, slot, func)
 
-                self.writeBackend(devid, 'dev-%i' % (num_olddevs + i), dev)
-                self.writeBackend(devid, 'state-%i' % (num_olddevs + i),
+                self.writeBackend(devid, 'dev-%i' % devno, dev)
+                self.writeBackend(devid, 'state-%i' % devno,
                                   str(xenbusState['Initialising']))
-                self.writeBackend(devid, 'uuid-%i' % (num_olddevs + i), uuid)
+                self.writeBackend(devid, 'uuid-%i' % devno, uuid)
                 if len(opts) > 0:
-                    self.writeBackend(devid, 'opts-%i' % (num_olddevs + i), opts)
-                self.writeBackend(devid, 'num_devs', str(num_olddevs + i + 1))
+                    self.writeBackend(devid, 'opts-%i' % devno, opts)
+                if back.has_key('vslot-%i' % i):
+                    self.writeBackend(devid, 'vslot-%i' % devno,
+                                      back['vslot-%i' % i])
 
-                # Update vslots
-                if back['vslots'] is not None:
-                    vslots = old_vslots + back['vslots']
-                    self.writeBackend(devid, 'vslots', vslots)
+                # If a device is being attached then num_devs will grow
+                if attaching:
+                    self.writeBackend(devid, 'num_devs', str(devno + 1))
 
             elif state == 'Closing':
                 # PCI device detachment
-                found = False
-                for j in range(num_olddevs):
-                    if dev == self.readBackend(devid, 'dev-%i' % j):
-                        found = True
-                        log.debug('Detaching device %s' % dev)
-                        self.writeBackend(devid, 'state-%i' % j,
-                                          str(xenbusState['Closing']))
-                if not found:
+                devno = self.reconfigureDevice_find(devid, num_olddevs, dev)
+                if devno == None:
                     raise XendError('Device %s is not connected' % dev)
-
-                # Update vslots
-                if back.get('vslots') is not None:
-                    vslots = old_vslots
-                    for vslot in back['vslots'].split(';'):
-                        if vslot != '':
-                            vslots = vslots.replace(vslot + ';', '', 1)
-                    if vslots == '':
-                        self.removeBackend(devid, 'vslots')
-                    else:
-                        self.writeBackend(devid, 'vslots', vslots)
+                log.debug('Detaching device %s' % dev)
+                self.writeBackend(devid, 'state-%i' % devno,
+                                  str(xenbusState['Closing']))
 
             else:
                 raise XendError('Error configuring device %s: invalid state %s'
@@ -191,12 +175,6 @@ class PciController(DevController):
         result = DevController.getDeviceConfiguration(self, devid, transaction)
         num_devs = self.readBackend(devid, 'num_devs')
         pci_devs = []
-        
-        vslots = self.readBackend(devid, 'vslots')
-        if vslots is not None:
-            if vslots[-1] == ";":
-                vslots = vslots[:-1]
-            slot_list = vslots.split(';')
 
         for i in range(int(num_devs)):
             dev_config = self.readBackend(devid, 'dev-%d' % i)
@@ -215,13 +193,11 @@ class PciController(DevController):
 
                 # Per device uuid info
                 dev_dict['uuid'] = self.readBackend(devid, 'uuid-%d' % i)
-
-                #append vslot info
-                if vslots is not None:
-                    try:
-                        dev_dict['vslot'] = slot_list[i]
-                    except IndexError:
-                        dev_dict['vslot'] = AUTO_PHP_SLOT_STR
+                vslot = self.readBackend(devid, 'vslot-%d' % i)
+                if vslot != None:
+                    dev_dict['vslot'] = self.readBackend(devid, 'vslot-%d' % i)
+                else:
+                    dev_dict['vslot'] = AUTO_PHP_SLOT_STR
 
                 #append opts info
                 opts = self.readBackend(devid, 'opts-%d' % i)