Re: [libvirt] [PATCH 2/2] nodedev: Expose PCI header type

On Tue, Mar 15, 2016 at 04:37:55PM +0000, Daniel P. Berrange wrote: >On Tue, Mar 15, 2016 at 05:23:30PM +0100, Martin Kletzander wrote: >>If we expose this information, which is one byte in every PCI config >>file, we let all mgmt apps know whether the device itself is an endpoint >>or not so it's easier for them to decide whether such device can be >>passed through into a VM (endpoint) or not (*-bridge). >> >>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1317531 >> >>Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt; >>--- >> docs/schemas/nodedev.rng | 17 ++++++++++ >> src/conf/node_device_conf.c | 37 +++++++++++++++++++++ >> src/conf/node_device_conf.h | 2 ++ >> src/libvirt_private.syms | 3 ++ >> src/node_device/node_device_udev.c | 5 +++ >> src/util/virpci.c | 38 ++++++++++++++++++++++ >> src/util/virpci.h | 12 +++++++ >> .../pci_0000_00_02_0_header_type.xml | 16 +++++++++ >> .../pci_0000_00_1c_0_header_type.xml | 22 +++++++++++++ >> tests/nodedevxml2xmltest.c | 2 ++ >> 10 files changed, 154 insertions(+) >> create mode 100644 tests/nodedevschemadata/pci_0000_00_02_0_header_type.xml >> create mode 100644 tests/nodedevschemadata/pci_0000_00_1c_0_header_type.xml >> >>diff --git a/docs/schemas/nodedev.rng b/docs/schemas/nodedev.rng >>index 744dccdf5fa9..7aec2adf48e9 100644 >>--- a/docs/schemas/nodedev.rng >>+++ b/docs/schemas/nodedev.rng >>@@ -169,6 +169,23 @@ >> </optional> >> >> <optional> >>+ <element name='header'> >>+ <attribute name='type'> >>+ <choice> >>+ <value>endpoint</value> >>+ <value>pci-bridge</value> >>+ <value>cardbus-bridge</value> >>+ </choice> >>+ </attribute> > >We already have nested <capability> elements in the node device >data, and reporting this header type just looks like another >set of capabilities to me. ie <capability type='endpoint'/> > But the devices that this is valid for are exactly only PCI ones. And all PCI ones. That's why I included it in capability type='pci'. It's not capability by itself, it's just information extracted from one Byte in PCI config.

>In any case we already have a > > > <capability type='virt_functions'> > <address domain='0x0000' bus='0x05' slot='0x10' function='0x0'/> > <address domain='0x0000' bus='0x05' slot='0x10' function='0x4'/> > <address domain='0x0000' bus='0x05' slot='0x11' function='0x0'/> > <address domain='0x0000' bus='0x05' slot='0x11' function='0x4'/> > </capability> > >which is reported against physical functions. What's missing >is that we omit the capability entirely if no functions are >currently enabled. We also don't report the total number of >functions that are possible. IMHO we should address it those >problems rather than addign a new element. > The 'multifunction' element I added has nothing to do with virtual functions. It merely indicates that the physical device is supposed to be multifunction; as in there is another device with the same address, but different value of 'function' part of that address.