xend: fix options for assigned pci

pci global options and per-device options for HVM device model have been broken for some time, the patch tries to fix the problem. It:
* maintains global options in xend, and merge it into per-device option when creating the backend
* merge the global options also into the parameter of pci-ins dm-command

The second one is there because the backend is effectively skipped in ioemu at present, ioemu solely relies on the parameter string to create the device.

Cc Simon Horman <[email protected]>

Cc Stefano Stabellini <[email protected]>

Signed-off-by Qing He <[email protected]>

---
diff -r 683bf4ce2e93 tools/python/xen/util/pci.py

tools/python/xen/util/pci.py +4 -0

@@ -158,6 +158,10 @@
     return map(lambda x: x.split('='),
                filter(lambda x: x != '', opts.split(',')))
 
+def append_default_pci_opts(opts, defopts):
+    optsdict = dict(opts)
+    return opts + filter(lambda (k, v): not optsdict.has_key(k), defopts)
+
 def pci_opts_list_to_sxp(list):
     return dev_dict_to_sxp({'opts': list})
 

tools/python/xen/util/pci.py +0 -0

     if pci_dev_info['opts'] != '':
         template['opts'] = split_pci_opts(pci_dev_info['opts'])
         check_pci_opts(template['opts'])

diff -r 683bf4ce2e93 tools/python/xen/xend/XendDomainInfo.py

tools/python/xen/xend/XendDomainInfo.py +14 -1

@@ -42,6 +42,7 @@
 from xen.util import xsconstants
 from xen.util import mkdir
 from xen.util.pci import serialise_pci_opts, pci_opts_list_to_sxp, \
+                         append_default_pci_opts, \
                          pci_dict_to_bdf_str, pci_dict_to_xc_str, \
                          pci_convert_sxp_to_dict, pci_convert_dict_to_sxp, \
                          pci_dict_cmp, PCI_DEVFN, PCI_SLOT, PCI_FUNC, parse_hex

@@ -766,8 +767,20 @@
 
         if self.domid is not None:
             opts = ''
+            optslist = []
+            pci_defopts = []
+            if 'pci_msitranslate' in self.info['platform']:
+                pci_defopts.append(['msitranslate',
+                        str(self.info['platform']['pci_msitranslate'])])
+            if 'pci_power_mgmt' in self.info['platform']:
+                pci_defopts.append(['power_mgmt',
+                        str(self.info['platform']['pci_power_mgmt'])])
             if new_dev.has_key('opts'):
-                opts = ',' + serialise_pci_opts(new_dev['opts'])
+                optslist += new_dev['opts']
+
+            if optslist or pci_defopts:
+                opts = ',' + serialise_pci_opts(
+                       append_default_pci_opts(optslist, pci_defopts))
 
             bdf_str = "%s@%02x%s" % (pci_dict_to_bdf_str(new_dev),
                                      int(new_dev['vdevfn'], 16), opts)

diff -r 683bf4ce2e93 tools/python/xen/xend/server/pciif.py

tools/python/xen/xend/server/pciif.py +14 -5

@@ -97,6 +97,15 @@
         """@see DevController.getDeviceDetails"""
         back = {}
         pcidevid = 0
+        pci_defopts = []
+
+        if 'pci_msitranslate' in self.vm.info['platform']:
+            pci_defopts.append(['msitranslate',
+                    str(self.vm.info['platform']['pci_msitranslate'])])
+        if 'pci_power_mgmt' in self.vm.info['platform']:
+            pci_defopts.append(['power_mgmt',
+                    str(self.vm.info['platform']['pci_power_mgmt'])])
+
         for pci_config in config.get('devs', []):
             domain = parse_hex(pci_config.get('domain', 0))
             bus = parse_hex(pci_config.get('bus', 0))

@@ -105,8 +114,12 @@
             vdevfn = parse_hex(pci_config.get('vdevfn', \
                                               '0x%02x' % AUTO_PHP_SLOT))
 
+            optslist = []
             if pci_config.has_key('opts'):
-                opts = serialise_pci_opts(pci_config['opts'])
+                optslist += pci_config['opts']
+            if optslist or pci_defopts:
+                opts = serialise_pci_opts(
+                       append_default_pci_opts(optslist, pci_defopts))
                 back['opts-%i' % pcidevid] = opts
 
             back['dev-%i' % pcidevid] = "%04x:%02x:%02x.%01x" % \

@@ -118,10 +131,6 @@
 
         back['num_devs']=str(pcidevid)
         back['uuid'] = config.get('uuid','')
-        if 'pci_msitranslate' in self.vm.info['platform']:
-            back['msitranslate']=str(self.vm.info['platform']['pci_msitranslate'])
-        if 'pci_power_mgmt' in self.vm.info['platform']:
-            back['power_mgmt']=str(self.vm.info['platform']['pci_power_mgmt'])
 
         return (0, back, {})
 

tools/python/xen/util/pci.py line 335

Quoted 6 lines

tools/python/xen/util/pci.py +1 -1

@@ -328,7 +332,7 @@
     template['domain'] = "0x%04x" % domain
     template['bus']    = "0x%02x" % int(pci_dev_info['bus'], 16)
     template['slot']   = "0x%02x" % int(pci_dev_info['slot'], 16)
-    template['key']    = pci_dev_str
+    template['key']    = pci_dev_str.split(',')[0]

Splitting on a comma won't work as desired here, as the function portion of multi-function devices may include comas.

Leaving the key as pci_dev_str, is probably ok. Otherwise I suggest constructing the key from template['domain'], template['bus'], template['slot'], pci_dev_infom['func'] and conditionally pci_dev_infom['vdevfn'].

IIRC, it doesn't really matter what the key is, as long as it is unique for a device. Which is why I think that leaving it as pci_dev_str should be ok.

Quoted 10 lines · Simon Horman

tools/python/xen/util/pci.py line 335

Quoted 6 lines

tools/python/xen/util/pci.py +1 -1

@@ -328,7 +332,7 @@
     template['domain'] = "0x%04x" % domain
     template['bus']    = "0x%02x" % int(pci_dev_info['bus'], 16)
     template['slot']   = "0x%02x" % int(pci_dev_info['slot'], 16)
-    template['key']    = pci_dev_str
+    template['key']    = pci_dev_str.split(',')[0]

Splitting on a comma won't work as desired here, as the function portion of multi-function devices may include comas.

Quoted 4 lines

Leaving the key as pci_dev_str, is probably ok. Otherwise I suggest constructing the key from template['domain'], template['bus'], template['slot'], pci_dev_infom['func'] and conditionally pci_dev_infom['vdevfn'].

Thank you for commenting
In fact, changing the key here doesn't explictly impact the option part I'm trying to fix, because ioemu doesn't seem to use backend when creating devices. I just changed it for consistency since I found that options were also attached in key using comma, but I failed to notice its usage as a separator of multi-function devices.

But if comma is also used for multi-function device, can you show me how it looks like? Is there any conflicts between it and the pci options?

Quoted 3 lines

IIRC, it doesn't really matter what the key is, as long as it is unique for a device. Which is why I think that leaving it as pci_dev_str should be ok.

Changing it back doesn't affect the pci options functionally, AFAIK.