Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Cornelia Huck <cohuck@redhat.com>
To:     "Michael S. Tsirkin" <mst@redhat.com>,
        Jason Wang <jasowang@redhat.com>
Cc:     linux-kernel@vger.kernel.org, stable@vger.kernel.org,
        Halil Pasic <pasic@linux.ibm.com>,
        virtualization@lists.linux-foundation.org
Subject: Re: [PATCH] virtio: acknowledge all features before access
In-Reply-To: <20220117032429-mutt-send-email-mst@kernel.org>
Organization: Red Hat GmbH
References: <20220114200744.150325-1-mst@redhat.com>
 <d6c4e521-1538-bbbf-30e6-f658a095b3ae@redhat.com>
 <20220117032429-mutt-send-email-mst@kernel.org>
User-Agent: Notmuch/0.34 (https://notmuchmail.org)
Date:   Mon, 17 Jan 2022 13:38:42 +0100
Message-ID: <87mtjuv8od.fsf@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Jan 17 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Jan 17, 2022 at 02:31:49PM +0800, Jason Wang wrote:
>>=20
>> =E5=9C=A8 2022/1/15 =E4=B8=8A=E5=8D=884:09, Michael S. Tsirkin =E5=86=99=
=E9=81=93:
>> > @@ -495,6 +494,10 @@ int virtio_device_restore(struct virtio_device *d=
ev)
>> >   	/* We have a driver! */
>> >   	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
>> > +	ret =3D dev->config->finalize_features(dev);
>> > +	if (ret)
>> > +		goto err;
>>=20
>>=20
>> Is this part of code related?
>>=20
>> Thanks
>>=20
>
> Yes. virtio_finalize_features no longer calls dev->config->finalize_featu=
res.
>
> I think the dev->config->finalize_features callback is actually
> a misnomer now, it just sends the features to device,
> finalize is FEATURES_OK. Renaming that is a bigger
> patch though, and I'd like this one to be cherry-pickable
> to stable.

Do we want to add a comment before the calls to ->finalize_features()
(/* write features to device */) and adapt the comment in virtio_ring.h?
Should still be stable-friendly, and giving the callback a better name
can be a follow-up patch.

>
>> > +
>> >   	ret =3D virtio_finalize_features(dev);
>> >   	if (ret)
>> >   		goto err;