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.derr...@intel.com> > --- > 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, 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. : 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 firstname.lastname@example.org https://www.redhat.com/mailman/listinfo/libvir-list