From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=gquxI3H7; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 714875A004C for ; Fri, 23 Aug 2024 00:14:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1724364869; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=fvVp1Ql+JdaNBh1qfJOZx4AoFh0ZQoBGLi4VtWiduQk=; b=gquxI3H7YaPzlpC0YNjPS1VgHbnfZEsFJuKMP6/cBBrPV3W5jl/QJbApd0iK80rJchsSHa QEJbyL0FoL6qC+Zn84G/BMkCjr+/DzugL3CwzWYS7nY52wtQvEtEyE/CYHcWYYzQSespft bjeiX2SxbPMuCUSxugGEjPwE4iBwck4= Received: from mail-pg1-f198.google.com (mail-pg1-f198.google.com [209.85.215.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-612-2bEEolv4Nayzftf5eLJk0Q-1; Thu, 22 Aug 2024 18:14:27 -0400 X-MC-Unique: 2bEEolv4Nayzftf5eLJk0Q-1 Received: by mail-pg1-f198.google.com with SMTP id 41be03b00d2f7-6507e2f0615so1272740a12.1 for ; Thu, 22 Aug 2024 15:14:27 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724364866; x=1724969666; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=XfhoZCx+/uqcnHYX3ycjiysyYwVS/HU7iszxJEXeQko=; b=DBObQ9z2OfifkIwUE+9XTb78favCvWphYSqJB5SbOkl8x5nLcY+kx5HGfqXuHNqzVM D0DRJqRWhmMhGDF4usV1Kp/+bEEG9iPlkG7AMNva0zzqR4R/41m7VtrhRgG57grbXndi amyzCIAL6zwM+GoyAOAuDjsSQvJeZvtjYLLogWovryt52FSkH+TFOMt8YFQb4tePSjTT Vx2dbu3q/C4Jd4dBQ6YYxp+exsFXIROYeBMomNJFdjdA9XC+TaVI4t0HPX1cFHoBksfX Ub/qGLBokmmPH14d2TceTRL+M5GDgLpG5DIwJf+hcAwUqqpOfI1PuY+hgCX8lflIXdFt L7Hg== X-Gm-Message-State: AOJu0YwauMb34bcSe7wFoxHO3Hmk1TjcAiNVYuWDsjvmg4V9jw4aQbNz wLVeckFqp4xpZTE5P4dOv/SzSbKfKXyD4bOegpOVD4oxhgKC950mI/t9CUpgmbw8RmrzXCxHd6/ cy4IP/oakyLIact1hPqH/BZCFZwKH43HVgYkfUH9qy4pXb34llskGOPvGp8swGOWODDiqVhAxWf HAwoc78S5lfvQS1btke0X//GPaYLTg7qBD X-Received: by 2002:a05:6a20:d510:b0:1c2:912f:ca70 with SMTP id adf61e73a8af0-1cc89fe6657mr544045637.42.1724364865666; Thu, 22 Aug 2024 15:14:25 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF9qUaluf67u8v5LF0fsoWgKSI7cwYdCS0CWP6/gM/nMIAvJJK2//FOEwpUCBpdeE6Tvly0bQ== X-Received: by 2002:a05:6a20:d510:b0:1c2:912f:ca70 with SMTP id adf61e73a8af0-1cc89fe6657mr544007637.42.1724364864666; Thu, 22 Aug 2024 15:14:24 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2038556686asm17241035ad.40.2024.08.22.15.14.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Aug 2024 15:14:23 -0700 (PDT) Date: Fri, 23 Aug 2024 00:14:15 +0200 From: Stefano Brivio To: Laurent Vivier Subject: Re: [PATCH v3 2/4] vhost-user: introduce virtio API Message-ID: <20240823001415.31955825@elisabeth> In-Reply-To: <20240815155024.827956-3-lvivier@redhat.com> References: <20240815155024.827956-1-lvivier@redhat.com> <20240815155024.827956-3-lvivier@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Message-ID-Hash: WHKQTZXUEKBHYTNL3EA2HGJF5XZ2SCTK X-Message-ID-Hash: WHKQTZXUEKBHYTNL3EA2HGJF5XZ2SCTK X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu, 15 Aug 2024 17:50:21 +0200 Laurent Vivier wrote: > Add virtio.c and virtio.h that define the functions needed > to manage virtqueues. >=20 > Signed-off-by: Laurent Vivier > --- > Makefile | 4 +- > util.h | 8 + > virtio.c | 662 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > virtio.h | 185 ++++++++++++++++ > 4 files changed, 857 insertions(+), 2 deletions(-) > create mode 100644 virtio.c > create mode 100644 virtio.h >=20 > diff --git a/Makefile b/Makefile > index b6329e35f884..f171c7955ac9 100644 > --- a/Makefile > +++ b/Makefile > @@ -47,7 +47,7 @@ FLAGS +=3D -DDUAL_STACK_SOCKETS=3D$(DUAL_STACK_SOCKETS) > PASST_SRCS =3D arch.c arp.c checksum.c conf.c dhcp.c dhcpv6.c flow.c fwd= .c \ > =09icmp.c igmp.c inany.c iov.c ip.c isolation.c lineread.c log.c mld.c \ > =09ndp.c netlink.c packet.c passt.c pasta.c pcap.c pif.c tap.c tcp.c \ > -=09tcp_buf.c tcp_splice.c udp.c udp_flow.c util.c > +=09tcp_buf.c tcp_splice.c udp.c udp_flow.c util.c virtio.c > QRAP_SRCS =3D qrap.c > SRCS =3D $(PASST_SRCS) $(QRAP_SRCS) > =20 > @@ -57,7 +57,7 @@ PASST_HEADERS =3D arch.h arp.h checksum.h conf.h dhcp.h= dhcpv6.h flow.h fwd.h \ > =09flow_table.h icmp.h icmp_flow.h inany.h iov.h ip.h isolation.h \ > =09lineread.h log.h ndp.h netlink.h packet.h passt.h pasta.h pcap.h pif.= h \ > =09siphash.h tap.h tcp.h tcp_buf.h tcp_conn.h tcp_internal.h tcp_splice.= h \ > -=09udp.h udp_flow.h util.h > +=09udp.h udp_flow.h util.h virtio.h > HEADERS =3D $(PASST_HEADERS) seccomp.h > =20 > C :=3D \#include \nstruct tcp_info x =3D { .tcpi_snd_wnd = =3D 0 }; > diff --git a/util.h b/util.h > index b7541ce24e5a..7944cfe1219d 100644 > --- a/util.h > +++ b/util.h > @@ -132,6 +132,14 @@ static inline uint32_t ntohl_unaligned(const void *p= ) > =09return ntohl(val); > } > =20 > +static inline void barrier(void) { __asm__ __volatile__("" ::: "memory")= ; } > +#define smp_mb()=09=09do { barrier(); __atomic_thread_fence(__ATOMIC_SEQ= _CST); } while (0) > +#define smp_mb_release()=09do { barrier(); __atomic_thread_fence(__ATOMI= C_RELEASE); } while (0) > +#define smp_mb_acquire()=09do { barrier(); __atomic_thread_fence(__ATOMI= C_ACQUIRE); } while (0) > + > +#define smp_wmb()=09smp_mb_release() > +#define smp_rmb()=09smp_mb_acquire() > + > #define NS_FN_STACK_SIZE=09(RLIMIT_STACK_VAL * 1024 / 8) > int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int= flags, > =09 void *arg); > diff --git a/virtio.c b/virtio.c > new file mode 100644 > index 000000000000..8354f6052aee > --- /dev/null > +++ b/virtio.c > @@ -0,0 +1,662 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later Even though I don't see the point, this needs to include "AND BSD-3-Clause" according to REUSE and SPDX guidelines (similarly to what Debian and Fedora require in package tags). And the SPDX tag needs to be in a C++-style comment. So, altogether: // SPDX-License-Identifier: GPL-2.0-or-later AND BSD-3-Clause > + * > + * virtio API, vring and virtqueue functions definition > + * > + * Copyright Red Hat > + * Author: Laurent Vivier > + */ > + > +/* some parts copied from QEMU subprojects/libvhost-user/libvhost-user.c s/some/Some/ > + * licensed under the following terms: I would say "originally licensed", to make it clear that we have no intention of modifying the original licensing terms. And given that it's multiple notices we're quoting here, perhaps separate them with something like: * -- * * notice * * -- > + * > + * Copyright IBM, Corp. 2007 > + * Copyright (c) 2016 Red Hat, Inc. > + * > + * Authors: > + * Anthony Liguori > + * Marc-Andr=C3=A9 Lureau > + * Victor Kaplansky > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > + * later. See the COPYING file in the top-level directory. > + * > + * Some parts copied from QEMU hw/virtio/virtio.c > + * licensed under the following terms: Same here. > + * > + * Copyright IBM, Corp. 2007 > + * > + * Authors: > + * Anthony Liguori > + * > + * This work is licensed under the terms of the GNU GPL, version 2. See > + * the COPYING file in the top-level directory. > + * > + * virtq_used_event() and virtq_avail_event() from > + * https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd0= 1.html#x1-712000A > + * licensed under the following terms: > + * > + * This header is BSD licensed so anyone can use the definitions > + * to implement compatible drivers/servers. > + * > + * Copyright 2007, 2009, IBM Corporation > + * Copyright 2011, Red Hat, Inc > + * All rights reserved. > + * > + * Redistribution and use in source and binary forms, with or without > + * modification, are permitted provided that the following conditions > + * are met: > + * 1. Redistributions of source code must retain the above copyright > + * notice, this list of conditions and the following disclaimer. > + * 2. Redistributions in binary form must reproduce the above copyright > + * notice, this list of conditions and the following disclaimer in th= e > + * documentation and/or other materials provided with the distributio= n. > + * 3. Neither the name of IBM nor the names of its contributors > + * may be used to endorse or promote products derived from this softw= are > + * without specific prior written permission. > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS = =E2=80=98=E2=80=98AS IS=E2=80=99=E2=80=99 AND > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PU= RPOSE > + * ARE DISCLAIMED. IN NO EVENT SHALL IBM OR CONTRIBUTORS BE LIABLE > + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUE= NTIAL > + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOO= DS > + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, S= TRICT > + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY= WAY > + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY O= F > + * SUCH DAMAGE. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "util.h" > +#include "virtio.h" > + > +#define VIRTQUEUE_MAX_SIZE 1024 > + > +/** > + * vu_gpa_to_va() - Translate guest physical address to our virtual addr= ess. > + * @dev:=09Vhost-user device > + * @plen:=09Physical length to map (input), virtual address mapped (outp= ut) I found this a bit misleading, in the sense that the output is not a virtual address, it's the size we pass as input, capped as needed if the remaining of the region is smaller... correct? In that case, I would say: ", capped to region (output)" or something like that. > + * @guest_addr:=09Guest physical address > + * > + * Return: virtual address in our address space of the guest physical ad= dress > + */ > +static void *vu_gpa_to_va(struct vu_dev *dev, uint64_t *plen, uint64_t g= uest_addr) > +{ > +=09unsigned int i; > + > +=09if (*plen =3D=3D 0) > +=09=09return NULL; > + > +=09/* Find matching memory region. */ > +=09for (i =3D 0; i < dev->nregions; i++) { > +=09=09const struct vu_dev_region *r =3D &dev->regions[i]; > + > +=09=09if ((guest_addr >=3D r->gpa) && > +=09=09 (guest_addr < (r->gpa + r->size))) { > +=09=09=09if ((guest_addr + *plen) > (r->gpa + r->size)) > +=09=09=09=09*plen =3D r->gpa + r->size - guest_addr; > +=09=09=09/* NOLINTNEXTLINE(performance-no-int-to-ptr) */ > +=09=09=09return (void *)(guest_addr - r->gpa + r->mmap_addr + > +=09=09=09=09=09=09 r->mmap_offset); > +=09=09} > +=09} > + > +=09return NULL; > +} > + > +/** > + * vring_avail_flags() - Read the available ring flags > + * @vq:=09=09Virtqueue > + * > + * Return: the available ring descriptor flags of the given virtqueue > + */ > +static inline uint16_t vring_avail_flags(const struct vu_virtq *vq) > +{ > +=09return le16toh(vq->vring.avail->flags); > +} > + > +/** > + * vring_avail_idx() - Read the available ring index > + * @vq:=09=09Virtqueue > + * > + * Return: the available ring index of the given virtqueue > + */ > +static inline uint16_t vring_avail_idx(struct vu_virtq *vq) > +{ > +=09vq->shadow_avail_idx =3D le16toh(vq->vring.avail->idx); > + > +=09return vq->shadow_avail_idx; > +} > + > +/** > + * vring_avail_ring() - Read an available ring entry > + * @vq:=09=09Virtqueue > + * @i:=09=09Index of the entry to read > + * > + * Return: the ring entry content (head of the descriptor chain) > + */ > +static inline uint16_t vring_avail_ring(const struct vu_virtq *vq, int i= ) > +{ > +=09return le16toh(vq->vring.avail->ring[i]); > +} > + > +/** > + * virtq_used_event - Get location of used event indices > + *=09=09 (only with VIRTIO_F_EVENT_IDX) > + * @vq=09=09Virtqueue > + * > + * Return: return the location of the used event index > + */ > +static inline uint16_t *virtq_used_event(const struct vu_virtq *vq) > +{ > + /* For backwards compat, used event index is at *end* of avail r= ing. */ > + return &vq->vring.avail->ring[vq->vring.num]; > +} > + > +/** > + * vring_get_used_event() - Get the used event from the available ring > + * @vq=09=09Virtqueue > + * > + * Return: the used event (available only if VIRTIO_RING_F_EVENT_IDX is = set) > + * used_event is a performant alternative where the driver > + * specifies how far the device can progress before a notificati= on > + * is required. > + */ > +static inline uint16_t vring_get_used_event(const struct vu_virtq *vq) > +{ > +=09return le16toh(*virtq_used_event(vq)); > +} > + > +/** > + * virtqueue_get_head() - Get the head of the descriptor chain for a giv= en > + * index > + * @vq:=09=09Virtqueue > + * @idx:=09Available ring entry index > + * @head:=09Head of the descriptor chain > + */ > +static void virtqueue_get_head(const struct vu_virtq *vq, > +=09=09=09 unsigned int idx, unsigned int *head) > +{ > +=09/* Grab the next descriptor number they're advertising, and increment > +=09 * the index we've seen. > +=09 */ > +=09*head =3D vring_avail_ring(vq, idx % vq->vring.num); > + > +=09/* If their number is silly, that's a fatal mistake. */ For the original code doing this, yes. But for passt, not necessarily: I think we should clarify some aspects of the lifecycle when we run with vhost-user support. At the moment, you allow mmap(2). But eventually I think it would be nice if we could postpone the application of the seccomp filter, perhaps just for the vhost-user case, so that we can lock things back up after receiving VHOST_USER_SET_MEM_TABLE. Should we get another command like that, we can report failure and quit, the user would need to restart us (libvirt already does), and the hypervisor would need to reconnect. If we can go that way, then we can very well die() here. But if that's not possible for some reason (perhaps we already discussed this?), then I think it would be more reasonable to close the connection here, instead of terminating. > +=09if (*head >=3D vq->vring.num) > +=09=09die("Guest says index %u is available", *head); In any case, this is a bit mysterious if one just sees it printed. Maybe: =09=09die("vhost-user: Guest sent invalid descriptor %u"... ? > +} > + > +/** > + * virtqueue_read_indirect_desc() - Copy virtio ring descriptors from gu= est > + * memory > + * @dev:=09Vhost-user device > + * @desc:=09Destination address to copy the descriptors ...to copy the descriptors to > + * @addr:=09Guest memory address to copy from > + * @len:=09Length of memory to copy > + * > + * Return: -1 if there is an error, 0 otherwise > + */ > +static int virtqueue_read_indirect_desc(struct vu_dev *dev, struct vring= _desc *desc, > +=09=09=09=09=09uint64_t addr, size_t len) > +{ > +=09uint64_t read_len; > + > +=09if (len > (VIRTQUEUE_MAX_SIZE * sizeof(struct vring_desc))) > +=09=09return -1; > + > +=09if (len =3D=3D 0) > +=09=09return -1; > + > +=09while (len) { > +=09=09const struct vring_desc *orig_desc; > + > +=09=09read_len =3D len; > +=09=09orig_desc =3D vu_gpa_to_va(dev, &read_len, addr); Should we also return if read_len < sizeof(struct vring_desc) after this call? Can that ever happen, if we pick a particular value of addr so that it's almost at the end of a region? > +=09=09if (!orig_desc) > +=09=09=09return -1; > + > +=09=09memcpy(desc, orig_desc, read_len); > +=09=09len -=3D read_len; > +=09=09addr +=3D read_len; > +=09=09desc +=3D read_len / sizeof(struct vring_desc); > +=09} > + > +=09return 0; > +} > + > +/** > + * enum virtqueue_read_desc_state - State in the descriptor chain > + * @VIRTQUEUE_READ_DESC_ERROR=09Found an invalid descriptor > + * @VIRTQUEUE_READ_DESC_DONE=09No more descriptors in the chain > + * @VIRTQUEUE_READ_DESC_MORE=09there are more descriptors in the chain > + */ > +enum virtqueue_read_desc_state { > +=09VIRTQUEUE_READ_DESC_ERROR =3D -1, > +=09VIRTQUEUE_READ_DESC_DONE =3D 0, /* end of chain */ > +=09VIRTQUEUE_READ_DESC_MORE =3D 1, /* more buffers in chain */ > +}; > + > +/** > + * virtqueue_read_next_desc() - Read the the next descriptor in the chai= n > + * @desc:=09Virtio ring descriptors > + * @i:=09=09Index of the current descriptor > + * @max:=09Maximum value of the descriptor index > + * @next:=09Index of the next descriptor in the chain (output value) > + * > + * Return: current chain descriptor state (error, next, done) > + */ > +static int virtqueue_read_next_desc(const struct vring_desc *desc, > +=09=09=09=09 int i, unsigned int max, unsigned int *next) > +{ > +=09/* If this descriptor says it doesn't chain, we're done. */ > +=09if (!(le16toh(desc[i].flags) & VRING_DESC_F_NEXT)) > +=09=09return VIRTQUEUE_READ_DESC_DONE; > + > +=09/* Check they're not leading us off end of descriptors. */ > +=09*next =3D le16toh(desc[i].next); > +=09/* Make sure compiler knows to grab that: we don't want it changing! = */ > +=09smp_wmb(); > + > +=09if (*next >=3D max) > +=09=09return VIRTQUEUE_READ_DESC_ERROR; > + > +=09return VIRTQUEUE_READ_DESC_MORE; > +} > + > +/** > + * vu_queue_empty() - Check if virtqueue is empty > + * @vq:=09=09Virtqueue > + * > + * Return: true if the virtqueue is empty, false otherwise > + */ > +bool vu_queue_empty(struct vu_virtq *vq) > +{ > +=09if (!vq->vring.avail) > +=09=09return true; > + > +=09if (vq->shadow_avail_idx !=3D vq->last_avail_idx) > +=09=09return false; > + > +=09return vring_avail_idx(vq) =3D=3D vq->last_avail_idx; > +} > + > +/** > + * vring_can_notify() - Check if a notification can be sent > + * @dev:=09Vhost-user device > + * @vq:=09=09Virtqueue > + * > + * Return: true if notification can be sent > + */ > +static bool vring_can_notify(const struct vu_dev *dev, struct vu_virtq *= vq) > +{ > +=09uint16_t old, new; > +=09bool v; > + > +=09/* We need to expose used array entries before checking used event. *= / > +=09smp_mb(); > + > +=09/* Always notify when queue is empty (when feature acknowledge) */ > +=09if (vu_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY) && > +=09=09!vq->inuse && vu_queue_empty(vq)) { Nit: indentation: =09if (vu_has_feature(...) && =09 !vq->inuse && ...) and curly brackets can go away. > +=09=09return true; > +=09} > + > +=09if (!vu_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) > +=09=09return !(vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT); > + > +=09v =3D vq->signalled_used_valid; > +=09vq->signalled_used_valid =3D true; > +=09old =3D vq->signalled_used; > +=09new =3D vq->signalled_used =3D vq->used_idx; > +=09return !v || vring_need_event(vring_get_used_event(vq), new, old); > +} > + > +/** > + * vu_queue_notify() - Send a notification to the given virtqueue > + * @dev:=09Vhost-user device > + * @vq:=09=09Virtqueue > + */ > +/* cppcheck-suppress unusedFunction */ > +void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq) > +{ > +=09if (!vq->vring.avail) > +=09=09return; > + > +=09if (!vring_can_notify(dev, vq)) { > +=09=09debug("vhost-user: virtqueue can skip notify..."); > +=09=09return; > +=09} > + > +=09if (eventfd_write(vq->call_fd, 1) < 0) > +=09=09die_perror("Error writing eventfd"); Same as the error above, this is missing a bit of context. It could say "Error writing vhost-user queue eventfd", perhaps. > +} > + > +/* virtq_avail_event() - Get location of available event indices > + *=09=09=09 (only with VIRTIO_F_EVENT_IDX) > + * @vq:=09=09Virtqueue > + * > + * Return: return the location of the available event index > + */ > +static inline uint16_t *virtq_avail_event(const struct vu_virtq *vq) > +{ > + /* For backwards compat, avail event index is at *end* of used r= ing. */ > + return (uint16_t *)&vq->vring.used->ring[vq->vring.num]; > +} > + > +/** > + * vring_set_avail_event() - Set avail_event > + * @vq:=09=09Virtqueue > + * @val:=09Value to set to avail_event > + *=09=09avail_event is used in the same way the used_event is in the > + *=09=09avail_ring. > + *=09=09avail_event is used to advise the driver that notifications > + *=09=09are unnecessary until the driver writes entry with an index > + *=09=09specified by avail_event into the available ring. > + */ > +static inline void vring_set_avail_event(const struct vu_virtq *vq, > +=09=09=09=09=09 uint16_t val) > +{ > +=09uint16_t val_le =3D htole16(val); > + > +=09if (!vq->notification) > +=09=09return; > + > +=09memcpy(virtq_avail_event(vq), &val_le, sizeof(val_le)); > +} > + > +/** > + * virtqueue_map_desc() - Translate descriptor ring physical address int= o our > + * =09=09=09 virtual address space > + * @dev:=09Vhost-user device > + * @p_num_sg:=09First iov entry to use (input), > + *=09=09first iov entry not used (output) > + * @iov:=09Iov array to use to store buffer virtual addresses > + * @max_num_sg:=09Maximum number of iov entries > + * @pa:=09=09Guest physical address of the buffer to map into our virtua= l > + * =09=09address > + * @sz:=09=09Size of the buffer > + * > + * Return: false on error, true otherwise > + */ > +static bool virtqueue_map_desc(struct vu_dev *dev, > +=09=09=09 unsigned int *p_num_sg, struct iovec *iov, > +=09=09=09 unsigned int max_num_sg, > +=09=09=09 uint64_t pa, size_t sz) > +{ > +=09unsigned int num_sg =3D *p_num_sg; > + > +=09ASSERT(num_sg < max_num_sg); > +=09ASSERT(sz); > + > +=09while (sz) { > +=09=09uint64_t len =3D sz; > + > +=09=09iov[num_sg].iov_base =3D vu_gpa_to_va(dev, &len, pa); > +=09=09if (iov[num_sg].iov_base =3D=3D NULL) > +=09=09=09die("virtio: invalid address for buffers"); > +=09=09iov[num_sg].iov_len =3D len; > +=09=09num_sg++; > +=09=09sz -=3D len; > +=09=09pa +=3D len; > +=09} > + > +=09*p_num_sg =3D num_sg; > +=09return true; > +} > + > +/** > + * vu_queue_map_desc - Map the virqueue descriptor ring into our virtual virtqueue into our virtual one > + * =09=09 address space > + * @dev:=09Vhost-user device > + * @vq:=09=09Virtqueue > + * @idx:=09First descriptor ring entry to map > + * @elem:=09Virtqueue element to store descriptor ring iov > + * > + * Return: -1 if there is an error, 0 otherwise > + */ > +static int vu_queue_map_desc(struct vu_dev *dev, struct vu_virtq *vq, un= signed int idx, > +=09=09=09 struct vu_virtq_element *elem) > +{ > +=09const struct vring_desc *desc =3D vq->vring.desc; > +=09struct vring_desc desc_buf[VIRTQUEUE_MAX_SIZE]; > +=09unsigned int out_num =3D 0, in_num =3D 0; > +=09unsigned int max =3D vq->vring.num; > +=09unsigned int i =3D idx; > +=09uint64_t read_len; > +=09int rc; > + > +=09if (le16toh(desc[i].flags) & VRING_DESC_F_INDIRECT) { > +=09=09unsigned int desc_len; > +=09=09uint64_t desc_addr; > + > +=09=09if (le32toh(desc[i].len) % sizeof(struct vring_desc)) > +=09=09=09die("Invalid size for indirect buffer table"); Same as above: prefix by "virtio:" or "vhost-user"? > + > +=09=09/* loop over the indirect descriptor table */ > +=09=09desc_addr =3D le64toh(desc[i].addr); > +=09=09desc_len =3D le32toh(desc[i].len); > +=09=09max =3D desc_len / sizeof(struct vring_desc); > +=09=09read_len =3D desc_len; > +=09=09desc =3D vu_gpa_to_va(dev, &read_len, desc_addr); > +=09=09if (desc && read_len !=3D desc_len) { > +=09=09=09/* Failed to use zero copy */ > +=09=09=09desc =3D NULL; > +=09=09=09if (!virtqueue_read_indirect_desc(dev, desc_buf, desc_addr, des= c_len)) > +=09=09=09=09desc =3D desc_buf; > +=09=09} > +=09=09if (!desc) > +=09=09=09die("Invalid indirect buffer table"); Same here. > +=09=09i =3D 0; > +=09} > + > +=09/* Collect all the descriptors */ > +=09do { > +=09=09if (le16toh(desc[i].flags) & VRING_DESC_F_WRITE) { > +=09=09=09if (!virtqueue_map_desc(dev, &in_num, elem->in_sg, > +=09=09=09=09=09=09elem->in_num, > +=09=09=09=09=09=09le64toh(desc[i].addr), > +=09=09=09=09=09=09le32toh(desc[i].len))) { > +=09=09=09=09return -1; > +=09=09=09} No need for curly brackets here, we have a single line in the block. > +=09=09} else { > +=09=09=09if (in_num) > +=09=09=09=09die("Incorrect order for descriptors"); > +=09=09=09if (!virtqueue_map_desc(dev, &out_num, elem->out_sg, > +=09=09=09=09=09=09elem->out_num, > +=09=09=09=09=09=09le64toh(desc[i].addr), > +=09=09=09=09=09=09le32toh(desc[i].len))) { > +=09=09=09=09return -1; Same here. > +=09=09=09} > +=09=09} > + > +=09=09/* If we've got too many, that implies a descriptor loop. */ > +=09=09if ((in_num + out_num) > max) > +=09=09=09die("Looped descriptor"); "vhost-user: Loop in queue descriptor list"? > +=09=09rc =3D virtqueue_read_next_desc(desc, i, max, &i); > +=09} while (rc =3D=3D VIRTQUEUE_READ_DESC_MORE); > + > +=09if (rc =3D=3D VIRTQUEUE_READ_DESC_ERROR) > +=09=09die("read descriptor error"); "vhost-user: Failed to read descriptor list"? > + > +=09elem->index =3D idx; > +=09elem->in_num =3D in_num; > +=09elem->out_num =3D out_num; > + > +=09return 0; > +} > + > +/** > + * vu_queue_pop() - Pop an entry from the virtqueue > + * @dev:=09Vhost-user device > + * @vq:=09=09Virtqueue > + * @elem:=09Virtqueue element to file with the entry information > + * > + * Return: -1 if there is an error, 0 otherwise > + */ > +/* cppcheck-suppress unusedFunction */ > +int vu_queue_pop(struct vu_dev *dev, struct vu_virtq *vq, struct vu_virt= q_element *elem) > +{ > +=09unsigned int head; > +=09int ret; > + > +=09if (!vq->vring.avail) > +=09=09return -1; > + > +=09if (vu_queue_empty(vq)) > +=09=09return -1; > + > +=09/* > +=09 * Needed after vu_queue_empty(), see comment in No need for extra newline at the beginning of the comment, =09/* Needed after ... > +=09 * virtqueue_num_heads(). > +=09 */ > +=09smp_rmb(); > + > +=09if (vq->inuse >=3D vq->vring.num) > +=09=09die("Virtqueue size exceeded"); "vhost-user queue size exceeded"? > + > +=09virtqueue_get_head(vq, vq->last_avail_idx++, &head); > + > +=09if (vu_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) > +=09=09vring_set_avail_event(vq, vq->last_avail_idx); > + > +=09ret =3D vu_queue_map_desc(dev, vq, head, elem); > + > +=09if (ret < 0) > +=09=09return ret; > + > +=09vq->inuse++; > + > +=09return 0; > +} > + > +/** > + * vu_queue_detach_element() - Detach an element from the virqueue > + * @vq:=09=09Virtqueue > + */ > +void vu_queue_detach_element(struct vu_virtq *vq) > +{ > +=09vq->inuse--; > +=09/* unmap, when DMA support is added */ > +} > + > +/** > + * vu_queue_unpop() - Push back the previously popped element from the v= irqueue > + * @vq:=09=09Virtqueue > + */ > +/* cppcheck-suppress unusedFunction */ > +void vu_queue_unpop(struct vu_virtq *vq) > +{ > +=09vq->last_avail_idx--; > +=09vu_queue_detach_element(vq); > +} > + > +/** > + * vu_queue_rewind() - Push back a given number of popped elements > + * @vq:=09=09Virtqueue > + * @num:=09Number of element to unpop > + */ > +/* cppcheck-suppress unusedFunction */ > +bool vu_queue_rewind(struct vu_virtq *vq, unsigned int num) > +{ > +=09if (num > vq->inuse) > +=09=09return false; > + > +=09vq->last_avail_idx -=3D num; > +=09vq->inuse -=3D num; > +=09return true; > +} > + > +/** > + * vring_used_write() - Write an entry in the used ring > + * @vq:=09=09Virtqueue > + * @uelem:=09Entry to write > + * @i:=09=09Index of the entry in the used ring > + */ > +static inline void vring_used_write(struct vu_virtq *vq, > +=09=09=09=09 const struct vring_used_elem *uelem, int i) > +{ > +=09struct vring_used *used =3D vq->vring.used; > + > +=09used->ring[i] =3D *uelem; > +} > + > +/** > + * vu_queue_fill_by_index() - Update information of a descriptor ring en= try > + *=09=09=09 in the used ring > + * @vq:=09=09Virtqueue > + * @index:=09Descriptor ring index > + * @len:=09Size of the element > + * @idx:=09Used ring entry index > + */ > +void vu_queue_fill_by_index(struct vu_virtq *vq, unsigned int index, > +=09=09=09 unsigned int len, unsigned int idx) > +{ > +=09struct vring_used_elem uelem; > + > +=09if (!vq->vring.avail) > +=09=09return; > + > +=09idx =3D (idx + vq->used_idx) % vq->vring.num; > + > +=09uelem.id =3D htole32(index); > +=09uelem.len =3D htole32(len); > +=09vring_used_write(vq, &uelem, idx); > +} > + > +/** > + * vu_queue_fill() - Update information of a given element in the used r= ing > + * @dev:=09Vhost-user device > + * @vq:=09=09Virtqueue > + * @elem:=09Element information to fill > + * @len:=09Size of the element > + * @idx:=09Used ring entry index > + */ > +/* cppcheck-suppress unusedFunction */ > +void vu_queue_fill(struct vu_virtq *vq, const struct vu_virtq_element *e= lem, > +=09=09 unsigned int len, unsigned int idx) > +{ > +=09vu_queue_fill_by_index(vq, elem->index, len, idx); > +} > + > +/** > + * vring_used_idx_set() - Set the descriptor ring current index > + * @vq:=09=09Virtqueue > + * @val:=09Value to set in the index > + */ > +static inline void vring_used_idx_set(struct vu_virtq *vq, uint16_t val) > +{ > +=09vq->vring.used->idx =3D htole16(val); > + > +=09vq->used_idx =3D val; > +} > + > +/** > + * vu_queue_flush() - Flush the virtqueue > + * @vq:=09=09Virtqueue > + * @count:=09Number of entry to flush > + */ > +/* cppcheck-suppress unusedFunction */ > +void vu_queue_flush(struct vu_virtq *vq, unsigned int count) > +{ > +=09uint16_t old, new; > + > +=09if (!vq->vring.avail) > +=09=09return; > + > +=09/* Make sure buffer is written before we update index. */ > +=09smp_wmb(); > + > +=09old =3D vq->used_idx; > +=09new =3D old + count; > +=09vring_used_idx_set(vq, new); > +=09vq->inuse -=3D count; > +=09if ((uint16_t)(new - vq->signalled_used) < (uint16_t)(new - old)) > +=09=09vq->signalled_used_valid =3D false; > +} > diff --git a/virtio.h b/virtio.h > new file mode 100644 > index 000000000000..af9cadc990b9 > --- /dev/null > +++ b/virtio.h > @@ -0,0 +1,185 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later > + * > + * virtio API, vring and virtqueue functions definition > + * > + * Copyright Red Hat > + * Author: Laurent Vivier > + */ > + > +#ifndef VIRTIO_H > +#define VIRTIO_H > + > +#include > +#include > + > +/* Maximum size of a virtqueue */ > +#define VIRTQUEUE_MAX_SIZE 1024 > + > +/** > + * struct vu_ring - Virtqueue rings > + * @num:=09=09Size of the queue > + * @desc:=09=09Descriptor ring > + * @avail:=09=09Available ring > + * @used:=09=09Used ring > + * @log_guest_addr:=09Guest address for logging > + * @flags:=09=09Vring flags > + * =09=09=09VHOST_VRING_F_LOG is set if log address is valid > + */ > +struct vu_ring { > +=09unsigned int num; > +=09struct vring_desc *desc; > +=09struct vring_avail *avail; > +=09struct vring_used *used; > +=09uint64_t log_guest_addr; > +=09uint32_t flags; > +}; > + > +/** > + * struct vu_virtq - Virtqueue definition > + * @vring:=09=09=09Virtqueue rings > + * @last_avail_idx:=09=09Next head to pop > + * @shadow_avail_idx:=09=09Last avail_idx read from VQ. > + * @used_idx:=09=09=09Descriptor ring current index > + * @signalled_used:=09=09Last used index value we have signalled on > + * @signalled_used_valid:=09True if signalled_used if valid > + * @notification:=09=09True if the queues notify (via event > + * =09=09=09=09index or interrupt) > + * @inuse:=09=09=09Number of entries in use > + * @call_fd:=09=09=09The event file descriptor to signal when > + * =09=09=09=09buffers are used. > + * @kick_fd:=09=09=09The event file descriptor for adding > + * =09=09=09=09buffers to the vring > + * @err_fd:=09=09=09The event file descriptor to signal when > + * =09=09=09=09error occurs > + * @enable:=09=09=09True if the virtqueue is enabled > + * @started:=09=09=09True if the virtqueue is started > + * @vra:=09=09=09QEMU address of our rings > + */ > +struct vu_virtq { > +=09struct vu_ring vring; > +=09uint16_t last_avail_idx; > +=09uint16_t shadow_avail_idx; > +=09uint16_t used_idx; > +=09uint16_t signalled_used; > +=09bool signalled_used_valid; > +=09bool notification; > +=09unsigned int inuse; > +=09int call_fd; > +=09int kick_fd; > +=09int err_fd; > +=09unsigned int enable; > +=09bool started; > +=09struct vhost_vring_addr vra; > +}; > + > +/** > + * struct vu_dev_region - guest shared memory region > + * @gpa:=09=09Guest physical address of the region > + * @size:=09=09Memory size in bytes > + * @qva:=09=09QEMU virtual address > + * @mmap_offset:=09Offset where the region starts in the mapped memory > + * @mmap_addr:=09=09Address of the mapped memory > + */ > +struct vu_dev_region { > +=09uint64_t gpa; > +=09uint64_t size; > +=09uint64_t qva; > +=09uint64_t mmap_offset; > +=09uint64_t mmap_addr; > +}; > + > +#define VHOST_USER_MAX_QUEUES 2 > + > +/* > + * Set a reasonable maximum number of ram slots, which will be supported= by > + * any architecture. > + */ > +#define VHOST_USER_MAX_RAM_SLOTS 32 > + > +/** > + * struct vu_dev - vhost-user device information > + * @context:=09=09Execution context > + * *nregions:=09=09Number of shared memory regions > + * @regions:=09=09Guest shared memory regions > + * @features:=09=09Vhost-user features > + * @protocol_features:=09Vhost-user protocol features > + * @hdrlen:=09=09Virtio -net header length > + */ > +struct vu_dev { > +=09uint32_t nregions; > +=09struct vu_dev_region regions[VHOST_USER_MAX_RAM_SLOTS]; > +=09struct vu_virtq vq[VHOST_USER_MAX_QUEUES]; > +=09uint64_t features; > +=09uint64_t protocol_features; > +=09int hdrlen; > +}; > + > +/** > + * struct vu_virtq_element - virtqueue element > + * @index:=09Descriptor ring index > + * @out_num:=09Number of outgoing iovec buffers > + * @in_num:=09Number of incoming iovec buffers > + * @in_sg:=09Incoming iovec buffers > + * @out_sg:=09Outgoing iovec buffers > + */ > +struct vu_virtq_element { > +=09unsigned int index; > +=09unsigned int out_num; > +=09unsigned int in_num; > +=09struct iovec *in_sg; > +=09struct iovec *out_sg; > +}; > + > +/** > + * has_feature() - Check a feature bit in a features set > + * @features:=09Features set > + * @fb:=09=09Feature bit to check > + * > + * Return:=09True if the feature bit is set > + */ > +static inline bool has_feature(uint64_t features, unsigned int fbit) > +{ > +=09return !!(features & (1ULL << fbit)); > +} > + > +/** > + * vu_has_feature() - Check if a virtio-net feature is available > + * @vdev:=09Vhost-user device > + * @bit:=09Feature to check > + * > + * Return:=09True if the feature is available > + */ > +static inline bool vu_has_feature(const struct vu_dev *vdev, > +=09=09=09=09 unsigned int fbit) > +{ > +=09return has_feature(vdev->features, fbit); > +} > + > +/** > + * vu_has_protocol_feature() - Check if a vhost-user feature is availabl= e > + * @vdev:=09Vhost-user device > + * @bit:=09Feature to check > + * > + * Return:=09True if the feature is available > + */ > +/* cppcheck-suppress unusedFunction */ > +static inline bool vu_has_protocol_feature(const struct vu_dev *vdev, > +=09=09=09=09=09 unsigned int fbit) > +{ > +=09return has_feature(vdev->protocol_features, fbit); > +} > + > +bool vu_queue_empty(struct vu_virtq *vq); > +void vu_queue_notify(const struct vu_dev *dev, struct vu_virtq *vq); > +int vu_queue_pop(struct vu_dev *dev, struct vu_virtq *vq, > +=09=09 struct vu_virtq_element *elem); > +void vu_queue_detach_element(struct vu_virtq *vq); > +void vu_queue_unpop(struct vu_virtq *vq); > +bool vu_queue_rewind(struct vu_virtq *vq, unsigned int num); > +void vu_queue_fill_by_index(struct vu_virtq *vq, unsigned int index, > +=09=09=09 unsigned int len, unsigned int idx); > +void vu_queue_fill(struct vu_virtq *vq, > +=09=09 const struct vu_virtq_element *elem, unsigned int len, > +=09=09 unsigned int idx); > +void vu_queue_flush(struct vu_virtq *vq, unsigned int count); > +#endif /* VIRTIO_H */ The rest looks good to me. --=20 Stefano