Re: [PATCH] backends/hostmem: Fix support of memory-backend-memfd in qemu_maxrampagesize()

[Thomas Huth]

On 10/08/2022 09.32, David Hildenbrand wrote:
> On 10.08.22 08:32, Thomas Huth wrote:
> > It is currently not possible yet to use "memory-backend-memfd" on s390x
> > with hugepages enabled. This problem is caused by qemu_maxrampagesize()
> > not taking memory-backend-memfd objects into account yet, so the code
> > in s390_memory_init() fails to enable the huge page support there via
> > s390_set_max_pagesize(). Fix it by looking at the memory-backend-memfd
> > in the host_memory_backend_pagesize() function, too.
> >
> > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2116496
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   include/sysemu/hostmem.h |  8 +++++++-
> >   backends/hostmem-memfd.c |  2 --
> >   backends/hostmem.c       | 27 +++++++++++++++++----------
> >   3 files changed, 24 insertions(+), 13 deletions(-)
> >
> > diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> > index 9ff5c16963..d983ae6c01 100644
> > --- a/include/sysemu/hostmem.h
> > +++ b/include/sysemu/hostmem.h
> > @@ -34,10 +34,16 @@ OBJECT_DECLARE_TYPE(HostMemoryBackend, 
> > HostMemoryBackendClass,
> >   /* hostmem-file.c */
> >   /**
> >    * @TYPE_MEMORY_BACKEND_FILE:
> > - * name of backend that uses mmap on a file descriptor
> > + * name of backend that uses mmap on a file
> >    */
> >   #define TYPE_MEMORY_BACKEND_FILE "memory-backend-file"
> >   
> > +/* hostmem-memfd.c */
> > +/**
> > + * @TYPE_MEMORY_BACKEND_MEMFD:
> > + * name of backend that uses mmap on a memfd file descriptor
> > + */
> > +#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
> >   
> >   /**
> >    * HostMemoryBackendClass:
> > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > index 3fc85c3db8..1ab2085e49 100644
> > --- a/backends/hostmem-memfd.c
> > +++ b/backends/hostmem-memfd.c
> > @@ -18,8 +18,6 @@
> >   #include "qapi/error.h"
> >   #include "qom/object.h"
> >   
> > -#define TYPE_MEMORY_BACKEND_MEMFD "memory-backend-memfd"
> > -
> >   OBJECT_DECLARE_SIMPLE_TYPE(HostMemoryBackendMemfd, MEMORY_BACKEND_MEMFD)
> >   
> >   
> > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > index 624bb7ecd3..ebce887105 100644
> > --- a/backends/hostmem.c
> > +++ b/backends/hostmem.c
> > @@ -306,22 +306,29 @@ bool host_memory_backend_is_mapped(HostMemoryBackend 
> > *backend)
> >       return backend->is_mapped;
> >   }
> >   
> > -#ifdef __linux__
> >   size_t host_memory_backend_pagesize(HostMemoryBackend *memdev)
> >   {
> > +    size_t pagesize = 0;
> > +
> > +#ifdef __linux__
> >       Object *obj = OBJECT(memdev);
> > -    char *path = object_property_get_str(obj, "mem-path", NULL);
> > -    size_t pagesize = qemu_mempath_getpagesize(path);
> >   
> > -    g_free(path);
> > +    if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND_FILE)) {
> > +        char *path = object_property_get_str(obj, "mem-path", NULL);
> > +        pagesize = qemu_mempath_getpagesize(path);
> > +        g_free(path);
> > +    } else if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND_MEMFD) &&
> > +               object_property_get_bool(obj, "hugetlb", &error_abort)) {
> > +        pagesize = object_property_get_int(obj, "hugetlbsize", &error_abort);
> > +    }
> > +#endif
>
> Why can't we simply rely on
>
> qemu_ram_pagesize(memdev->mr.ram_block);

Good idea! That way, we could even get rid of the "#ifdef __linux__" macros 
here, I guess ... I'll give it a try and send a v2 if it works.

 Thomas