[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH] hw/virtio/vhost: re-factor vhost-section and allow DIRTY
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [RFC PATCH] hw/virtio/vhost: re-factor vhost-section and allow DIRTY_MEMORY_CODE |
Date: |
Thu, 4 Jun 2020 14:26:35 +0100 |
User-agent: |
Mutt/1.13.4 (2020-02-15) |
* Alex Bennée (alex.bennee@linaro.org) wrote:
> The purpose of vhost_section is to identify RAM regions that need to
> be made available to a vhost client. However when running under TCG
> all RAM sections have DIRTY_MEMORY_CODE set which leads to problems
> down the line. The original comment implies VGA regions are a problem
> but doesn't explain why vhost has a problem with it.
>
> Re-factor the code so:
>
> - steps are clearer to follow
> - reason for rejection is recorded in the trace point
> - we allow DIRTY_MEMORY_CODE when TCG is enabled
The problem with VGA is that a VGA page can become mapped and unmapped
under the control of the guest; somewhere in a low address. This tends
to break hugepage mappings.
For vhost-user, and in particular vhost-user-postcopy this means it
fails the mapping on the vhost-user client.
However the other problem is that with vhost-user, the vhost-user client
is changing memory; and won't mark the pages as dirty - except for
migration (I'm not clear if vhost kernel does this).
So TCG won't notice a page that's been changed by the driver; now in
most cases it's rare for a device to be writing directly into a page
you're going to execute out of, but it's not unknown.
So, as it is, any area that's expecting to get non-migration dirty
notifications is going to be disappointed by a vhost-user backend.
Dave
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> hw/virtio/vhost.c | 46 ++++++++++++++++++++++++++++++++--------------
> 1 file changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index aff98a0ede5..f81fc87e74c 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -27,6 +27,7 @@
> #include "migration/blocker.h"
> #include "migration/qemu-file-types.h"
> #include "sysemu/dma.h"
> +#include "sysemu/tcg.h"
> #include "trace.h"
>
> /* enabled until disconnected backend stabilizes */
> @@ -403,26 +404,43 @@ static int vhost_verify_ring_mappings(struct vhost_dev
> *dev,
> return r;
> }
>
> +/*
> + * vhost_section: identify sections needed for vhost access
> + *
> + * We only care about RAM sections here (where virtqueue can live). If
> + * we find one we still allow the backend to potentially filter it out
> + * of our list.
> + */
> static bool vhost_section(struct vhost_dev *dev, MemoryRegionSection
> *section)
> {
> - bool result;
> - bool log_dirty = memory_region_get_dirty_log_mask(section->mr) &
> - ~(1 << DIRTY_MEMORY_MIGRATION);
> - result = memory_region_is_ram(section->mr) &&
> - !memory_region_is_rom(section->mr);
> -
> - /* Vhost doesn't handle any block which is doing dirty-tracking other
> - * than migration; this typically fires on VGA areas.
> - */
> - result &= !log_dirty;
> + enum { OK = 0, NOT_RAM, DIRTY, FILTERED } result = NOT_RAM;
> +
> + if (memory_region_is_ram(section->mr) &&
> !memory_region_is_rom(section->mr)) {
> + uint8_t dirty_mask = memory_region_get_dirty_log_mask(section->mr);
> + uint8_t handled_dirty;
>
> - if (result && dev->vhost_ops->vhost_backend_mem_section_filter) {
> - result &=
> - dev->vhost_ops->vhost_backend_mem_section_filter(dev, section);
> + /*
> + * Vhost doesn't handle any block which is doing dirty-tracking other
> + * than migration; this typically fires on VGA areas. However
> + * for TCG we also do dirty code page tracking which shouldn't
> + * get in the way.
> + */
> + handled_dirty = (1 << DIRTY_MEMORY_MIGRATION);
> + if (tcg_enabled()) {
> + handled_dirty |= (1 << DIRTY_MEMORY_CODE);
> + }
> + if (dirty_mask & ~handled_dirty) {
> + result = DIRTY;
> + } else if (dev->vhost_ops->vhost_backend_mem_section_filter &&
> + !dev->vhost_ops->vhost_backend_mem_section_filter(dev, section))
> {
> + result = FILTERED;
> + } else {
> + result = OK;
> + }
> }
>
> trace_vhost_section(section->mr->name, result);
> - return result;
> + return result == OK;
> }
>
> static void vhost_begin(MemoryListener *listener)
> --
> 2.20.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [RFC PATCH] hw/virtio/vhost: re-factor vhost-section and allow DIRTY_MEMORY_CODE,
Dr. David Alan Gilbert <=