Virtualization - Nov 2014 - [PATCH v4 11/42] virtio: add legacy feature table support

virtio blk has some legacy feature bits that modern drivers
must not negotiate, but are needed for old legacy hosts
(e.g. that dn't support virtio scsi).
Allow a separate legacy feature table for such cases.

Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
---
 include/linux/virtio.h  |  4 ++++
 drivers/virtio/virtio.c | 25 ++++++++++++++++++++++++-
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d6359a5..f70411e 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -130,6 +130,8 @@ int virtio_device_restore(struct virtio_device *dev);
  * @id_table: the ids serviced by this driver.
  * @feature_table: an array of feature numbers supported by this driver.
  * @feature_table_size: number of entries in the feature table array.
+ * @feature_table_legacy: same as feature_table but when working in legacy
mode.
+ * @feature_table_size_legacy: number of entries in feature table legacy array.
  * @probe: the function to call when a device is found.  Returns 0 or -errno.
  * @remove: the function to call when a device is removed.
  * @config_changed: optional function to call when the device configuration
@@ -140,6 +142,8 @@ struct virtio_driver {
 	const struct virtio_device_id *id_table;
 	const unsigned int *feature_table;
 	unsigned int feature_table_size;
+	const unsigned int *feature_table_legacy;
+	unsigned int feature_table_size_legacy;
 	int (*probe)(struct virtio_device *dev);
 	void (*scan)(struct virtio_device *dev);
 	void (*remove)(struct virtio_device *dev);
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 0f44cff..e9018b4 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -113,6 +113,13 @@ void virtio_check_driver_offered_feature(const struct
virtio_device *vdev,
 	for (i = 0; i < drv->feature_table_size; i++)
 		if (drv->feature_table[i] == fbit)
 			return;
+
+	if (drv->feature_table_legacy) {
+		for (i = 0; i < drv->feature_table_size_legacy; i++)
+			if (drv->feature_table_legacy[i] == fbit)
+				return;
+	}
+
 	BUG();
 }
 EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
@@ -161,6 +168,7 @@ static int virtio_dev_probe(struct device *_d)
 	struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
 	u64 device_features;
 	u64 driver_features;
+	u64 driver_features_legacy;
 	unsigned status;
 
 	/* We have a driver! */
@@ -177,7 +185,22 @@ static int virtio_dev_probe(struct device *_d)
 		driver_features |= (1ULL << f);
 	}
 
-	dev->features = driver_features & device_features;
+	/* Some drivers have a separate feature tables for virtio v1.0 */
+	if (drv->feature_table_legacy) {
+		driver_features_legacy = 0;
+		for (i = 0; i < drv->feature_table_size_legacy; i++) {
+			unsigned int f = drv->feature_table_legacy[i];
+			BUG_ON(f >= 64);
+			driver_features_legacy |= (1ULL << f);
+		}
+	} else {
+		driver_features_legacy = driver_features;
+	}
+
+	if (driver_features & device_features & (1ULL <<
VIRTIO_F_VERSION_1))
+		dev->features = driver_features & device_features;
+	else
+		dev->features = driver_features_legacy & device_features;
 
 	/* Transport features always preserved to pass to finalize_features. */
 	for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++)
-- 
MST

On Tue, 25 Nov 2014 18:42:09 +0200
"Michael S. Tsirkin" <mst at redhat.com> wrote:
> virtio blk has some legacy feature bits that modern drivers
virtio-blk
> must not negotiate, but are needed for old legacy hosts
> (e.g. that dn't support virtio scsi).
(that e.g. don't support virtio-scsi)
> Allow a separate legacy feature table for such cases.
> 
> Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> ---
>  include/linux/virtio.h  |  4 ++++
>  drivers/virtio/virtio.c | 25 ++++++++++++++++++++++++-
>  2 files changed, 28 insertions(+), 1 deletion(-)
> 
> @@ -177,7 +185,22 @@ static int virtio_dev_probe(struct device *_d)
>  		driver_features |= (1ULL << f);
>  	}
> 
> -	dev->features = driver_features & device_features;
> +	/* Some drivers have a separate feature tables for virtio v1.0 */
/* some drivers have a separate feature table for legacy virtio */

?
> +	if (drv->feature_table_legacy) {
> +		driver_features_legacy = 0;
> +		for (i = 0; i < drv->feature_table_size_legacy; i++) {
> +			unsigned int f = drv->feature_table_legacy[i];
> +			BUG_ON(f >= 64);
> +			driver_features_legacy |= (1ULL << f);
> +		}
> +	} else {
> +		driver_features_legacy = driver_features;
> +	}
(...)

On Tue, Nov 25, 2014 at 06:53:42PM +0100, Cornelia Huck
wrote:
> On Tue, 25 Nov 2014 18:42:09 +0200
> "Michael S. Tsirkin" <mst at redhat.com> wrote:
> 
> > virtio blk has some legacy feature bits that modern drivers
> 
> virtio-blk
> 
> > must not negotiate, but are needed for old legacy hosts
> > (e.g. that dn't support virtio scsi).
> 
> (that e.g. don't support virtio-scsi)
> 
> > Allow a separate legacy feature table for such cases.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst at redhat.com>
> > ---
> >  include/linux/virtio.h  |  4 ++++
> >  drivers/virtio/virtio.c | 25 ++++++++++++++++++++++++-
> >  2 files changed, 28 insertions(+), 1 deletion(-)
> > 
> 
> > @@ -177,7 +185,22 @@ static int virtio_dev_probe(struct device *_d)
> >  		driver_features |= (1ULL << f);
> >  	}
> > 
> > -	dev->features = driver_features & device_features;
> > +	/* Some drivers have a separate feature tables for virtio v1.0 */
> 
> /* some drivers have a separate feature table for legacy virtio */
> 
> ?
I prefer have separate feature tables:  drivers (plural) have tables
(plural).

Agree?
> > +	if (drv->feature_table_legacy) {
> > +		driver_features_legacy = 0;
> > +		for (i = 0; i < drv->feature_table_size_legacy; i++) {
> > +			unsigned int f = drv->feature_table_legacy[i];
> > +			BUG_ON(f >= 64);
> > +			driver_features_legacy |= (1ULL << f);
> > +		}
> > +	} else {
> > +		driver_features_legacy = driver_features;
> > +	}
> (...)