Re: [libvirt] [PATCH] qemu: process: Fix automatic setting of locked memory limit for VFIO

For VFIO passthrough the guest memory including the device memory to be

resident in memory. Previously we used a magic constant of 1GiB that we added to the current memory size of the VM to accomodate all this. It is possible though that the device that is passed through exposes more than that. To avoid guessing wrong again in the future set the memory lock limit to unlimited at the point when VFIO will be used.

This problem is similar to the issue where we tried to infer the hard limit for a VM according to it's configuration. This proved to be really problematic so we moved this burden to the user. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1273480 Additionally this patch also fixes a similar bug, where the mlock limit was not increased prior to a memory hotplug operation. https://bugzilla.redhat.com/show_bug.cgi?id=1273491 --- src/qemu/qemu_command.c | 17 +++++------------ src/qemu/qemu_hotplug.c | 20 +++++--------------- src/util/virprocess.c | 3 +++ 3 files changed, 13 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8824541..183aa85 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11448,20 +11448,13 @@ qemuBuildCommandLine(virConnectPtr conn, } if (mlock) { - unsigned long long memKB; - - /* VFIO requires all of the guest's memory to be - * locked resident, plus some amount for IO - * space. Alex Williamson suggested adding 1GiB for IO - * space just to be safe (some finer tuning might be - * nice, though). - */ + /* VFIO requires all of the guest's memory and the IO space of the + * device to be locked resident. Since we can't really know what the + * users decide to plug in we need to set the limit to unlimited. */ if (virMemoryLimitIsSet(def->mem.hard_limit)) - memKB = def->mem.hard_limit; + virCommandSetMaxMemLock(cmd, def->mem.hard_limit << 10); else - memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024; - - virCommandSetMaxMemLock(cmd, memKB * 1024); + virCommandSetMaxMemLock(cmd, ULLONG_MAX); } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MSG_TIMESTAMP) && diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8f2fda9..1cf61ac 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1254,7 +1254,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, bool teardowncgroup = false; bool teardownlabel = false; int backend; - unsigned long long memKB; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; @@ -1279,20 +1278,11 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, goto error; } - /* VFIO requires all of the guest's memory to be locked - * resident (plus an additional 1GiB to cover IO space). During - * hotplug, the guest's memory may already be locked, but it - * doesn't hurt to "change" the limit to the same value. - * NB: the domain's memory tuning parameters are stored as - * Kibibytes, but virProcessSetMaxMemLock expects the value in - * bytes. - */ - if (virMemoryLimitIsSet(vm->def->mem.hard_limit)) - memKB = vm->def->mem.hard_limit; - else - memKB = virDomainDefGetMemoryActual(vm->def) + (1024 * 1024); - - virProcessSetMaxMemLock(vm->pid, memKB * 1024); + /* VFIO requires all of the guest's memory and the IO space of the + * device to be locked resident. Since we can't really know what the + * users decide to plug in we need to set the limit to unlimited. */ + if (!virMemoryLimitIsSet(vm->def->mem.hard_limit)) + virProcessSetMaxMemLock(vm->pid, ULLONG_MAX);

break; default: diff --git a/src/util/virprocess.c b/src/util/virprocess.c index 103c114..92195df 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -749,6 +749,9 @@ virProcessSetMaxMemLock(pid_t pid, unsigned long long bytes) if (bytes == 0) return 0; + if (bytes == ULLONG_MAX) + bytes = RLIM_INFINITY; + rlim.rlim_cur = rlim.rlim_max = bytes; if (pid == 0) { if (setrlimit(RLIMIT_MEMLOCK, &rlim) < 0) {