[PATCH v4 5/5] backends/hostmem: Report error when memory size is unaligned

Michal Privoznik posted 5 patches 5 months, 3 weeks ago
Maintainers: David Hildenbrand <david@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
[PATCH v4 5/5] backends/hostmem: Report error when memory size is unaligned
Posted by Michal Privoznik 5 months, 3 weeks ago
If memory-backend-{file,ram} has a size that's not aligned to
underlying page size it is not only wasteful, but also may lead
to hard to debug behaviour. For instance, in case
memory-backend-file and hugepages, madvise() and mbind() fail.
Rightfully so, page is the smallest unit they can work with. And
even though an error is reported, the root cause it not very
clear:

  qemu-system-x86_64: Couldn't set property 'dump' on 'memory-backend-file': Invalid argument

After this commit:

  qemu-system-x86_64: backend 'memory-backend-file' memory size must be multiple of 2 MiB

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Mario Casquero <mcasquer@redhat.com>
---
 backends/hostmem.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 012a8c190f..4d6c69fe4d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -20,6 +20,7 @@
 #include "qom/object_interfaces.h"
 #include "qemu/mmap-alloc.h"
 #include "qemu/madvise.h"
+#include "qemu/cutils.h"
 #include "hw/qdev-core.h"
 
 #ifdef CONFIG_NUMA
@@ -337,6 +338,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
     HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
     void *ptr;
     uint64_t sz;
+    size_t pagesize;
     bool async = !phase_check(PHASE_LATE_BACKENDS_CREATED);
 
     if (!bc->alloc) {
@@ -348,6 +350,14 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
 
     ptr = memory_region_get_ram_ptr(&backend->mr);
     sz = memory_region_size(&backend->mr);
+    pagesize = qemu_ram_pagesize(backend->mr.ram_block);
+
+    if (!QEMU_IS_ALIGNED(sz, pagesize)) {
+        g_autofree char *pagesize_str = size_to_str(pagesize);
+        error_setg(errp, "backend '%s' memory size must be multiple of %s",
+                   object_get_typename(OBJECT(uc)), pagesize_str);
+        return;
+    }
 
     if (backend->merge &&
         qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {
-- 
2.44.1


Re: [PATCH v4 5/5] backends/hostmem: Report error when memory size is unaligned
Posted by Mario Casquero 5 months, 3 weeks ago
This patch has been successfully tested. Try to allocate some 2M
hugepages in the host, then boot up a VM with the memory size
unaligned and backed by a file, QEMU prompts the following message:
qemu-system-x86_64: backend 'memory-backend-file' memory size must be
multiple of 2 MiB

Tested-by: Mario Casquero <mcasquer@redhat.com>

On Wed, Jun 5, 2024 at 12:45 PM Michal Privoznik <mprivozn@redhat.com> wrote:
>
> If memory-backend-{file,ram} has a size that's not aligned to
> underlying page size it is not only wasteful, but also may lead
> to hard to debug behaviour. For instance, in case
> memory-backend-file and hugepages, madvise() and mbind() fail.
> Rightfully so, page is the smallest unit they can work with. And
> even though an error is reported, the root cause it not very
> clear:
>
>   qemu-system-x86_64: Couldn't set property 'dump' on 'memory-backend-file': Invalid argument
>
> After this commit:
>
>   qemu-system-x86_64: backend 'memory-backend-file' memory size must be multiple of 2 MiB
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Mario Casquero <mcasquer@redhat.com>
> ---
>  backends/hostmem.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index 012a8c190f..4d6c69fe4d 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -20,6 +20,7 @@
>  #include "qom/object_interfaces.h"
>  #include "qemu/mmap-alloc.h"
>  #include "qemu/madvise.h"
> +#include "qemu/cutils.h"
>  #include "hw/qdev-core.h"
>
>  #ifdef CONFIG_NUMA
> @@ -337,6 +338,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>      HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
>      void *ptr;
>      uint64_t sz;
> +    size_t pagesize;
>      bool async = !phase_check(PHASE_LATE_BACKENDS_CREATED);
>
>      if (!bc->alloc) {
> @@ -348,6 +350,14 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>
>      ptr = memory_region_get_ram_ptr(&backend->mr);
>      sz = memory_region_size(&backend->mr);
> +    pagesize = qemu_ram_pagesize(backend->mr.ram_block);
> +
> +    if (!QEMU_IS_ALIGNED(sz, pagesize)) {
> +        g_autofree char *pagesize_str = size_to_str(pagesize);
> +        error_setg(errp, "backend '%s' memory size must be multiple of %s",
> +                   object_get_typename(OBJECT(uc)), pagesize_str);
> +        return;
> +    }
>
>      if (backend->merge &&
>          qemu_madvise(ptr, sz, QEMU_MADV_MERGEABLE)) {
> --
> 2.44.1
>
>