Xen devel - Mar 2009 - RE: [Xen-changelog] [xen-unstable] xend: specify the slot for pass-through devices

Hi Simon,
Did you actually test your patch and verify it works? -- It doesn''t
work at least in my side.

For a statically assigned device ("a pass-through device that is inserted
at boot time"), needn''t we modify ioemu to handle the vslot info
properly?

BTW,  in the python codes inside tools/python/xen, somewhere we use the stirng
''vslot'', somewhere we use ''vslt''. We may as
well use the same string.
In your patch, tools/python/xen/xend/server/pciif.py:
+            vslot = parse_hex(pci_config.get(''vslot'', 0))   
===> in the newly-added line, vslot is not used later??

And in your patch, you only allow a vslot 0x0~0xf. I think we should support
0x0~0x1f.

Thanks,
-- Dexuan

Xen patchbot-unstable wrote:
> # HG changeset patch
> # User Keir Fraser <keir.fraser@citrix.com>
> # Date 1237550960 0
> # Node ID 538a64b1ed6389da2c669a76ba26a8e4d670351a
> # Parent  2cd2e78d7c2d9deb083f0035df2876805ffeb065
> xend: specify the slot for pass-through devices
> 
> Currently a slot may be specified for a hot-plug device,
> but not for a pass-through device that is inserted at boot time.
> This patch adds support for the latter.
> 
> The syntax is:
>      BUS:DEV.FUNC[@VSLOT]
> e.g: 0000:00:1d:0@7
> 
> This may be important as recent changes that allow any free PCI
> slot to be used for pass-through (and hotplug) may case pass-through
> devices to be assigned in different locations to before. Amongst
> other things, specifying the slot will allow users to move them
> back, if there is a need.
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>
> ---
>  tools/python/xen/xend/server/pciif.py |    1 +
>  tools/python/xen/xm/create.py         |   18 +++++++++++++-----
>  2 files changed, 14 insertions(+), 5 deletions(-)
> 
> diff -r 2cd2e78d7c2d -r 538a64b1ed63
> tools/python/xen/xend/server/pciif.py ---
> a/tools/python/xen/xend/server/pciif.py	Fri Mar 20 10:37:12 2009
> +0000 +++ b/tools/python/xen/xend/server/pciif.py	Fri Mar 20 12:09:20
>              2009 +0000 @@ -75,6 +75,7 @@ class
>              PciController(DevController): bus >             
parse_hex(pci_config.get(''bus'', 0)) slot >
parse_hex(pci_config.get(''slot'', 0)) func >
parse_hex(pci_config.get(''func'', 0)) +            vslot >
parse_hex(pci_config.get(''vslot'', 0))
> 
>              opts = pci_config.get(''opts'',
'''')
>              if len(opts) > 0:
> diff -r 2cd2e78d7c2d -r 538a64b1ed63 tools/python/xen/xm/create.py
> --- a/tools/python/xen/xm/create.py	Fri Mar 20 10:37:12 2009 +0000
> +++ b/tools/python/xen/xm/create.py	Fri Mar 20 12:09:20 2009 +0000
> @@ -1,4 +1,4 @@
>
-#===========================================================================>
+#============================================================================UTO
>  # This library is free software; you can redistribute it and/or
>  # modify it under the terms of version 2.1 of the GNU Lesser General
>  Public # License as published by the Free Software Foundation.
> @@ -32,6 +32,7 @@ from xen.xend import osdep
>  from xen.xend import osdep
>  import xen.xend.XendClient
>  from xen.xend.XendBootloader import bootloader
> +from xen.xend.XendConstants import *
>  from xen.xend.server.DevConstants import xenbusState
>  from xen.util import blkif
>  from xen.util import vscsi_util
> @@ -322,10 +323,12 @@ gopts.var(''disk'',
val=''phy:DEV,VDEV,MODE
>            backend driver domain to use for the disk.
>            The option may be repeated to add more than one
disk.""")
> 
> -gopts.var(''pci'',
> val=''BUS:DEV.FUNC[,msitranslate=0|1][,power_mgmt=0|1]'',
>            +gopts.var(''pci'',
>           
>           
>
val=''BUS:DEV.FUNC[@VSLOT][,msitranslate=0|1][,power_mgmt=0|1]'',
> fn=append_value, default=[], use="""Add a PCI device to a
domain,
>            using given params (in hex). For example
''pci=c0:02.1''. +
>            If VSLOT is supplied the device will be inserted into that
>            +          virtual slot in the guest, else a free slot is
> selected. If msitranslate is set, MSI-INTx translation is enabled if
>      possible. Guest that doesn''t support MSI will get IO-APIC
type
>      IRQs translated from physical MSI, HVM only. Default is 1. @@
>      -696,7 +699,7 @@ def configure_pci(config_devs, vals):
"""Create
> the config for pci devices. """ config_pci = [] -    for
(domain,
> bus, slot, func, opts) in vals.pci: +    for (domain, bus, slot,
>          func, vslot, opts) in vals.pci: config_pci_opts = []
>          d = comma_sep_kv_to_dict(opts)
> 
> @@ -707,7 +710,7 @@ def configure_pci(config_devs, vals):
>              config_pci_opts.append([k, d[k]])
> 
>          config_pci_bdf = [''dev'',
[''domain'', domain], [''bus'', bus], \
> -                          [''slot'', slot],
[''func'', func]]
> +                          [''slot'', slot],
[''func'', func], [''vslot'',
>          vslot]] map(f, d.keys())
>          if len(config_pci_opts)>0:
>              config_pci_bdf.append([''opts'',
config_pci_opts])
> @@ -1054,16 +1057,21 @@ def preprocess_pci(vals):
>                  r"(?P<bus>[0-9a-fA-F]{1,2})[:,]" + \
>                  r"(?P<slot>[0-9a-fA-F]{1,2})[.,]" + \
>                  r"(?P<func>[0-7])" + \
> -                r"(,(?P<opts>.*))?$", pci_dev_str)
> +                r"(@(?P<vslot>[0-9a-fA-F]))?" + \
> +                r"(,(?P<opts>.*))?$", \
> +                pci_dev_str)
>          if pci_match!=None:
>              pci_dev_info = pci_match.groupdict('''')
>              if
pci_dev_info[''domain'']=='''':
>                 
pci_dev_info[''domain'']=''0''
> +            if
pci_dev_info[''vslot'']=='''':
> +                pci_dev_info[''vslot'']="%02x" %
AUTO_PHP_SLOT
>              try:
>                  pci.append(
(''0x''+pci_dev_info[''domain''], \
>                         
''0x''+pci_dev_info[''bus''], \
>                         
''0x''+pci_dev_info[''slot''], \
>                         
''0x''+pci_dev_info[''func''], \
> +                       
''0x''+pci_dev_info[''vslot''], \
>                          pci_dev_info[''opts'']))
>              except IndexError:
>                  err(''Error in PCI slot syntax
"%s"''%(pci_dev_str))
> 
> _______________________________________________
> Xen-changelog mailing list
> Xen-changelog@lists.xensource.com
> http://lists.xensource.com/xen-changelog
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, Mar 25, 2009 at 04:53:07PM +0800, Cui, Dexuan
wrote:
> Hi Simon,
> Did you actually test your patch and verify it works? -- It
doesn''t work at least in my side.
>
> For a statically assigned device ("a pass-through device that is
inserted
> at boot time"), needn''t we modify ioemu to handle the vslot
info
> properly?
There has been an additional patch posted[1] as well as some ioemu
changes[2, 3]. I think you have subsequently found these. Sorry for the
confusion, I did make some miss-steps along the way.
> BTW,  in the python codes inside tools/python/xen, somewhere we use the
> stirng ''vslot'', somewhere we use
''vslt''. We may as well use the same
> string.
Agreed. Though I am working on allowing multi-function devices to be
passed-through as multi-function devices. As part of this change it seems
to be convenient to change vslot to vdevfn. I wonder if at that time
changing thins around to vdevfn would make sense.

I was planing to post multi-function these patches after 3.4 has been
released. Partly to avoid any confusion about what I am submitting
for 3.4. And partly because they aren''t quite finished yet.

If it weren''t for these multi-function changes that I am working on,
I would be in favour on standardising on vslot throughout the code.
And if you want to change it anyway, I have no objections.
> In your patch, tools/python/xen/xend/server/pciif.py: +
> vslot = parse_hex(pci_config.get(''vslot'', 0))    ===>
in the newly-added
> line, vslot is not used later??
This was addressed in subsequent patches[2,3].
> And in your patch, you only allow a vslot 0x0~0xf. I think we should
> support 0x0~0x1f.
Thanks, I''ll get that fixed.
> Thanks,
Thans for your review :-)

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en



[1] [patch] xend: Supply pass-through vslot to qemu-dm
[2] [patch 0/4 v2] ioemu: specify the slot for pass-through devices
[3] [rfc 0/4] ioemu: specify the slot for pass-through devices

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