[libvirt] [PATCH 0/3] qemu: Tweak VFIO limit documentation
Peter Krempa (3): qemu: domain: Restructurate control flow in qemuDomainGetMlockLimitBytes qemu: Explain mlock limit size more in detail qemu: Explain why mlock size is modified when def->mem.locked is enabled src/qemu/qemu_domain.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) -- 2.6.2
Break early when hard limit is set so that it's not intermixed by other logic for determining the limit. --- src/qemu/qemu_domain.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 416ab5b..12517a5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3633,14 +3633,18 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def) { unsigned long long memKB; + /* prefer the hard limit */ + if (virMemoryLimitIsSet(def->mem.hard_limit)) { + memKB = def->mem.hard_limit; + goto done; + } + /* 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). */ - if (virMemoryLimitIsSet(def->mem.hard_limit)) - memKB = def->mem.hard_limit; - else - memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024; + memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024; + done: return memKB << 10; } -- 2.6.2
On Wed, 2015-11-11 at 06:57 +0100, Peter Krempa wrote:
Break early when hard limit is set so that it's not intermixed by other logic for determining the limit. --- src/qemu/qemu_domain.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 416ab5b..12517a5 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3633,14 +3633,18 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def) { unsigned long long memKB;
+ /* prefer the hard limit */ + if (virMemoryLimitIsSet(def->mem.hard_limit)) { + memKB = def->mem.hard_limit; + goto done; + } + /* 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). */ - if (virMemoryLimitIsSet(def->mem.hard_limit)) - memKB = def->mem.hard_limit; - else - memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024; + memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
+ done: return memKB << 10; }
ACK - I have the same diff lined up along with the ppc64-specific algorithm :) -- Andrea Bolognani Software Engineer - Virtualization Team
Based on Alex's explanation [1] in the recent discussion let's update the comment explaining the memory lock limit calculation. [1] http://www.redhat.com/archives/libvir-list/2015-November/msg00329.html --- src/qemu/qemu_domain.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 12517a5..11cd39e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3639,9 +3639,22 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def) goto done; } - /* 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). */ + /* For device pasthrough using VFIO the guest memory and MMIO memory regions + * need to be locked persistent in order to allow DMA. + * + * Currently the below limit is based on assumptions about the x86 platform. + * + * The chosen value of 1GiB below originates from x86 systems where it was + * the used as space reserved for the MMIO region for the whole system. + * + * On x86_64 systems the MMIO regions of the IOMMU mapped devices don't + * count towards the locked memory limit since the memory is owned by the + * device. Emulated devices though do count, but the regions are usually + * small. Although it's not guaranteed that the limit will be enough for all + * configurations it didn't pose a problem for now. + * + * Note that this may not be valid for all platforms. + */ memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024; done: -- 2.6.2
On Wed, 2015-11-11 at 06:57 +0100, Peter Krempa wrote:
Based on Alex's explanation [1] in the recent discussion let's update the comment explaining the memory lock limit calculation.
[1] http://www.redhat.com/archives/libvir-list/2015-November/msg00329.html
I'd rather have the reference to Alex's explanation as part of the comment itself, so that it's easier to look up later without having to dig through the git log, but that's just my personal preference.
--- src/qemu/qemu_domain.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 12517a5..11cd39e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3639,9 +3639,22 @@ qemuDomainGetMlockLimitBytes(virDomainDefPtr def) goto done; }
- /* 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). */ + /* For device pasthrough using VFIO the guest memory and MMIO memory regions
s/pasthrough/passthrough/
+ * need to be locked persistent in order to allow DMA. + * + * Currently the below limit is based on assumptions about the x86 platform. + * + * The chosen value of 1GiB below originates from x86 systems where it was + * the used as space reserved for the MMIO region for the whole system.
s/the used/used/
+ * + * On x86_64 systems the MMIO regions of the IOMMU mapped devices don't + * count towards the locked memory limit since the memory is owned by the + * device. Emulated devices though do count, but the regions are usually + * small. Although it's not guaranteed that the limit will be enough for all + * configurations it didn't pose a problem for now. + * + * Note that this may not be valid for all platforms. + */ memKB = virDomainDefGetMemoryActual(def) + 1024 * 1024;
done:
ACK with the typos fixed. Cheers. -- Andrea Bolognani Software Engineer - Virtualization Team
On Wed, Nov 11, 2015 at 15:23:55 +0100, Andrea Bolognani wrote:
On Wed, 2015-11-11 at 06:57 +0100, Peter Krempa wrote:
Based on Alex's explanation [1] in the recent discussion let's update the comment explaining the memory lock limit calculation.
[1] http://www.redhat.com/archives/libvir-list/2015-November/msg00329.html
I'd rather have the reference to Alex's explanation as part of the comment itself, so that it's easier to look up later without having to dig through the git log, but that's just my personal preference.
It makes sense. I've added it to the commit message.
--- src/qemu/qemu_domain.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)
ACK with the typos fixed.
Fixed the typos and pushed. Thanks. Peter
--- src/qemu/qemu_domain.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 11cd39e..7f2783b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3673,6 +3673,11 @@ qemuDomainRequiresMlock(virDomainDefPtr def) { size_t i; + /* Note that we do assume that the locked memory limit needs to be + * automatically adjusted during memory hotplug as it would cause a + * regression in behavior. The behavior was caused by improperly reverting + * all functionality depending on qemuDomainMemoryLimit in commit 16bcb3b616 + */ if (def->mem.locked) return true; -- 2.6.2
participants (2)
-
Andrea Bolognani -
Peter Krempa