From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 0A8F1C433F5 for ; Mon, 31 Jan 2022 10:20:44 +0000 (UTC) Received: from localhost ([::1]:46436 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nETnf-00008Z-Pg for qemu-devel@archiver.kernel.org; Mon, 31 Jan 2022 05:20:43 -0500 Received: from eggs.gnu.org ([209.51.188.92]:49586) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nETmZ-0007YP-3f for qemu-devel@nongnu.org; Mon, 31 Jan 2022 05:19:35 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]:40179) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nETmW-0005nx-Cd for qemu-devel@nongnu.org; Mon, 31 Jan 2022 05:19:34 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1643624365; 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=fkJLGZME/+hsZXtGB+83k/vwiy7v5qdBQnLhYtdlajY=; b=LLMsOJtNf3mKXQ1n1/jXou+9vLCPOwbW00mIV7oEd+zptpLou6esEXixzGKKtgdUfkodTl SkA2VgUiwXysW8ufPDD4PIS2JFEls7UtmOrVUwURTv67X7gwAtG6hCYnCzseFNVHn2u0vo i9+yu5j7klWAgJFKPXUI1sQRzIyzu4U= Received: from mail-qk1-f200.google.com (mail-qk1-f200.google.com [209.85.222.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-639-wJwfJx6bMZuhxMqEejpzXw-1; Mon, 31 Jan 2022 05:19:24 -0500 X-MC-Unique: wJwfJx6bMZuhxMqEejpzXw-1 Received: by mail-qk1-f200.google.com with SMTP id j66-20020a375545000000b004e0e071f37eso6699358qkb.23 for ; Mon, 31 Jan 2022 02:19:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=fkJLGZME/+hsZXtGB+83k/vwiy7v5qdBQnLhYtdlajY=; b=sGK+APq3EKtAFSjR4xBYO2eYgghciLwmmy4F4QjoZrRwStVPZHWRTROP9BlU9WX4qT Xus3cwk1M9j76NwvbglRDnNmvuPjobC8d0VAIZq3rm3972jK4+sbr1L3i8ZCY3lOzI+o YJM7xshyzppIVFAxNeM9V1wPNuUBoUT6XmsrFM8twufJeQaeiUALa5BR29eCbG0xR13Z zQ1mZsMUb0rRF8NuIRISXhyUYYN7QdmzX2RKRFaUsKZ4EfqKduJp8u3ytrm6kda/lV9c 4WmmrFK66L+18y2jBOygmm9WMRpGOteytPBJ/9E6cCnAxKn9K8RRf3MStuDJYwsLfNjI TdYw== X-Gm-Message-State: AOAM531l5qthVpHh6vtbiQ1aL8zhxEXt72pYBkX1TE40qhftY+Rtj9AW OOnK71rg79CK2e+OAPtBxPPytTCAKEYcJbyy2qNJnRUpji4hWKZiSprZRjWK33jd4IkgOFjuNhs wzZXRLF5DOd3oeHhtBlop7IaMvvhChCw= X-Received: by 2002:a05:6214:20ad:: with SMTP id 13mr16532686qvd.40.1643624362003; Mon, 31 Jan 2022 02:19:22 -0800 (PST) X-Google-Smtp-Source: ABdhPJwv14CBKOkWG38eu1vqXl661BXlEE53xOcdOkS/cl+4xyu77DnkEN9QNiyEq7SHuAnwloQf+Y1Oglokz7J45YI= X-Received: by 2002:a05:6214:20ad:: with SMTP id 13mr16532660qvd.40.1643624361680; Mon, 31 Jan 2022 02:19:21 -0800 (PST) MIME-Version: 1.0 References: <20220121202733.404989-1-eperezma@redhat.com> <20220121202733.404989-5-eperezma@redhat.com> In-Reply-To: From: Eugenio Perez Martin Date: Mon, 31 Jan 2022 11:18:45 +0100 Message-ID: Subject: Re: [PATCH 04/31] vdpa: Add vhost_svq_set_svq_kick_fd To: Jason Wang Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=eperezma@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Received-SPF: pass client-ip=170.10.129.124; envelope-from=eperezma@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.088, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Laurent Vivier , Parav Pandit , Cindy Lu , "Michael S. Tsirkin" , Juan Quintela , Richard Henderson , qemu-level , Gautam Dawar , Markus Armbruster , Eduardo Habkost , Harpreet Singh Anand , Xiao W Wang , Peter Xu , Stefan Hajnoczi , Eli Cohen , Paolo Bonzini , Zhu Lingshan , virtualization , Eric Blake , Stefano Garzarella Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" On Fri, Jan 28, 2022 at 7:29 AM Jason Wang wrote: > > > =E5=9C=A8 2022/1/22 =E4=B8=8A=E5=8D=884:27, Eugenio P=C3=A9rez =E5=86=99= =E9=81=93: > > This function allows the vhost-vdpa backend to override kick_fd. > > > > Signed-off-by: Eugenio P=C3=A9rez > > --- > > hw/virtio/vhost-shadow-virtqueue.h | 1 + > > hw/virtio/vhost-shadow-virtqueue.c | 45 +++++++++++++++++++++++++++++= + > > 2 files changed, 46 insertions(+) > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h b/hw/virtio/vhost-shado= w-virtqueue.h > > index 400effd9f2..a56ecfc09d 100644 > > --- a/hw/virtio/vhost-shadow-virtqueue.h > > +++ b/hw/virtio/vhost-shadow-virtqueue.h > > @@ -15,6 +15,7 @@ > > > > typedef struct VhostShadowVirtqueue VhostShadowVirtqueue; > > > > +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick= _fd); > > const EventNotifier *vhost_svq_get_dev_kick_notifier( > > const VhostShadowVirtqu= eue *svq); > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shado= w-virtqueue.c > > index bd87110073..21534bc94d 100644 > > --- a/hw/virtio/vhost-shadow-virtqueue.c > > +++ b/hw/virtio/vhost-shadow-virtqueue.c > > @@ -11,6 +11,7 @@ > > #include "hw/virtio/vhost-shadow-virtqueue.h" > > > > #include "qemu/error-report.h" > > +#include "qemu/main-loop.h" > > > > /* Shadow virtqueue to relay notifications */ > > typedef struct VhostShadowVirtqueue { > > @@ -18,8 +19,20 @@ typedef struct VhostShadowVirtqueue { > > EventNotifier hdev_kick; > > /* Shadow call notifier, sent to vhost */ > > EventNotifier hdev_call; > > + > > + /* > > + * Borrowed virtqueue's guest to host notifier. > > + * To borrow it in this event notifier allows to register on the e= vent > > + * loop and access the associated shadow virtqueue easily. If we u= se the > > + * VirtQueue, we don't have an easy way to retrieve it. > > + * > > + * So shadow virtqueue must not clean it, or we would lose VirtQue= ue one. > > + */ > > + EventNotifier svq_kick; > > } VhostShadowVirtqueue; > > > > +#define INVALID_SVQ_KICK_FD -1 > > + > > /** > > * The notifier that SVQ will use to notify the device. > > */ > > @@ -29,6 +42,35 @@ const EventNotifier *vhost_svq_get_dev_kick_notifier= ( > > return &svq->hdev_kick; > > } > > > > +/** > > + * Set a new file descriptor for the guest to kick SVQ and notify for = avail > > + * > > + * @svq The svq > > + * @svq_kick_fd The new svq kick fd > > + */ > > +void vhost_svq_set_svq_kick_fd(VhostShadowVirtqueue *svq, int svq_kick= _fd) > > +{ > > + EventNotifier tmp; > > + bool check_old =3D INVALID_SVQ_KICK_FD !=3D > > + event_notifier_get_fd(&svq->svq_kick); > > + > > + if (check_old) { > > + event_notifier_set_handler(&svq->svq_kick, NULL); > > + event_notifier_init_fd(&tmp, event_notifier_get_fd(&svq->svq_k= ick)); > > + } > > > It looks to me we don't do similar things in vhost-net. Any reason for > caring about the old svq_kick? > Do you mean to check for old kick_fd in case we miss notifications, and explicitly omit the INVALID_SVQ_KICK_FD? If you mean qemu's vhost-net, I guess it's because the device's kick fd is never changed in all the vhost device lifecycle, it's only set at the beginning. Previous RFC also depended on that, but you suggested better vhost and SVQ in v4 feedback if I understood correctly [1]. Or am I missing something? Qemu's vhost-net does not need to use this because it is not polling it. For kernel's vhost, I guess the closest is the use of pollstop and pollstart at vhost_vring_ioctl. In my opinion, I think that SVQ code size can benefit from now allowing to override kick_fd from the start of the operation. Not from initialization, but start. But I can see the benefits of having the change into account from this moment so it's more resilient to the future. > > > + > > + /* > > + * event_notifier_set_handler already checks for guest's notificat= ions if > > + * they arrive to the new file descriptor in the switch, so there = is no > > + * need to explicitely check for them. > > + */ > > + event_notifier_init_fd(&svq->svq_kick, svq_kick_fd); > > + > > + if (!check_old || event_notifier_test_and_clear(&tmp)) { > > + event_notifier_set(&svq->hdev_kick); > > > Any reason we need to kick the device directly here? > At this point of the series only notifications are forwarded, not buffers. If kick_fd is set, we need to check the old one, the same way as vhost checks the masked notifier in case of change. Thanks! [1] https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg03152.html , from "I'd suggest to not depend on this since it:" > Thanks > > > > + } > > +} > > + > > /** > > * Creates vhost shadow virtqueue, and instruct vhost device to use t= he shadow > > * methods and file descriptors. > > @@ -52,6 +94,9 @@ VhostShadowVirtqueue *vhost_svq_new(void) > > goto err_init_hdev_call; > > } > > > > + /* Placeholder descriptor, it should be deleted at set_kick_fd */ > > + event_notifier_init_fd(&svq->svq_kick, INVALID_SVQ_KICK_FD); > > + > > return g_steal_pointer(&svq); > > > > err_init_hdev_call: >