[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Adding a memory alias breaks v-rings
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: Adding a memory alias breaks v-rings |
Date: |
Thu, 24 Oct 2019 13:07:03 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.1.1 |
Hi Geoffrey,
On 10/24/19 10:27 AM, address@hidden wrote:
Hi All,
I have been working on adding a feature as a proof of concept to improve
the performance of applications like Looking Glass by avoiding
additional memory copies. My goal is to alias part of the IVSHMEM shared
memory over a pointer provided by the guest OS capture API (DXGI Desktop
Duplication or NVIDIA Frame Buffer Capture). I have managed to get this
working by adding a few additional configuration registers to the
IVSHMEM device and enhanced the IVSHMEM windows driver with suitable
IOCTLs to set this all up. While this concept is backwards it needs to
work this way as we do not have control over the destination buffer
allocation by the GPU driver.
This all works, however, it has exposed a bug (or I am doing things
improperly) with the way that vhost tracks memory. When calling
memory_region_add_subregion_overlap the memory listener in vhost fires
triggering vhost_region_add_section. According to the comments this code
depends on being called in memory address order, but because I am adding
the alias region late, it's out of order, and also splits the upper
memory region. This has the effect of corrupting/breaking one or more
random vrings, as evidenced by the crash/hang of vhost-net or other
virtio devices.
I'm not sure this is the same issue I had before, but you might
find Frederic and Alexey suggestions from this thread helpful:
https://www.mail-archive.com/address@hidden/msg525833.html
Also note vhost_region_add_section() you mentioned has this comment:
if (need_add) {
...
/* The flatview isn't stable and we don't use it, making it NULL
* means we can memcmp the list.
*/
dev->tmp_sections[dev->n_tmp_sections - 1].fv = NULL;
Maybe you need this change:
-- >8 --
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -642,6 +642,7 @@ static void vhost_region_add_section(struct
vhost_dev *dev,
*/
dev->tmp_sections[dev->n_tmp_sections - 1].fv = NULL;
memory_region_ref(section->mr);
+ memory_region_update_container_subregions(section->mr);
}
}
---
Regards,
Phil.
The following and errors are also logged regarding
section alignment:
qemu-system-x86_64: vhost_region_add_section:Section rounded to
3c0000000 prior to previous 3fc4f9000
qemu-system-x86_64: vhost_region_add_section:Section rounded to
3c0000000 prior to previous 3fc4f9000
Here is the flat view after the alias has been added.
0000000100000000-00000003fc4f8fff (prio 0, ram): mem
@0000000080000000 kvm
00000003fc4f9000-00000003fc4f9fff (prio 1, ram): ivshmem kvm
00000003fc4fa000-000000043fffffff (prio 0, ram): mem
@000000037c4fa000 kvm
When the guest doesn't crash out due to the obvious corruption it is
possible to verify that the alias is in the right place and fully
functional. Unfortunately, I simply do not have enough of a grasp on
vhost to understand exactly what is going on and how to correct it.
Getting this feature working is highly desirable as it should be
possible to obtain GPU -> GPU memory transfers between guests without
requiring workstation/professional graphics cards.
Kind Regards,
Geoffrey McRae