[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] dataplane: simplify indirect d
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] dataplane: simplify indirect descriptor read |
Date: |
Thu, 29 Oct 2015 11:28:05 +0100 |
On Wed, 28 Oct 2015 17:48:02 +0200
"Michael S. Tsirkin" <address@hidden> wrote:
> Use address_space_read to make sure we handle the case of an indirect
> descriptor crossing DIMM boundary correctly.
>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>
> Warning: compile-tested only.
>
> hw/virtio/dataplane/vring.c | 28 ++++++++++++++++++----------
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
> index 68f1994..0b92fcf 100644
> --- a/hw/virtio/dataplane/vring.c
> +++ b/hw/virtio/dataplane/vring.c
> @@ -257,6 +257,21 @@ static void copy_in_vring_desc(VirtIODevice
> *vdev, host->next = virtio_lduw_p(vdev, &guest->next);
> }
>
> +static bool read_vring_desc(VirtIODevice *vdev,
> + hwaddr guest,
> + struct vring_desc *host)
> +{
> + if (address_space_read(&address_space_memory, guest,
> MEMTXATTRS_UNSPECIFIED,
> + (uint8_t *)host, sizeof *host)) {
> + return false;
> + }
> + host->addr = virtio_tswap64(vdev, host->addr);
> + host->len = virtio_tswap32(vdev, host->len);
> + host->flags = virtio_tswap16(vdev, host->flags);
> + host->next = virtio_tswap16(vdev, host->next);
> + return true;
> +}
> +
> /* This is stolen from linux/drivers/vhost/vhost.c. */
> static int get_indirect(VirtIODevice *vdev, Vring *vring,
> VirtQueueElement *elem, struct vring_desc
> *indirect) @@ -284,23 +299,16 @@ static int get_indirect(VirtIODevice
> *vdev, Vring *vring, }
>
> do {
> - struct vring_desc *desc_ptr;
> - MemoryRegion *mr;
> -
> /* Translate indirect descriptor */
> - desc_ptr = vring_map(&mr,
> - indirect->addr + found * sizeof(desc),
Is it correct to use 'found' as iterator here vs using desc.next
as is done when translating direct descriptors?
> - sizeof(desc), false);
> - if (!desc_ptr) {
> - error_report("Failed to map indirect descriptor "
> + if (!read_vring_desc(vdev, indirect->addr + found *
> sizeof(desc),
> + &desc)) {
> + error_report("Failed to read indirect descriptor "
> "addr %#" PRIx64 " len %zu",
> (uint64_t)indirect->addr + found *
> sizeof(desc), sizeof(desc));
> vring->broken = true;
> return -EFAULT;
> }
> - copy_in_vring_desc(vdev, desc_ptr, &desc);
> - memory_region_unref(mr);
>
> /* Ensure descriptor has been loaded before accessing fields
> */ barrier(); /* read_barrier_depends(); */