[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 091/104] libvhost-user: Fix some memtable remap cases
From: |
Marc-André Lureau |
Subject: |
Re: [PATCH 091/104] libvhost-user: Fix some memtable remap cases |
Date: |
Fri, 17 Jan 2020 17:58:39 +0400 |
Hi
On Thu, Dec 12, 2019 at 10:05 PM Dr. David Alan Gilbert (git)
<address@hidden> wrote:
>
> From: "Dr. David Alan Gilbert" <address@hidden>
>
> If a new setmemtable command comes in once the vhost threads are
> running, it will remap the guests address space and the threads
> will now be looking in the wrong place.
>
> Fortunately we're running this command under lock, so we can
> update the queue mappings so that threads will look in the new-right
> place.
>
> Note: This doesn't fix things that the threads might be doing
> without a lock (e.g. a readv/writev!) That's for another time.
>
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
> contrib/libvhost-user/libvhost-user.c | 33 ++++++++++++++++++++-------
> contrib/libvhost-user/libvhost-user.h | 3 +++
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c
> b/contrib/libvhost-user/libvhost-user.c
> index 63e41062a4..b89bf18501 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -564,6 +564,21 @@ vu_reset_device_exec(VuDev *dev, VhostUserMsg *vmsg)
> return false;
> }
>
> +static bool
> +map_ring(VuDev *dev, VuVirtq *vq)
> +{
> + vq->vring.desc = qva_to_va(dev, vq->vra.desc_user_addr);
> + vq->vring.used = qva_to_va(dev, vq->vra.used_user_addr);
> + vq->vring.avail = qva_to_va(dev, vq->vra.avail_user_addr);
> +
> + DPRINT("Setting virtq addresses:\n");
> + DPRINT(" vring_desc at %p\n", vq->vring.desc);
> + DPRINT(" vring_used at %p\n", vq->vring.used);
> + DPRINT(" vring_avail at %p\n", vq->vring.avail);
> +
> + return !(vq->vring.desc && vq->vring.used && vq->vring.avail);
> +}
> +
> static bool
> vu_set_mem_table_exec_postcopy(VuDev *dev, VhostUserMsg *vmsg)
> {
> @@ -767,6 +782,14 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
> close(vmsg->fds[i]);
> }
>
> + for (i = 0; i < dev->max_queues; i++) {
> + if (dev->vq[i].vring.desc) {
The code usually checks for all (vq->vring.desc && vq->vring.used &&
vq->vring.avail).
Perhaps we should introduce a VQ_RING_IS_SET() macro?
> + if (map_ring(dev, &dev->vq[i])) {
> + vu_panic(dev, "remaping queue %d during setmemtable", i);
> + }
> + }
> + }
> +
> return false;
> }
>
> @@ -853,18 +876,12 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg)
> DPRINT(" avail_user_addr: 0x%016" PRIx64 "\n", vra->avail_user_addr);
> DPRINT(" log_guest_addr: 0x%016" PRIx64 "\n", vra->log_guest_addr);
>
> + vq->vra = *vra;
> vq->vring.flags = vra->flags;
> - vq->vring.desc = qva_to_va(dev, vra->desc_user_addr);
> - vq->vring.used = qva_to_va(dev, vra->used_user_addr);
> - vq->vring.avail = qva_to_va(dev, vra->avail_user_addr);
> vq->vring.log_guest_addr = vra->log_guest_addr;
>
> - DPRINT("Setting virtq addresses:\n");
> - DPRINT(" vring_desc at %p\n", vq->vring.desc);
> - DPRINT(" vring_used at %p\n", vq->vring.used);
> - DPRINT(" vring_avail at %p\n", vq->vring.avail);
>
> - if (!(vq->vring.desc && vq->vring.used && vq->vring.avail)) {
> + if (map_ring(dev, vq)) {
> vu_panic(dev, "Invalid vring_addr message");
> return false;
> }
> diff --git a/contrib/libvhost-user/libvhost-user.h
> b/contrib/libvhost-user/libvhost-user.h
> index 1844b6f8d4..5cb7708559 100644
> --- a/contrib/libvhost-user/libvhost-user.h
> +++ b/contrib/libvhost-user/libvhost-user.h
> @@ -327,6 +327,9 @@ typedef struct VuVirtq {
> int err_fd;
> unsigned int enable;
> bool started;
> +
> + /* Guest addresses of our ring */
> + struct vhost_vring_addr vra;
> } VuVirtq;
>
> enum VuWatchCondtion {
> --
> 2.23.0
>
>
Looks reasonable otherwise (assuming all running threads were flushed
somehow by other means)
--
Marc-André Lureau
- Re: [PATCH 091/104] libvhost-user: Fix some memtable remap cases,
Marc-André Lureau <=