virtio: use u32, not bitmap for features

It seemed like a good idea to use bitmap for features
in struct virtio_device, but it's actually a pain,
and seems to become even more painful when we get more
than 32 feature bits.  Just change it to a u32 for now.

Based on patch by Rusty.

Suggested-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index cf7a561fad7cd9f3cdd94fe16e5c22d65318428c..8d00aa7f60f0d721da47d3a3325d1f9f5146e428 100644 (file)
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -355,7 +355,7 @@ static inline bool use_multiport(struct ports_device *portdev)
         */
        if (!portdev->vdev)
                return 0;
-       return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
+       return __virtio_test_bit(portdev->vdev, VIRTIO_CONSOLE_F_MULTIPORT);
 }
 
 static DEFINE_SPINLOCK(dma_bufs_lock);

diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index d0a1d8a45c815e347e249beaff4855455e1e708c..97aeb7dce5fcc0e7fa633ba7f2e364f4a4b9187d 100644 (file)
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -137,14 +137,14 @@ static void lg_finalize_features(struct virtio_device *vdev)
        vring_transport_features(vdev);
 
        /*
-        * The vdev->feature array is a Linux bitmask: this isn't the same as a
-        * the simple array of bits used by lguest devices for features.  So we
-        * do this slow, manual conversion which is completely general.
+        * Since lguest is currently x86-only, we're little-endian.  That
+        * means we could just memcpy.  But it's not time critical, and in
+        * case someone copies this code, we do it the slow, obvious way.
         */
        memset(out_features, 0, desc->feature_len);
        bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8;
        for (i = 0; i < bits; i++) {
-               if (test_bit(i, vdev->features))
+               if (__virtio_test_bit(vdev, i))
                        out_features[i / 8] |= (1 << (i % 8));
        }
 

diff --git a/drivers/misc/mic/card/mic_virtio.c b/drivers/misc/mic/card/mic_virtio.c
index e64794730e21561ed89011915da7c622b40e0c3a..4f070ad1875c21a1e1a103fc5782ae3830b36588 100644 (file)
--- a/drivers/misc/mic/card/mic_virtio.c
+++ b/drivers/misc/mic/card/mic_virtio.c
@@ -101,7 +101,7 @@ static void mic_finalize_features(struct virtio_device *vdev)
        bits = min_t(unsigned, feature_len,
                sizeof(vdev->features)) * 8;
        for (i = 0; i < bits; i++) {
-               if (test_bit(i, vdev->features))
+               if (__virtio_test_bit(vdev, i))
                        iowrite8(ioread8(&out_features[i / 8]) | (1 << (i % 8)),
                                 &out_features[i / 8]);
        }

diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c
index a34b50690b4efd28a3616d98f304185b723233b8..dafaf38dac0a2c959a5b58b03dc58cb8f7451a8f 100644 (file)
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -231,7 +231,7 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev)
         * Remember the finalized features of our vdev, and provide it
         * to the remote processor once it is powered on.
         */
-       rsc->gfeatures = vdev->features[0];
+       rsc->gfeatures = vdev->features;
 }
 
 static void rproc_virtio_get(struct virtio_device *vdev, unsigned offset,

diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 643129070c51e6fa8883b39216cfd269e439b7e6..fcd312d0c0184b0cd35074021d3f39b186fa3360 100644 (file)
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -106,7 +106,7 @@ static void kvm_finalize_features(struct virtio_device *vdev)
        memset(out_features, 0, desc->feature_len);
        bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8;
        for (i = 0; i < bits; i++) {
-               if (test_bit(i, vdev->features))
+               if (__virtio_test_bit(vdev, i))
                        out_features[i / 8] |= (1 << (i % 8));
        }
 }

diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
index bda52f18e9670aefe1a7757209fdf038c866ad5a..1dbee95838fe3965b228b7907a86e9ee36a2755c 100644 (file)
--- a/drivers/s390/kvm/virtio_ccw.c
+++ b/drivers/s390/kvm/virtio_ccw.c
@@ -701,7 +701,6 @@ static void virtio_ccw_finalize_features(struct virtio_device *vdev)
 {
        struct virtio_ccw_device *vcdev = to_vc_device(vdev);
        struct virtio_feature_desc *features;
-       int i;
        struct ccw1 *ccw;
 
        ccw = kzalloc(sizeof(*ccw), GFP_DMA | GFP_KERNEL);
@@ -715,19 +714,15 @@ static void virtio_ccw_finalize_features(struct virtio_device *vdev)
        /* Give virtio_ring a chance to accept features. */
        vring_transport_features(vdev);
 
-       for (i = 0; i < sizeof(*vdev->features) / sizeof(features->features);
-            i++) {
-               int highbits = i % 2 ? 32 : 0;
-               features->index = i;
-               features->features = cpu_to_le32(vdev->features[i / 2]
-                                                >> highbits);
-               /* Write the feature bits to the host. */
-               ccw->cmd_code = CCW_CMD_WRITE_FEAT;
-               ccw->flags = 0;
-               ccw->count = sizeof(*features);
-               ccw->cda = (__u32)(unsigned long)features;
-               ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT);
-       }
+       features->index = 0;
+       features->features = cpu_to_le32(vdev->features);
+       /* Write the feature bits to the host. */
+       ccw->cmd_code = CCW_CMD_WRITE_FEAT;
+       ccw->flags = 0;
+       ccw->count = sizeof(*features);
+       ccw->cda = (__u32)(unsigned long)features;
+       ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_WRITE_FEAT);
+
 out_free:
        kfree(features);
        kfree(ccw);

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index df598dd8c5c8c42ae366307d409aa5423c0c7fba..2b9aafb4d6792d4f676b752e4aa0031cc6ec8674 100644 (file)
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -49,9 +49,9 @@ static ssize_t features_show(struct device *_d,
 
        /* We actually represent this as a bitstring, as it could be
         * arbitrary length in future. */
-       for (i = 0; i < ARRAY_SIZE(dev->features)*BITS_PER_LONG; i++)
+       for (i = 0; i < sizeof(dev->features)*8; i++)
                len += sprintf(buf+len, "%c",
-                              test_bit(i, dev->features) ? '1' : '0');
+                              __virtio_test_bit(dev, i) ? '1' : '0');
        len += sprintf(buf+len, "\n");
        return len;
 }
@@ -168,18 +168,18 @@ static int virtio_dev_probe(struct device *_d)
        device_features = dev->config->get_features(dev);
 
        /* Features supported by both device and driver into dev->features. */
-       memset(dev->features, 0, sizeof(dev->features));
+       dev->features = 0;
        for (i = 0; i < drv->feature_table_size; i++) {
                unsigned int f = drv->feature_table[i];
                BUG_ON(f >= 32);
                if (device_features & (1 << f))
-                       set_bit(f, dev->features);
+                       __virtio_set_bit(dev, f);
        }
 
        /* Transport features always preserved to pass to finalize_features. */
        for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++)
                if (device_features & (1 << i))
-                       set_bit(i, dev->features);
+                       __virtio_set_bit(dev, i);
 
        dev->config->finalize_features(dev);
 

diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
index ef9a1650bb804147410f5f8c7ce2bb70426f2728..eb5b0e2b434ad61612274deb2f096c1d32387d0e 100644 (file)
--- a/drivers/virtio/virtio_mmio.c
+++ b/drivers/virtio/virtio_mmio.c
@@ -155,16 +155,12 @@ static u32 vm_get_features(struct virtio_device *vdev)
 static void vm_finalize_features(struct virtio_device *vdev)
 {
        struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev);
-       int i;
 
        /* Give virtio_ring a chance to accept features. */
        vring_transport_features(vdev);
 
-       for (i = 0; i < ARRAY_SIZE(vdev->features); i++) {
-               writel(i, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL);
-               writel(vdev->features[i],
-                               vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES);
-       }
+       writel(0, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL);
+       writel(vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES);
 }
 
 static void vm_get(struct virtio_device *vdev, unsigned offset,

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index d34ebfa604f3dc40b38fef4e467325049a1c0b4f..4e112c1584889cf282630c4e4ffac43028b0cf2b 100644 (file)
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -120,8 +120,7 @@ static void vp_finalize_features(struct virtio_device *vdev)
        vring_transport_features(vdev);
 
        /* We only support 32 feature bits. */
-       BUILD_BUG_ON(ARRAY_SIZE(vdev->features) != 1);
-       iowrite32(vdev->features[0], vp_dev->ioaddr+VIRTIO_PCI_GUEST_FEATURES);
+       iowrite32(vdev->features, vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES);
 }
 
 /* virtio config->get() implementation */

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 3b1f89b6e7435576315c1b75e7754e3658080200..839247cd8263114bf9bd9096b71a5e9abbcd36b5 100644 (file)
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -781,7 +781,7 @@ void vring_transport_features(struct virtio_device *vdev)
                        break;
                default:
                        /* We don't understand this bit. */
-                       clear_bit(i, vdev->features);
+                       __virtio_clear_bit(vdev, i);
                }
        }
 }

diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 65261a7244fcea2e3d0c2562e40e6ba4e5b1bc48..7828a7f47c2c55573cb8cfe4e91466c60412ee41 100644 (file)
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -101,8 +101,7 @@ struct virtio_device {
        const struct virtio_config_ops *config;
        const struct vringh_config_ops *vringh_config;
        struct list_head vqs;
-       /* Note that this is a Linux set_bit-style bitmap. */
-       unsigned long features[1];
+       u32 features;
        void *priv;
 };
 

diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index d8e28a2a5738fe06d58490ab8587249fd64f8149..ffc2ae04879cd626a4021051b95aeff2563dd9d3 100644 (file)
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -92,7 +92,7 @@ static inline bool __virtio_test_bit(const struct virtio_device *vdev,
        else
                BUG_ON(fbit >= 32);
 
-       return test_bit(fbit, vdev->features);
+       return vdev->features & BIT(fbit);
 }
 
 /**
@@ -109,7 +109,7 @@ static inline void __virtio_set_bit(struct virtio_device *vdev,
        else
                BUG_ON(fbit >= 32);
 
-       set_bit(fbit, vdev->features);
+       vdev->features |= BIT(fbit);
 }
 
 /**
@@ -126,7 +126,7 @@ static inline void __virtio_clear_bit(struct virtio_device *vdev,
        else
                BUG_ON(fbit >= 32);
 
-       clear_bit(fbit, vdev->features);
+       vdev->features &= ~BIT(fbit);
 }
 
 /**

diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h
index 5a2d1f0f6bc7cd9c47658ff0a03d53c314a6f16a..72bff70bfeeb4fa5283026d293dbe358ff7660f4 100644 (file)
--- a/tools/virtio/linux/virtio.h
+++ b/tools/virtio/linux/virtio.h
@@ -6,31 +6,11 @@
 /* TODO: empty stubs for now. Broken but enough for virtio_ring.c */
 #define list_add_tail(a, b) do {} while (0)
 #define list_del(a) do {} while (0)
-
-#define BIT_WORD(nr)           ((nr) / BITS_PER_LONG)
-#define BITS_PER_BYTE          8
-#define BITS_PER_LONG (sizeof(long) * BITS_PER_BYTE)
-#define BIT_MASK(nr)           (1UL << ((nr) % BITS_PER_LONG))
-
-/* TODO: Not atomic as it should be:
- * we don't use this for anything important. */
-static inline void clear_bit(int nr, volatile unsigned long *addr)
-{
-       unsigned long mask = BIT_MASK(nr);
-       unsigned long *p = ((unsigned long *)addr) + BIT_WORD(nr);
-
-       *p &= ~mask;
-}
-
-static inline int test_bit(int nr, const volatile unsigned long *addr)
-{
-        return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1)));
-}
 /* end of stubs */
 
 struct virtio_device {
        void *dev;
-       unsigned long features[1];
+       u32 features;
 };
 
 struct virtqueue {

diff --git a/tools/virtio/linux/virtio_config.h b/tools/virtio/linux/virtio_config.h
index 5049967f99f7d2becb208844d07f6f5a49a9f681..83b27e8e9d72699552c0d8271f5771df050b7343 100644 (file)
--- a/tools/virtio/linux/virtio_config.h
+++ b/tools/virtio/linux/virtio_config.h
@@ -2,5 +2,5 @@
 #define VIRTIO_TRANSPORT_F_END         32
 
 #define virtio_has_feature(dev, feature) \
-       test_bit((feature), (dev)->features)
+       (__virtio_test_bit((dev), feature))
 

diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 00ea679b3826e8fa92971767ae66fb613e428dcd..db3437c641a64c467d3d7f01d84ffdb8670ef4ea 100644 (file)
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -60,7 +60,7 @@ void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
 {
        struct vhost_vring_state state = { .index = info->idx };
        struct vhost_vring_file file = { .index = info->idx };
-       unsigned long long features = dev->vdev.features[0];
+       unsigned long long features = dev->vdev.features;
        struct vhost_vring_addr addr = {
                .index = info->idx,
                .desc_user_addr = (uint64_t)(unsigned long)info->vring.desc,
@@ -113,8 +113,7 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
 {
        int r;
        memset(dev, 0, sizeof *dev);
-       dev->vdev.features[0] = features;
-       dev->vdev.features[1] = features >> 32;
+       dev->vdev.features = features;
        dev->buf_size = 1024;
        dev->buf = malloc(dev->buf_size);
        assert(dev->buf);

diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 14a4f4cab5b97a1070abc656bca1bc72c60b5aa1..9d4b1bca54be581d5c33f716ea9a2830c15d5866 100644 (file)
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -304,7 +304,7 @@ static int parallel_test(unsigned long features,
                close(to_guest[1]);
                close(to_host[0]);
 
-               gvdev.vdev.features[0] = features;
+               gvdev.vdev.features = features;
                gvdev.to_host_fd = to_host[1];
                gvdev.notifies = 0;
 
@@ -449,13 +449,13 @@ int main(int argc, char *argv[])
        bool fast_vringh = false, parallel = false;
 
        getrange = getrange_iov;
-       vdev.features[0] = 0;
+       vdev.features = 0;
 
        while (argv[1]) {
                if (strcmp(argv[1], "--indirect") == 0)
-                       vdev.features[0] |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
+                       __virtio_set_bit(&vdev, VIRTIO_RING_F_INDIRECT_DESC);
                else if (strcmp(argv[1], "--eventidx") == 0)
-                       vdev.features[0] |= (1 << VIRTIO_RING_F_EVENT_IDX);
+                       __virtio_set_bit(&vdev, VIRTIO_RING_F_EVENT_IDX);
                else if (strcmp(argv[1], "--slow-range") == 0)
                        getrange = getrange_slow;
                else if (strcmp(argv[1], "--fast-vringh") == 0)
@@ -468,7 +468,7 @@ int main(int argc, char *argv[])
        }
 
        if (parallel)
-               return parallel_test(vdev.features[0], getrange, fast_vringh);
+               return parallel_test(vdev.features, getrange, fast_vringh);
 
        if (posix_memalign(&__user_addr_min, PAGE_SIZE, USER_MEM) != 0)
                abort();
@@ -483,7 +483,7 @@ int main(int argc, char *argv[])
 
        /* Set up host side. */
        vring_init(&vrh.vring, RINGSIZE, __user_addr_min, ALIGN);
-       vringh_init_user(&vrh, vdev.features[0], RINGSIZE, true,
+       vringh_init_user(&vrh, vdev.features, RINGSIZE, true,
                         vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
 
        /* No descriptor to get yet... */
@@ -652,13 +652,13 @@ int main(int argc, char *argv[])
        }
 
        /* Test weird (but legal!) indirect. */
-       if (vdev.features[0] & (1 << VIRTIO_RING_F_INDIRECT_DESC)) {
+       if (__virtio_test_bit(&vdev, VIRTIO_RING_F_INDIRECT_DESC)) {
                char *data = __user_addr_max - USER_MEM/4;
                struct vring_desc *d = __user_addr_max - USER_MEM/2;
                struct vring vring;
 
                /* Force creation of direct, which we modify. */
-               vdev.features[0] &= ~(1 << VIRTIO_RING_F_INDIRECT_DESC);
+               __virtio_clear_bit(&vdev, VIRTIO_RING_F_INDIRECT_DESC);
                vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, &vdev, true,
                                         __user_addr_min,
                                         never_notify_host,