Re: [libvirt] [PATCH v4 1/2] qemu_domain: NVLink2 bridge detection function for PPC64

On Tue, Mar 12, 2019 at 18:55:49 -0300, Daniel Henrique Barboza wrote: > The NVLink2 support in QEMU implements the detection of NVLink2 > capable devices by verifying the attributes of the VFIO mem region > QEMU allocates for the NVIDIA GPUs. To properly allocate an > adequate amount of memLock, Libvirt needs this information before > a QEMU instance is even created, thus querying QEMU is not > possible and opening a VFIO window is too much. > > An alternative is presented in this patch. Making the following > assumptions: > > - if we want GPU RAM to be available in the guest, an NVLink2 bridge > must be passed through; > > - an unknown PCI device can be classified as a NVLink2 bridge > if its device tree node has 'ibm,gpu', 'ibm,nvlink', > 'ibm,nvlink-speed' and 'memory-region'. > > This patch introduces a helper called @ppc64VFIODeviceIsNV2Bridge > that checks the device tree node of a given PCI device and > check if it meets the criteria to be a NVLink2 bridge. This > new function will be used in a follow-up patch that, using the > first assumption, will set up the rlimits of the guest > accordingly. > > Signed-off-by: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; > --- > src/qemu/qemu_domain.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) disclaimer: This is a code review. I don't have much knowledge about what you are trying to implement. > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 1659e88478..dcc92d253c 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -10398,6 +10398,35 @@ qemuDomainUpdateCurrentMemorySize(virDomainObjPtr vm) > } > > > +/** > + * ppc64VFIODeviceIsNV2Bridge: > + * @device: string with the PCI device address > + * > + * This function receives a string that represents a PCI device, > + * such as '0004:04:00.0', and tells if the device is a NVLink2 > + * bridge. > + */ > +static bool > +ppc64VFIODeviceIsNV2Bridge(const char *device) > +{ > + const char *nvlink2Files[] = {"ibm,gpu", "ibm,nvlink", > + "ibm,nvlink-speed", "memory-region"}; > + char *file; > + size_t i; > + > + for (i = 0; i < 4; i++) { Use ARRAY_CARDINALITY(nvlink2Files) instead of '4'.

> + if ((virAsprintf(&file, "/sys/bus/pci/devices/%s/of_node/%s", > + device, nvlink2Files[i])) < 0) > + return false; > + > + if (!virFileExists(file)) > + return false; memory allocated to 'file' is leaked in every loop and also at exit. You can use VIR_AUTOFREE(char *) file above, but you need to free the pointer also in every single loop. > + } > + > + return true; > +} This function is unused and static, this means the build will fail after this patch.

> + > + > /** > * getPPC64MemLockLimitBytes: > * @def: domain definition > -- > 2.20.1 > > -- > libvir-list mailing list > libvir-list(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list