WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH]xend: pass-through: fix "xm pci-list-assignable-d

To: "Cui, Dexuan" <dexuan.cui@xxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH]xend: pass-through: fix "xm pci-list-assignable-devices' for pv_guest
From: Simon Horman <horms@xxxxxxxxxxxx>
Date: Tue, 28 Jul 2009 20:56:28 +1000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <Keir.Fraser@xxxxxxxxxxxxx>
Delivery-date: Tue, 28 Jul 2009 03:56:59 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <EADF0A36011179459010BDF5142A457501CB84C41C@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <EADF0A36011179459010BDF5142A457501CB84C41C@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
On Tue, Jul 28, 2009 at 05:06:43PM +0800, Cui, Dexuan wrote:
> 
> xc.test_assign_device(dev) can only tell us if dev is assigned to hvm guest;
> if a device is assigned to pv guest, xc.test_assign_device() still tells us
> the device is not assigned yet, as a result, the device still appears in the
> output of "xm pci-list-assignable-devices", and xend would not prevent us from
> assigning the device to a new pv or hvm guest.
> 
> To judge if a device has been assigned to guest, we have to scan xenstore.  
> 
> Thanks,
> -- Dexuan
> 
> xend: pass-through: fix "xm pci-list-assignable-devices' for pv_guest
> 
> xc.test_assign_device(dev) can only tell us if dev is assigned to hvm guest;
> if a device is assigned to pv guest, xc.test_assign_device() still tells us
> the device is not assigned yet, as a result, the device still appears in the
> output of "xm pci-list-assignable-devices", and xend would not prevent us from
> assigning the device to a new pv or hvm guest.
> 
> To judge if a device has been assigned to guest, we have to scan xenstore.
> 
> Signed-off-by: Dexuan Cui <dexuan.cui@xxxxxxxxx>
> 
> diff -r 8af26fef898c tools/python/xen/xend/XendConfig.py
> --- a/tools/python/xen/xend/XendConfig.py     Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/XendConfig.py     Tue Jul 28 16:42:21 2009 +0800
> @@ -2059,9 +2059,6 @@ class XendConfig(dict):
>          return self['platform'].get('hap', 0)
>  
>      def update_platform_pci(self):
> -        if not self.is_hvm():
> -            return
> -
>          pci = []
>          for dev_type, dev_info in self.all_devices_sxpr():
>              if dev_type != 'pci':
> diff -r 8af26fef898c tools/python/xen/xend/XendDomainInfo.py
> --- a/tools/python/xen/xend/XendDomainInfo.py Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/XendDomainInfo.py Tue Jul 28 16:42:21 2009 +0800
> @@ -42,7 +42,7 @@ from xen.util.pci import serialise_pci_o
>  from xen.util.pci import serialise_pci_opts, pci_opts_list_to_sxp, \
>                           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
> +                         pci_dict_cmp, PCI_DEVFN, PCI_SLOT, PCI_FUNC, 
> parse_hex
>  
>  from xen.xend import balloon, sxp, uuid, image, arch
>  from xen.xend import XendOptions, XendNode, XendConfig
> @@ -296,20 +296,11 @@ def dom_get(dom):
>          log.trace("domain_getinfo(%d) failed, ignoring: %s", dom, str(err))
>      return None
>  
> -def get_assigned_pci_devices(domid):
> -    dev_str_list = []
> -    path = '/local/domain/0/backend/pci/%u/0/' % domid
> -    num_devs = xstransact.Read(path + 'num_devs');
> -    if num_devs is None or num_devs == "":
> -        return dev_str_list
> -    num_devs = int(num_devs);
> -    for i in range(num_devs):
> -        dev_str = xstransact.Read(path + 'dev-%i' % i)
> -        dev_str_list = dev_str_list + [dev_str]
> -    return dev_str_list 
> +from xen.xend.server.pciif import parse_pci_name, PciDevice,\
> +    get_assigned_pci_devices, get_all_assigned_pci_devices
> +
>  
>  def do_FLR(domid):
> -    from xen.xend.server.pciif import parse_pci_name, PciDevice
>      dev_str_list = get_assigned_pci_devices(domid)
>  
>      for dev_str in dev_str_list:
> @@ -686,15 +677,14 @@ class XendDomainInfo:
>                      raise VmError("device is already inserted")
>  
>          # Test whether the devices can be assigned with VT-d
> -        bdf = xc.test_assign_device(0, pci_dict_to_xc_str(new_dev))
> -        if bdf != 0:
> -            if bdf == -1:
> -                raise VmError("failed to assign device: maybe the platform"
> -                              " doesn't support VT-d, or VT-d isn't enabled"
> -                              " properly?")
> -            raise VmError("fail to assign device(%s): maybe it has"
> -                          " already been assigned to other domain, or maybe"
> -                          " it doesn't exist." % 
> pci_dict_to_bdf_str(new_dev))
> +        pci_name = '%04x:%02x:%02x.%x' % \
> +          (parse_hex(new_dev['domain']),\
> +           parse_hex(new_dev['bus']),\
> +           parse_hex(new_dev['slot']),\
> +           parse_hex(new_dev['func']))

  You should be able to use pci_name = pci_dict_to_bdf_str(new_dev) here
  instead of the above 5 lines.

> +        if pci_name in get_all_assigned_pci_devices():
> +            raise VmError("failed to assign device %s that has"
> +                          " already been assigned to other domain." % 
> pci_str)

  Should pci_str be pci_name in the line above ?

>  
>          # Here, we duplicate some checkings (in some cases, we mustn't allow
>          # a device to be hot-plugged into an HVM guest) that are also done in
> @@ -732,8 +722,7 @@ class XendDomainInfo:
>          pci_device.devs_check_driver(coassignment_list)
>          assigned_pci_device_str_list = self._get_assigned_pci_devices()
>          for pci_str in coassignment_list:
> -            pci_dev = parse_pci_name(pci_str)
> -            if xc.test_assign_device(0, pci_dict_to_xc_str(pci_dev)) == 0:
> +            if not (pci_str in get_all_assigned_pci_devices()):
>                  continue
>              if not pci_str in assigned_pci_device_str_list:
>                  raise VmError(("pci: failed to pci-attach %s to domain %s" + 
> \
> @@ -859,7 +848,7 @@ class XendDomainInfo:
>          pci_dev = sxp.children(dev_sxp, 'dev')[0]
>          dev_config = pci_convert_sxp_to_dict(dev_sxp)
>          dev = dev_config['devs'][0]
> -                
> +
>          # Do HVM specific processing
>          if self.info.is_hvm():
>              if pci_state == 'Initialising':
> @@ -2454,7 +2443,8 @@ class XendDomainInfo:
>          if pci and len(pci) > 0:
>              pci = map(lambda x: x[0:4], pci)  # strip options 
>              pci_str = str(pci)
> -        if hvm and pci_str:
> +
> +        if hvm and pci_str != '':
>              bdf = xc.test_assign_device(0, pci_str)
>              if bdf != 0:
>                  if bdf == -1:
> @@ -2465,9 +2455,17 @@ class XendDomainInfo:
>                  devfn = (bdf >> 8) & 0xff
>                  dev = (devfn >> 3) & 0x1f
>                  func = devfn & 0x7
> -                raise VmError("fail to assign device(%x:%x.%x): maybe it has"
> +                raise VmError("failed to assign device(%x:%x.%x): maybe it 
> has"
>                                " already been assigned to other domain, or 
> maybe"
>                                " it doesn't exist." % (bus, dev, func))
> +
> +        # This test is done for both pv and hvm guest.
> +        for p in pci:
> +            pci_name = '%04x:%02x:%02x.%x' % \
> +                (parse_hex(p[0]), parse_hex(p[1]), parse_hex(p[2]), 
> parse_hex(p[3]))
> +            if pci_name in get_all_assigned_pci_devices():
> +                raise VmError("failed to assign device %s that has"
> +                              " already been assigned to other domain." % 
> pci_name)
>  
>          # register the domain in the list 
>          from xen.xend import XendDomain
> diff -r 8af26fef898c tools/python/xen/xend/XendNode.py
> --- a/tools/python/xen/xend/XendNode.py       Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/XendNode.py       Tue Jul 28 16:42:21 2009 +0800
> @@ -834,6 +834,9 @@ class XendNode:
>  
>  
>      def pciinfo(self):
> +        from xen.xend.server.pciif import get_all_assigned_pci_devices
> +        assigned_devs = get_all_assigned_pci_devices()
> +
>          # Each element of dev_list is a PciDevice
>          dev_list = PciUtil.find_all_assignable_devices()
>   
> @@ -847,11 +850,7 @@ class XendNode:
>          for dev_list in devs_list:
>              available = True
>              for d in dev_list:
> -                pci_str = '0x%x,0x%x,0x%x,0x%x' %(d.domain, d.bus, d.slot, 
> d.func)
> -                # Xen doesn't care what the domid is, so we pass 0 here...
> -                domid = 0
> -                bdf = self.xc.test_assign_device(domid, pci_str)
> -                if bdf != 0:
> +                if d.name in assigned_devs:

Nice use of d.name :-)

>                      available = False
>                      break
>              if available:
> diff -r 8af26fef898c tools/python/xen/xend/server/pciif.py
> --- a/tools/python/xen/xend/server/pciif.py   Fri Jul 24 12:08:54 2009 +0100
> +++ b/tools/python/xen/xend/server/pciif.py   Tue Jul 28 16:43:49 2009 +0800
> @@ -57,6 +57,25 @@ def parse_hex(val):
>              return val
>      except ValueError:
>          return None
> +
> +def get_assigned_pci_devices(domid):
> +    dev_str_list = []
> +    path = '/local/domain/0/backend/pci/%u/0/' % domid
> +    num_devs = xstransact.Read(path + 'num_devs');
> +    if num_devs is None or num_devs == "":
> +        return dev_str_list
> +    num_devs = int(num_devs)
> +    for i in range(num_devs):
> +        dev_str = xstransact.Read(path + 'dev-%i' % i)
> +        dev_str_list = dev_str_list + [dev_str]
> +    return dev_str_list
> +
> +def get_all_assigned_pci_devices():
> +    dom_list = xstransact.List('/local/domain')
> +    pci_str_list = []
> +    for d in dom_list:
> +        pci_str_list = pci_str_list + get_assigned_pci_devices(int(d))
> +    return pci_str_list
>  
>  class PciController(DevController):
>  
> @@ -368,11 +387,8 @@ class PciController(DevController):
>                      dev.devs_check_driver(funcs)
>                      for f in funcs:
>                          if not f in pci_str_list:
> -                            (f_dom, f_bus, f_slot, f_func) = 
> parse_pci_name(f)
> -                            f_pci_str = '0x%x,0x%x,0x%x,0x%x' % \
> -                                (f_dom, f_bus, f_slot, f_func)
>                              # f has been assigned to other guest?
> -                            if xc.test_assign_device(0, f_pci_str) != 0:
> +                            if f in get_all_assigned_pci_devices():
>                                  err_msg = 'pci: %s must be co-assigned to' + 
> \
>                                      ' the same guest with %s'
>                                  raise VmError(err_msg % (f, dev.name))
> @@ -396,9 +412,8 @@ class PciController(DevController):
>                      dev.devs_check_driver(devs_str)
>                      for s in devs_str:
>                          if not s in pci_str_list:
> -                            s_pci_str = 
> pci_dict_to_bdf_str(parse_pci_name(s))
>                              # s has been assigned to other guest?
> -                            if xc.test_assign_device(0, s_pci_str) != 0:
> +                            if s in get_all_assigned_pci_devices():
>                                  err_msg = 'pci: %s must be co-assigned to 
> the'+\
>                                      ' same guest with %s'
>                                  raise VmError(err_msg % (s, dev.name))
> 
> 

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