[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AM0PR04MB4723E4FB8898BF5464822C02D4699@AM0PR04MB4723.eurprd04.prod.outlook.com>
Date: Sun, 30 Apr 2023 18:18:25 +0000
From: Alvaro Karsz <alvaro.karsz@...id-run.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
CC: "jasowang@...hat.com" <jasowang@...hat.com>,
"davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"virtualization@...ts.linux-foundation.org"
<virtualization@...ts.linux-foundation.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"xuanzhuo@...ux.alibaba.com" <xuanzhuo@...ux.alibaba.com>
Subject: Re: [RFC PATCH net 1/3] virtio: re-negotiate features if probe fails
and features are blocked
> > +void virtio_block_feature(struct virtio_device *dev, unsigned int f)
> > +{
> > + BUG_ON(f >= 64);
> > + dev->blocked_features |= (1ULL << f);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_block_feature);
> > +
>
> Let's add documentation please. Also pls call it __virtio_block_feature
> since it has to be used in a special way - specifically only during
> probe.
>
Ok.
> > + /* Store blocked features and attempt to negotiate features & probe.
> > + * If the probe fails, we check if the driver has blocked any new features.
> > + * If it has, we reset the device and try again with the new features.
> > + */
> > + while (renegotiate) {
> > + blocked_features = dev->blocked_features;
> > + err = virtio_negotiate_features(dev);
> > + if (err)
> > + break;
> > +
> > + err = drv->probe(dev);
>
>
> there's no way to driver to clear blocked features, but
> just in case, I'd add BUG_ON to check.
>
Ok.
> > * @features: the features supported by both driver and device.
> > + * @blocked_features: the features blocked by the driver that can't be negotiated.
> > * @priv: private pointer for the driver's use.
> > */
> > struct virtio_device {
> > @@ -124,6 +125,7 @@ struct virtio_device {
> > const struct vringh_config_ops *vringh_config;
> > struct list_head vqs;
> > u64 features;
> > + u64 blocked_features;
>
> add comment here too, explain purpose and rules of use
>
Ok.
Powered by blists - more mailing lists