virtio: explicit advertisement of driver features

A recent proposed feature addition to the virtio block driver revealed
some flaws in the API: in particular, we assume that feature
negotiation is complete once a driver's probe function returns.

There is nothing in the API to require this, however, and even I
didn't notice when it was violated.

So instead, we require the driver to specify what features it supports
in a table, we can then move the feature negotiation into the virtio
core.  The intersection of device and driver features are presented in
a new 'features' bitmap in the struct virtio_device.

Note that this highlights the difference between Linux unsigned-long
bitmaps where each unsigned long is in native endian, and a
straight-forward little-endian array of bytes.

Drivers can still remove feature bits in their probe routine if they
really have to.

API changes:
- dev->config->feature() no longer gets and acks a feature.
- drivers should advertise their features in the 'feature_table' field
- use virtio_has_feature() for extra sanity when checking feature bits

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

1 files changed, 28 insertions, 20 deletions

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 2bc9bf7e88e5..7a643a6ee9a1 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -85,27 +85,34 @@ static unsigned desc_size(const struct lguest_device_desc *desc)
                 + desc->config_len;
 }
 
-/* This tests (and acknowleges) a feature bit. */
-static bool lg_feature(struct virtio_device *vdev, unsigned fbit)
+/* This gets the device's feature bits. */
+static u32 lg_get_features(struct virtio_device *vdev)
 {
+        unsigned int i;
+        u32 features = 0;
+        struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
+        u8 *in_features = lg_features(desc);
+
+        /* We do this the slow but generic way. */
+        for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+                if (in_features[i / 8] & (1 << (i % 8)))
+                        features |= (1 << i);
+
+        return features;
+}
+
+static void lg_set_features(struct virtio_device *vdev, u32 features)
+{
+        unsigned int i;
         struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
-        u8 *features;
-
-        /* Obviously if they ask for a feature off the end of our feature
-         * bitmap, it's not set. */
-        if (fbit / 8 > desc->feature_len)
-                return false;
-
-        /* The feature bitmap comes after the virtqueues. */
-        features = lg_features(desc);
-        if (!(features[fbit / 8] & (1 << (fbit % 8))))
-                return false;
-
-        /* We set the matching bit in the other half of the bitmap to tell the
-         * Host we want to use this feature.  We don't use this yet, but we
-         * could in future. */
-        features[desc->feature_len + fbit / 8] |= (1 << (fbit % 8));
-        return true;
+        /* Second half of bitmap is features we accept. */
+        u8 *out_features = lg_features(desc) + desc->feature_len;
+
+        memset(out_features, 0, desc->feature_len);
+        for (i = 0; i < min(desc->feature_len * 8, 32); i++) {
+                if (features & (1 << i))
+                        out_features[i / 8] |= (1 << (i % 8));
+        }
 }
 
 /* Once they've found a field, getting a copy of it is easy. */
@@ -286,7 +293,8 @@ static void lg_del_vq(struct virtqueue *vq)
 
 /* The ops structure which hooks everything together. */
 static struct virtio_config_ops lguest_config_ops = {
-        .feature = lg_feature,
+        .get_features = lg_get_features,
+        .set_features = lg_set_features,
         .get = lg_get,
         .set = lg_set,
         .get_status = lg_get_status,