* Adding a memory alias breaks v-rings
@ 2019-10-24 8:27 geoff
2019-10-24 11:07 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 3+ messages in thread
From: geoff @ 2019-10-24 8:27 UTC (permalink / raw)
To: qemu-devel
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. 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Adding a memory alias breaks v-rings
2019-10-24 8:27 Adding a memory alias breaks v-rings geoff
@ 2019-10-24 11:07 ` Philippe Mathieu-Daudé
2019-10-24 11:28 ` geoff--- via
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-10-24 11:07 UTC (permalink / raw)
To: Geoffrey McRae, qemu-devel, Alexey Kardashevskiy,
KONRAD Frederic, Michael S. Tsirkin, Paolo Bonzini
Hi Geoffrey,
On 10/24/19 10:27 AM, geoff@hostfission.com 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/qemu-devel@nongnu.org/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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Adding a memory alias breaks v-rings
2019-10-24 11:07 ` Philippe Mathieu-Daudé
@ 2019-10-24 11:28 ` geoff--- via
0 siblings, 0 replies; 3+ messages in thread
From: geoff--- via @ 2019-10-24 11:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Alexey Kardashevskiy, KONRAD Frederic,
Michael S. Tsirkin, Paolo Bonzini
Hi Phil
On 2019-10-24 22:07, Philippe Mathieu-Daudé wrote:
> Hi Geoffrey,
>
> On 10/24/19 10:27 AM, geoff@hostfission.com 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/qemu-devel@nongnu.org/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);
> }
> }
>
> ---
Unfortunately not, `memory_region_update_container_subregions` is
private in memory.c hangs the VM even if I expose it and call it as you
suggested. It is also already called via
memory_region_add_subregion_overlap anyway.
Thanks for the suggestion though :)
>
> 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
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-10-24 12:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-24 8:27 Adding a memory alias breaks v-rings geoff
2019-10-24 11:07 ` Philippe Mathieu-Daudé
2019-10-24 11:28 ` geoff--- via
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.