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@verge.net.au>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Qing He <qing.he@intel.com>
---
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]
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@lists.xensource.com
http://lists.xensource.com/xen-devel
Simon Horman
2010-Jan-11 07:16 UTC
[Xen-devel] Re: [PATCH] xend: fix options for assigned pci
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@verge.net.au> > Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > Signed-off-by: Qing He <qing.he@intel.com> > --- > 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@lists.xensource.com http://lists.xensource.com/xen-devel
On Mon, 2010-01-11 at 15:16 +0800, Simon Horman wrote:> > @@ -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''].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?> > 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. Thanks, Qing _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Simon Horman
2010-Jan-11 07:54 UTC
[Xen-devel] Re: [PATCH] xend: fix options for assigned pci
On Mon, Jan 11, 2010 at 03:24:59PM +0800, Qing He wrote:> On Mon, 2010-01-11 at 15:16 +0800, Simon Horman wrote: > > > @@ -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'']. > > 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?I don''t think there are any conflicts, because the function can''t include ''[a-z]'' and all options start with ''[a-z]'' I documented the syntax on http://wiki.xensource.com/xenwiki/BDFNotation Basically where the function used to go, it may optionally be a comma-delimited list of: * Function unit * A range of function numbers, denoted by: + The first function unit + A hyphen (-) + The last function unit * An asterisk (*), used to denote all physical functions that are present in the device Where a function unit comprises of: * Function number and optionally; * An equals sign and; * A virtual function number to use> > 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.Excellent. Lets do that. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel