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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 17, 2016 at 10:00:36AM +0000, Daniel P. Berrange wrote:
On Wed, Mar 16, 2016 at 10:46:10AM +0100, Martin Kletzander wrote:
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@xxxxxxxxxx>
>>---
>> 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.

We have nested capabilities already. This would be a nested capability
below the main PCI capability, which scopes it to just PCI devices.

>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.

That doesn't really seem to add any value then since an application can
already see there are multiple functions in the same slot via the address
info we provide


If that's always the case, then yes.  And I don't know about any
occasion when that wouldn't be true, so I'll remove that in v2.


Regards,
Daniel
--
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]