diff mbox

[v4] ivshmem: allow the sharing of hugepages

Message ID 017e01d1003b$959bc820$c0d35860$@samsung.com
State New
Headers show

Commit Message

Pavel Fedin Oct. 6, 2015, 1:33 p.m. UTC 
This patch permits to share memory areas that do not specifically belong to
/dev/shm. In such case, the file must be already present when launching qemu.
A new parameter 'file' has been added to specify the file to use.

A use case for this patch is sharing huge pages available through a
hugetlbfs mountpoint.

Signed-off-by: Damien Millescamps <damien.millescamps@6wind.com>
Signed-off-by: Pavel Fedin <p.fedin@samsung.com>
---
 2 years passed since the last review of this, and the original author never came back. So i decided
to pick up this work because my project is interested in this feature. I am keeping patch version
numbering, but the original changelog was lost.
 Relevant thread here: https://lists.gnu.org/archive/html/qemu-trivial/2013-09/msg00148.html

v3 => v4:
- Fixed documentation and wordings according to the last review
- Report also actual error messages
- Added missing exit()
- Fixed up code style according to latest checkpatch rules

---
 docs/specs/ivshmem_device_spec.txt |  7 ++--
 hw/misc/ivshmem.c                  | 67 +++++++++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 24 deletions(-)

Comments

Andrew Jones Oct. 6, 2015, 4:41 p.m. UTC | #1 
On Tue, Oct 06, 2015 at 04:33:23PM +0300, Pavel Fedin wrote:
> This patch permits to share memory areas that do not specifically belong to
> /dev/shm. In such case, the file must be already present when launching qemu.
> A new parameter 'file' has been added to specify the file to use.
> 
> A use case for this patch is sharing huge pages available through a
> hugetlbfs mountpoint.

ivshmem is getting a big reboot right now. See this series
"[PATCH v5 00/48] ivshmem improvements". Part of that work
is to better support sharing huge pages. Please look at that
series first, and if there's still something missing, then
additional patches to that series should be posted.

Thanks,
drew
Pavel Fedin Oct. 7, 2015, 1:13 p.m. UTC | #2 
Hello!

> ivshmem is getting a big reboot right now. See this series
> "[PATCH v5 00/48] ivshmem improvements". Part of that work
> is to better support sharing huge pages.

 I see there's a PULL, and i was waiting for it to be merged in order to test. But, is it going to
happen? I see there's some strong authorship-related conflict around it.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
Marc-André Lureau Oct. 7, 2015, 1:45 p.m. UTC | #3 
Hi

On Wed, Oct 7, 2015 at 3:13 PM, Pavel Fedin <p.fedin@samsung.com> wrote:
>  I see there's a PULL, and i was waiting for it to be merged in order to test. But, is it going to
> happen? I see there's some strong authorship-related conflict around it.

The authorship conflict is related to a single commit that could
easily be dropped in the next pull request. It's worth it if you could
test before it before it's actually merge!

thanks
Pavel Fedin Oct. 7, 2015, 1:50 p.m. UTC | #4 
Hello!

> The authorship conflict is related to a single commit that could
> easily be dropped in the next pull request. It's worth it if you could
> test before it before it's actually merge!

 Ok. In this case a single question. How to configure qemu to use ivshmem with the series. I have seen that "ivshmem can now use host memory backend", but how to set it up? I checked documentation patch, did not find good explanation there. But, well, programmers never read docs :)

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
Marc-André Lureau Oct. 7, 2015, 1:58 p.m. UTC | #5 
Hi

On Wed, Oct 7, 2015 at 3:50 PM, Pavel Fedin <p.fedin@samsung.com> wrote:
>  Ok. In this case a single question. How to configure qemu to use ivshmem with the series. I have seen that "ivshmem can now use host memory backend", but how to set it up? I checked documentation patch, did not find good explanation there. But, well, programmers never read docs :)

It is like other memdev properties, and there is an example in the
patch adding a test:
-object memory-backend-ram,size=1M,id=mb1 -device ivshmem,memdev=mb1

I will work on a patch to update the documentation.
Pavel Fedin Oct. 7, 2015, 2:22 p.m. UTC | #6 
Hello!

> It is like other memdev properties, and there is an example in the
> patch adding a test:
> -object memory-backend-ram,size=1M,id=mb1 -device ivshmem,memdev=mb1

 But what will cause memory-backend-ram to use hugetlb instead of just a RAM?

 P.S. Sorry if the question is stupid, i'm not really familiar in hugetlb details. I only know that managing huge pages is done via hugetlbfs.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
Marc-André Lureau Oct. 7, 2015, 2:24 p.m. UTC | #7 
Hi

On Wed, Oct 7, 2015 at 4:22 PM, Pavel Fedin <p.fedin@samsung.com> wrote:
>  But what will cause memory-backend-ram to use hugetlb instead of just a RAM?
>
>  P.S. Sorry if the question is stupid, i'm not really familiar in hugetlb details. I only know that managing huge pages is done via hugetlbfs.


Ah right, docs/memory-hotplug.txt should give you more details:
memory-backend-file,id=mem1,size=1G,mem-path=/mnt/hugepages-1GB

I wish they would have documented this in qemu-docs.texi too
Pavel Fedin Oct. 7, 2015, 2:25 p.m. UTC | #8 
Hi!

>  P.S. Sorry if the question is stupid, i'm not really familiar in hugetlb details. I only know
> that managing huge pages is done via hugetlbfs.

 Ops, i was double-stupid. :) The question should have actually been: "How to configure ivshmem to use hugetlb with the series?"

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
diff mbox

Patch

diff --git a/docs/specs/ivshmem_device_spec.txt b/docs/specs/ivshmem_device_spec.txt
index 667a862..8b108e7 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -4,8 +4,11 @@  Device Specification for Inter-VM shared memory device
 
 The Inter-VM shared memory device is designed to share a region of memory to
 userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+guest, but is a either a POSIX memory object or a mmap'd file (for instance,
+hugepage-backed file) on the host.
+
+Optionally, the device may support sending interrupts to other guests sharing
+the same memory region.
 
 
 The Inter-VM PCI device
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index cc76989..999ed7d 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -100,9 +100,10 @@  typedef struct IVShmemState {
 
     Error *migration_blocker;
 
-    char * shmobj;
-    char * sizearg;
-    char * role;
+    char *shmobj;
+    char *fileobj;
+    char *sizearg;
+    char *role;
     int role_val;   /* scalar to avoid multiple string comparisons */
 } IVShmemState;
 
@@ -769,9 +770,9 @@  static int pci_ivshmem_init(PCIDevice *dev)
         /* if we get a UNIX socket as the parameter we will talk
          * to the ivshmem server to receive the memory region */
 
-        if (s->shmobj != NULL) {
+        if (s->shmobj != NULL || s->fileobj != NULL) {
             error_report("WARNING: do not specify both 'chardev' "
-                         "and 'shm' with ivshmem");
+                         "and 'shm' or 'file' with ivshmem");
         }
 
         IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n",
@@ -797,28 +798,49 @@  static int pci_ivshmem_init(PCIDevice *dev)
     } else {
         /* just map the file immediately, we're not using a server */
         int fd;
+        int is_shm = !!(s->shmobj != NULL);
+        int is_file = !!(s->fileobj != NULL);
 
-        if (s->shmobj == NULL) {
-            error_report("Must specify 'chardev' or 'shm' to ivshmem");
+        if (!(is_shm ^ is_file)) {
+            error_report("Must specify one of 'chardev', 'shm' or 'file' "
+                         "to ivshmem");
             exit(1);
         }
 
-        IVSHMEM_DPRINTF("using shm_open (shm object = %s)\n", s->shmobj);
-
-        /* try opening with O_EXCL and if it succeeds zero the memory
-         * by truncating to 0 */
-        if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR|O_EXCL,
-                        S_IRWXU|S_IRWXG|S_IRWXO)) > 0) {
-           /* truncate file to length PCI device's memory */
-            if (ftruncate(fd, s->ivshmem_size) != 0) {
-                error_report("could not truncate shared file");
+        if (is_shm) {
+            IVSHMEM_DPRINTF("using shm_open (shm object = %s)\n", s->shmobj);
+
+            /* try opening with O_EXCL and if it succeeds zero the memory
+             * by truncating to 0 */
+            fd = shm_open(s->shmobj, O_CREAT|O_RDWR|O_EXCL,
+                          S_IRWXU|S_IRWXG|S_IRWXO);
+            if (fd > 0) {
+                /* truncate file to length PCI device's memory */
+                if (ftruncate(fd, s->ivshmem_size) != 0) {
+                    error_report("could not truncate shared file: %s",
+                                 strerror(errno));
+                    exit(1);
+                }
+            } else {
+                fd = shm_open(s->shmobj, O_CREAT|O_RDWR,
+                              S_IRWXU|S_IRWXG|S_IRWXO);
+                if (fd < 0) {
+                    error_report("could not open shared file: %s",
+                                 strerror(errno));
+                    exit(1);
+                }
+            }
+        } else {
+            IVSHMEM_DPRINTF("using open (file object = %s)\n", s->fileobj);
+
+            /* try opening a mmap's file. This file must have been previously
+             * created on the host */
+            fd = open(s->fileobj, O_RDWR);
+            if (fd < 0) {
+                error_report("ivshmem: could not open file: %s\n",
+                             strerror(errno));
+                exit(-1);
             }
-
-        } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR,
-                        S_IRWXU|S_IRWXG|S_IRWXO)) < 0) {
-            error_report("could not open shared file");
-            exit(1);
-
         }
 
         if (check_shm_size(s, fd) == -1) {
@@ -856,6 +878,7 @@  static Property ivshmem_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", IVShmemState, features, IVSHMEM_IOEVENTFD, false),
     DEFINE_PROP_BIT("msi", IVShmemState, features, IVSHMEM_MSI, true),
     DEFINE_PROP_STRING("shm", IVShmemState, shmobj),
+    DEFINE_PROP_STRING("file", IVShmemState, fileobj),
     DEFINE_PROP_STRING("role", IVShmemState, role),
     DEFINE_PROP_UINT32("use64", IVShmemState, ivshmem_64bit, 1),
     DEFINE_PROP_END_OF_LIST(),