diff mbox series

[v2,1/3] exec: fetch the alignment of Linux devdax pmem character device nodes

Message ID 20200415033538.43329-2-jingqi.liu@intel.com
State New
Headers show
Series fetch the alignment of device dax | expand

Commit Message

Liu, Jingqi April 15, 2020, 3:35 a.m. UTC
If the backend file is devdax pmem character device, the alignment
specified by the option 'align=NUM' in the '-object memory-backend-file'
needs to match the alignment requirement of the devdax pmem character device.

This patch uses the interfaces of libdaxctl to fetch the devdax pmem file
'align', so that we can compare it with the NUM of 'align=NUM'.
The NUM needs to be larger than or equal to the devdax pmem file 'align'.

It also fixes the problem that mmap() returns failure in qemu_ram_mmap()
when the NUM of 'align=NUM' is less than the devdax pmem file 'align'.

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Jingqi Liu <jingqi.liu@intel.com>
---
 exec.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

Comments

Joao Martins April 28, 2020, 4:22 p.m. UTC | #1
On 4/15/20 4:35 AM, Jingqi Liu wrote:
> If the backend file is devdax pmem character device, the alignment
> specified by the option 'align=NUM' in the '-object memory-backend-file'
> needs to match the alignment requirement of the devdax pmem character device.
> 
> This patch uses the interfaces of libdaxctl to fetch the devdax pmem file
> 'align', so that we can compare it with the NUM of 'align=NUM'.
> The NUM needs to be larger than or equal to the devdax pmem file 'align'.
> 
> It also fixes the problem that mmap() returns failure in qemu_ram_mmap()
> when the NUM of 'align=NUM' is less than the devdax pmem file 'align'.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Jingqi Liu <jingqi.liu@intel.com>

  Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

I also no longer see the previous problem we talked in v1 wrt to ABI differences
between dax_pmem_compat (your case) vs dax_pmem|dax_hmem (my case). The @align
validation also occurs as expected.

> ---
>  exec.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 53 insertions(+), 1 deletion(-)
> 
> diff --git a/exec.c b/exec.c
> index de9d949902..2c3444e47e 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -77,6 +77,10 @@
>  
>  #include "monitor/monitor.h"
>  
> +#ifdef CONFIG_LIBDAXCTL
> +#include <daxctl/libdaxctl.h>
> +#endif
> +
>  //#define DEBUG_SUBPAGE
>  
>  #if !defined(CONFIG_USER_ONLY)
> @@ -1736,6 +1740,46 @@ static int64_t get_file_size(int fd)
>      return size;
>  }
>  
> +static int64_t get_file_align(int fd)
> +{
> +    int64_t align = -1;
> +#if defined(__linux__) && defined(CONFIG_LIBDAXCTL)
> +    struct stat st;
> +
> +    if (fstat(fd, &st) < 0) {
> +        return -errno;
> +    }
> +
> +    /* Special handling for devdax character devices */
> +    if (S_ISCHR(st.st_mode)) {
> +        g_autofree char *path = NULL;
> +        g_autofree char *rpath = NULL;
> +        struct daxctl_ctx *ctx;
> +        struct daxctl_region *region;
> +        int rc = 0;
> +
> +        path = g_strdup_printf("/sys/dev/char/%d:%d",
> +                    major(st.st_rdev), minor(st.st_rdev));
> +        rpath = realpath(path, NULL);
> +
> +        rc = daxctl_new(&ctx);
> +        if (rc) {
> +            return -1;
> +        }
> +
> +        daxctl_region_foreach(ctx, region) {
> +            if (strstr(rpath, daxctl_region_get_path(region))) {
> +                align = daxctl_region_get_align(region);
> +                break;
> +            }
> +        }
> +        daxctl_unref(ctx);
> +    }
> +#endif /* defined(__linux__) && defined(CONFIG_LIBDAXCTL) */
> +
> +    return align;
> +}
> +
>  static int file_ram_open(const char *path,
>                           const char *region_name,
>                           bool *created,
> @@ -2275,7 +2319,7 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
>  {
>      RAMBlock *new_block;
>      Error *local_err = NULL;
> -    int64_t file_size;
> +    int64_t file_size, file_align;
>  
>      /* Just support these ram flags by now. */
>      assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0);
> @@ -2311,6 +2355,14 @@ RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
>          return NULL;
>      }
>  
> +    file_align = get_file_align(fd);
> +    if (file_align > 0 && mr && file_align > mr->align) {
> +        error_setg(errp, "backing store align 0x%" PRIx64
> +                   " is larger than 'align' option 0x" RAM_ADDR_FMT,
> +                   file_align, mr->align);
> +        return NULL;
> +    }
> +
>      new_block = g_malloc0(sizeof(*new_block));
>      new_block->mr = mr;
>      new_block->used_length = size;
>
diff mbox series

Patch

diff --git a/exec.c b/exec.c
index de9d949902..2c3444e47e 100644
--- a/exec.c
+++ b/exec.c
@@ -77,6 +77,10 @@ 
 
 #include "monitor/monitor.h"
 
+#ifdef CONFIG_LIBDAXCTL
+#include <daxctl/libdaxctl.h>
+#endif
+
 //#define DEBUG_SUBPAGE
 
 #if !defined(CONFIG_USER_ONLY)
@@ -1736,6 +1740,46 @@  static int64_t get_file_size(int fd)
     return size;
 }
 
+static int64_t get_file_align(int fd)
+{
+    int64_t align = -1;
+#if defined(__linux__) && defined(CONFIG_LIBDAXCTL)
+    struct stat st;
+
+    if (fstat(fd, &st) < 0) {
+        return -errno;
+    }
+
+    /* Special handling for devdax character devices */
+    if (S_ISCHR(st.st_mode)) {
+        g_autofree char *path = NULL;
+        g_autofree char *rpath = NULL;
+        struct daxctl_ctx *ctx;
+        struct daxctl_region *region;
+        int rc = 0;
+
+        path = g_strdup_printf("/sys/dev/char/%d:%d",
+                    major(st.st_rdev), minor(st.st_rdev));
+        rpath = realpath(path, NULL);
+
+        rc = daxctl_new(&ctx);
+        if (rc) {
+            return -1;
+        }
+
+        daxctl_region_foreach(ctx, region) {
+            if (strstr(rpath, daxctl_region_get_path(region))) {
+                align = daxctl_region_get_align(region);
+                break;
+            }
+        }
+        daxctl_unref(ctx);
+    }
+#endif /* defined(__linux__) && defined(CONFIG_LIBDAXCTL) */
+
+    return align;
+}
+
 static int file_ram_open(const char *path,
                          const char *region_name,
                          bool *created,
@@ -2275,7 +2319,7 @@  RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
 {
     RAMBlock *new_block;
     Error *local_err = NULL;
-    int64_t file_size;
+    int64_t file_size, file_align;
 
     /* Just support these ram flags by now. */
     assert((ram_flags & ~(RAM_SHARED | RAM_PMEM)) == 0);
@@ -2311,6 +2355,14 @@  RAMBlock *qemu_ram_alloc_from_fd(ram_addr_t size, MemoryRegion *mr,
         return NULL;
     }
 
+    file_align = get_file_align(fd);
+    if (file_align > 0 && mr && file_align > mr->align) {
+        error_setg(errp, "backing store align 0x%" PRIx64
+                   " is larger than 'align' option 0x" RAM_ADDR_FMT,
+                   file_align, mr->align);
+        return NULL;
+    }
+
     new_block = g_malloc0(sizeof(*new_block));
     new_block->mr = mr;
     new_block->used_length = size;