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 X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E4995C49EA6 for ; Thu, 24 Jun 2021 04:46:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D0FB4613DC for ; Thu, 24 Jun 2021 04:46:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230104AbhFXEtE (ORCPT ); Thu, 24 Jun 2021 00:49:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53452 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230046AbhFXEtC (ORCPT ); Thu, 24 Jun 2021 00:49:02 -0400 Received: from mail-ej1-x631.google.com (mail-ej1-x631.google.com [IPv6:2a00:1450:4864:20::631]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4AC21C06175F for ; Wed, 23 Jun 2021 21:46:43 -0700 (PDT) Received: by mail-ej1-x631.google.com with SMTP id hc16so7329433ejc.12 for ; Wed, 23 Jun 2021 21:46:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=qV+mylXL/2SS4mJCMLehWyslEuC3Qq8x/PXCguBgY/I=; b=yLsRfVae0FF0DXCc80Qe7rbCzVIrdmQDyeaVXSgU7731bg1QMqw4lgqLCxjyZUZjwu TDH7cb0lIajOurwizpXzJMb1Ld/YEZ/vOZ/tKUWon1ytyFmk7qtEyniUaT9wfomHXaCZ Q5k7DOlqy4oW2+TRbKidZkfTZk1foP9dbr9CEZLKOgBzOUjNXSWZEPhSFmyl+eKMt16P UetkLAJ4Ll73DccWxMTSvwQYbyiWwHF4SXeF68p10765hT8FeXCxeTOqWo6XDYYUASaW U0fLxWjMmOWM2r455SZayrz6m1rBoWQox1axwCaI6sMaC6VcTv4xi2V6T6wMsgxpGUUR 4qsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=qV+mylXL/2SS4mJCMLehWyslEuC3Qq8x/PXCguBgY/I=; b=Cyv4FThg0mR37ntrPlkA+Xf2lc/eybLdAwJDgRXVbSSMm9b6b6EP7xsaPCQFnkPz9w nlm/IjyOH0vo7Yu29jN+K6SnriBfEd3rvMthXBjCYJBNsd8dgCZPYD7INLPKji/TLOum WHN6xZ2xFP2ph0L4b59YGH3cE8QyShYfkrXly6lyhEnnhxT997QojVBhPZ3DutUZEWDw 9qUwO03nnz5NigbqvztZ46AW9iMxB8zRIH3xSZiVpQfOiHP8CR7Oq+lys1xWsGoaqyOm FpmbewkT9tB9thll2LALf2uXwMqxceWgY093NcVu2Hmcqk7ISlAuu3xtXAJtZW/d4tly 3iXg== X-Gm-Message-State: AOAM530vEEhu0hQIO05lJHPhvqOPyzNTZDh3VEVdjm4eK863MfkTr9ZF fj7YWKjwwDI7yXtQMV7ebk2Qi4Yn1sGrn+7Wl4Ko X-Google-Smtp-Source: ABdhPJxLv6gMYjF4IzwXDzBGPms1QJcwsfrq54qSD0tH6LJ386sUw0R2U1ERHQHkfRm0DArekdU8YwZzZP3+ubsA+P4= X-Received: by 2002:a17:906:3c4a:: with SMTP id i10mr3283231ejg.372.1624510001769; Wed, 23 Jun 2021 21:46:41 -0700 (PDT) MIME-Version: 1.0 References: <20210615141331.407-1-xieyongji@bytedance.com> <20210615141331.407-10-xieyongji@bytedance.com> <1bba439f-ffc8-c20e-e8a4-ac73e890c592@redhat.com> <0aeb7cb7-58e5-1a95-d830-68edd7e8ec2e@redhat.com> In-Reply-To: From: Yongji Xie Date: Thu, 24 Jun 2021 12:46:30 +0800 Message-ID: Subject: Re: Re: [PATCH v8 09/10] vduse: Introduce VDUSE - vDPA Device in Userspace To: Jason Wang Cc: "Michael S. Tsirkin" , Stefan Hajnoczi , Stefano Garzarella , Parav Pandit , Christoph Hellwig , Christian Brauner , Randy Dunlap , Matthew Wilcox , Al Viro , Jens Axboe , bcrl@kvack.org, Jonathan Corbet , =?UTF-8?Q?Mika_Penttil=C3=A4?= , Dan Carpenter , joro@8bytes.org, Greg KH , songmuchun@bytedance.com, virtualization , netdev@vger.kernel.org, kvm , linux-fsdevel@vger.kernel.org, iommu@lists.linux-foundation.org, linux-kernel Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 24, 2021 at 11:35 AM Jason Wang wrote: > > > =E5=9C=A8 2021/6/23 =E4=B8=8B=E5=8D=881:50, Yongji Xie =E5=86=99=E9=81=93= : > > On Wed, Jun 23, 2021 at 11:31 AM Jason Wang wrote= : > >> > >> =E5=9C=A8 2021/6/22 =E4=B8=8B=E5=8D=884:14, Yongji Xie =E5=86=99=E9=81= =93: > >>> On Tue, Jun 22, 2021 at 3:50 PM Jason Wang wrot= e: > >>>> =E5=9C=A8 2021/6/22 =E4=B8=8B=E5=8D=883:22, Yongji Xie =E5=86=99=E9= =81=93: > >>>>>> We need fix a way to propagate the error to the userspace. > >>>>>> > >>>>>> E.g if we want to stop the deivce, we will delay the status reset = until > >>>>>> we get respose from the userspace? > >>>>>> > >>>>> I didn't get how to delay the status reset. And should it be a DoS > >>>>> that we want to fix if the userspace doesn't give a response foreve= r? > >>>> You're right. So let's make set_status() can fail first, then propag= ate > >>>> its failure via VHOST_VDPA_SET_STATUS. > >>>> > >>> OK. So we only need to propagate the failure in the vhost-vdpa case, = right? > >> > >> I think not, we need to deal with the reset for virtio as well: > >> > >> E.g in register_virtio_devices(), we have: > >> > >> /* We always start by resetting the device, in case a previo= us > >> * driver messed it up. This also tests that code path a > >> little. */ > >> dev->config->reset(dev); > >> > >> We probably need to make reset can fail and then fail the > >> register_virtio_device() as well. > >> > > OK, looks like virtio_add_status() and virtio_device_ready()[1] should > > be also modified if we need to propagate the failure in the > > virtio-vdpa case. Or do we only need to care about the reset case? > > > > [1] https://lore.kernel.org/lkml/20210517093428.670-1-xieyongji@bytedan= ce.com/ > > > My understanding is DRIVER_OK is not something that needs to be validated= : > > " > > DRIVER_OK (4) > Indicates that the driver is set up and ready to drive the device. > > " > > Since the spec doesn't require to re-read the and check if DRIVER_OK is > set in 3.1.1 Driver Requirements: Device Initialization. > > It's more about "telling the device that driver is ready." > > But we don have some status bit that requires the synchronization with > the device. > > 1) FEATURES_OK, spec requires to re-read the status bit to check whether > or it it was set by the device: > > " > > Re-read device status to ensure the FEATURES_OK bit is still set: > otherwise, the device does not support our subset of features and the > device is unusable. > > " > > This is useful for some device which can only support a subset of the > features. E.g a device that can only work for packed virtqueue. This > means the current design of set_features won't work, we need either: > > 1a) relay the set_features request to userspace > > or > > 1b) introduce a mandated_device_features during device creation and > validate the driver features during the set_features(), and don't set > FEATURES_OK if they don't match. > > > 2) Some transports (PCI) requires to re-read the status to ensure the > synchronization. > > " > > After writing 0 to device_status, the driver MUST wait for a read of > device_status to return 0 before reinitializing the device. > > " > > So we need to deal with both FEATURES_OK and reset, but probably not > DRIVER_OK. > OK, I see. Thanks for the explanation. One more question is how about clearing the corresponding status bit in get_status() rather than making set_status() fail. Since the spec recommends this way for validation which is done in virtio_dev_remove() and virtio_finalize_features(). Thanks, Yongji