Cui, Dexuan
2009-Mar-25 08:53 UTC
[Xen-devel] 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
Simon Horman
2009-Mar-25 21:29 UTC
Re: [Xen-devel] RE: [Xen-changelog] [xen-unstable] xend: specify the slot for pass-through devices
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