Hi Laine,
Thanks for the thoughtful reply.
On 08/08/2017 05:35 PM, Laine Stump wrote:
> On 08/07/2017 03:46 PM, Jon Derrick wrote:
>> VMD domains start at 0x10000, so expand dev->name to fit at least this > many characters. > > Signed-off-by: Jon Derrick
> <jonathan.derrick@xxxxxxxxx> > --- > src/util/virpci.c | 2 +- > 1 file
> changed, 1 insertion(+), 1 deletion(-) > > diff --git
> a/src/util/virpci.c b/src/util/virpci.c > index 2c1b758..b3afefe 100644
>> --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -50,7 +50,7 @@
> VIR_LOG_INIT("util.pci"); > > #define PCI_SYSFS "/sys/bus/pci/" >
> #define PCI_ID_LEN 10 /* "XXXX XXXX" */ > -#define PCI_ADDR_LEN 13 /*
> "XXXX:XX:XX.X" */ > +#define PCI_ADDR_LEN 14 /* "XXXXX:XX:XX.X" */ > >
> VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST, > "", "2.5",
> "5", "8", "16")
> Does just this change by itself enable new functionality? Or are other
> changes required? (e.g. the type "pciDomain" in the XML schema is a
> uint16, so any domain > 0xFFFF in the config would fail validation).
It doesn't actually enable new functionality. The VMD product itself
doesn't currently support passthrough of child devices, so a 32-bit
domain will never be bound to a VM. This is being ensured by a kernel
patch effort[1], so this patch can wait until those land.
Onto other points, I see that pciDomain is uint16_t, however with my
patch I am able to return the following (which I'm not sure if it means
it still needs changing or not):
<device>
<name>pci_10000_11_00_0</name>
<path>/sys/devices/pci0000:5d/0000:5d:05.5/pci10000:00/10000:00:00.0/10000:01:00.0/10000:02:07.0/10000:06:00.0/10000:07:10.0/10000:11:00.0</path>
<parent>pci_10000_07_10_0</parent>
<driver>
<name>nvme</name>
</driver>
<capability type='pci'>
<domain>65536</domain>
<bus>17</bus>
<slot>0</slot>
<function>0</function>
<product id='0x0953'>PCIe Data Center SSD</product>
<vendor id='0x8086'>Intel Corporation</vendor>
<numa node='0'/>
<pci-express>
<link validity='cap' port='0' speed='8' width='4'/>
<link validity='sta' speed='8' width='4'/>
</pci-express>
</capability>
</device>
However the main goal of the patch was to squash the following warning
and potential issue:
Feb 23 21:16:24 localhost journal: internal error: dev->name buffer
overflow: 10000:00:00.0
If you would prefer, I can resend the patch with the above error
pointing out that the patch fixes this issue alone.
[1]: https://patchwork.ozlabs.org/patch/798846/
>
> Assuming that the VMD domain needs to be referenced in config somewhere
> in order to be useful, along with changing the type for pciDomain in
> docs/schemes/basictypes.rng, we would also need at least one new test
> case for the qemuxml2argv and qemuxml2xml tests (see the examples in the
> "hostdev-vfio-multidomain" and "net-hostdev-multidomain").
>
I'd be happy to look into this when, if ever, we do actually support
passthrough of VMD child devices within the kernel.
> Also, do all versions of qemu support domains > 0xFFFF? If not, is there
> a feature that can be used to detect that support so we can have a
> capability bit for it and warn if someone tries to use such a domain
> when the installed version of qemu doesn't support it? (If there is no
> way to tell in advance, then we'll just have to live with reporting any
> qemu error after the fact)
>
>
At a first glance, QEMU somewhat supports it. Several domain references
use int, but others use uint16_t. I can take a look at cleaning this up,
but as stated above, VMD child devices won't actually be passed-through
in the foreseeable future. In the past I have done QEMU passthrough
tests with just the VMD endpoint (which resides on a 16-bit domain). The
guest is then responsible for child devices. This mode of operation is
the only one we officially support.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
