[Xen-devel] Re: [PATCH] xend: fix options for assigned pci

[Simon Horman <horms@xxxxxxxxxxxx>]

On Fri, Jan 08, 2010 at 04:29:04PM +0800, Qing He wrote:
> 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 <horms@xxxxxxxxxxxx>
> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Signed-off-by: Qing He <qing.he@xxxxxxxxx>
> ---
> diff -r 683bf4ce2e93 tools/python/xen/util/pci.py
> --- a/tools/python/xen/util/pci.py    Mon Jan 04 10:35:16 2010 +0000
> +++ b/tools/python/xen/util/pci.py    Fri Jan 08 16:01:55 2010 +0800
> @@ -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})
>  
> @@ -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.

>      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
> --- a/tools/python/xen/xend/XendDomainInfo.py Mon Jan 04 10:35:16 2010 +0000
> +++ b/tools/python/xen/xend/XendDomainInfo.py Fri Jan 08 16:01:55 2010 +0800
> @@ -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
> --- a/tools/python/xen/xend/server/pciif.py   Mon Jan 04 10:35:16 2010 +0000
> +++ b/tools/python/xen/xend/server/pciif.py   Fri Jan 08 16:01:55 2010 +0800
> @@ -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, {})
>  

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