On Tue, Dec 01, 2009 at 12:03:49PM -0500, Dave Allan wrote:
> Daniel P. Berrange wrote:
> >On Tue, Dec 01, 2009 at 10:33:08AM -0500, Dave Allan wrote:
> >>Attached is a patch that exposes the relationships between physical and
> >>virtual functions on SR IOV capable devices.
> >>
> >
> >
> >>+ if (data->pci_dev.physical_function) {
> >>+ virBufferEscapeString(&buf, "
> >><physical_function>%s</physical_function>\n",
> >>+ data->pci_dev.physical_function);
> >>+ }
> >>+ if (data->pci_dev.num_virtual_functions > 0) {
> >>+ for (i = 0 ; i < data->pci_dev.num_virtual_functions ;
> >>i++) {
> >>+ virBufferEscapeString(&buf, "
> >><virtual_function>%s</virtual_function>\n",
> >>+
> >>data->pci_dev.virtual_functions[i]);
> >>+ }
> >>+ }
> >
> >
> >What is the actual content of those two new elements ? Are they
> >the names of the corresponding device, suitble for a
> >virNodeDevLooupByName() ?
>
> They are the PCI device BDF or config address, e.g.:
>
> <virtual_function>0000:01:10.0</virtual_function>
>
> so, no, they aren't suitable for lookup by name. Using the names of the
> corresponding devices was my first attempt, but there is a dependency
> problem there. At the time that we process one device, the others
> aren't necessarily created in libvirt, so it's not possible to look them
> up. If we wanted to go that route, we'd have to do additional work at
> the time of dumping the xml, which I think is a little kludgy. I'd
> rather we created an API to lookup a libvirt device by its BDF.
I don't much like including the raw BDF format, because it is effectively
adding a 3rd way of specifying PCI addresses in libvirt XML. We already
have 2 different ways, which is 1 too many
Node dev XML
<capability type='pci'>
<domain>0</domain>
<bus>0</bus>
<slot>31</slot>
<function>1</function>
</capability>
Domain XML for host devices & guest addresses
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x5'/>
Yes, my bad for screwing up the nodedev XML format when I designed
it, stupidly choosing decimal instead of hex :-(
I think we should make the virtual/physical function follow the domain
XML style, with an <address/> element. Perhaps also use nested capabilities
in the way we did for NPIV host/vport stuff
<capability type='physical_function'>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x1'/>
</capability>
<capability type='virtual_functions'>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x3'/>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x4'/>
</capability>
I'd even be inclined to retro-fit the existing <capability type='pci'>
XML to duplicate the address info in this saner format eg
<capability type='pci'>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x1' />
<domain>0</domain>
<bus>0</bus>
<slot>31</slot>
<function>1</function>
</capability>
So, a PCI card with SR-IOV would end up looking like
<capability type='pci'>
<domain>0</domain>
<bus>0</bus>
<slot>31</slot>
<function>1</function>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x1' />
<capability type='virtual_functions'>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x3'/>
<address domain='0x0000' bus='0x00' slot='0x1f' function='0x4'/>
</capability>
</capability>
The nice thing about this kind of nesting is that it would let us show
that a card is capable of exposing virtual functions, even if it does
not currently have any enabled
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
