From e16e12be34648777606a2c03a3526409b38f0e63 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 7 Oct 2014 16:39:42 +0200 Subject: 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 Signed-off-by: Rusty Russell Signed-off-by: Cornelia Huck Signed-off-by: Michael S. Tsirkin Reviewed-by: Cornelia Huck --- tools/virtio/linux/virtio.h | 22 +--------------------- tools/virtio/linux/virtio_config.h | 2 +- tools/virtio/virtio_test.c | 5 ++--- tools/virtio/vringh_test.c | 16 ++++++++-------- 4 files changed, 12 insertions(+), 33 deletions(-) (limited to 'tools') diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h index 5a2d1f0f6bc7..72bff70bfeeb 100644 --- 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 5049967f99f7..83b27e8e9d72 100644 --- 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 00ea679b3826..db3437c641a6 100644 --- 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 14a4f4cab5b9..9d4b1bca54be 100644 --- 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, -- cgit v1.2.3 From d025477368792b272802146a86e41f81a54d8a19 Mon Sep 17 00:00:00 2001 From: "Michael S. Tsirkin" Date: Tue, 7 Oct 2014 16:39:43 +0200 Subject: virtio: add support for 64 bit features. Change u32 to u64, and use BIT_ULL and 1ULL everywhere. Note: transports are unchanged, and only set low 32 bit. This guarantees that no transport sets e.g. VERSION_1 by mistake without proper support. Based on patch by Rusty. Signed-off-by: Rusty Russell Signed-off-by: Cornelia Huck Signed-off-by: Michael S. Tsirkin Reviewed-by: David Hildenbrand Reviewed-by: Cornelia Huck --- drivers/lguest/lguest_device.c | 2 +- drivers/misc/mic/card/mic_virtio.c | 2 +- drivers/remoteproc/remoteproc_virtio.c | 2 +- drivers/s390/kvm/kvm_virtio.c | 2 +- drivers/s390/kvm/virtio_ccw.c | 2 +- drivers/virtio/virtio.c | 8 ++++---- drivers/virtio/virtio_mmio.c | 2 +- drivers/virtio/virtio_pci.c | 2 +- include/linux/virtio.h | 2 +- include/linux/virtio_config.h | 20 ++++++++++---------- tools/virtio/linux/virtio.h | 2 +- 11 files changed, 23 insertions(+), 23 deletions(-) (limited to 'tools') diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index 97aeb7dce5fc..d81170a78a8d 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -94,7 +94,7 @@ static unsigned desc_size(const struct lguest_device_desc *desc) } /* This gets the device's feature bits. */ -static u32 lg_get_features(struct virtio_device *vdev) +static u64 lg_get_features(struct virtio_device *vdev) { unsigned int i; u32 features = 0; diff --git a/drivers/misc/mic/card/mic_virtio.c b/drivers/misc/mic/card/mic_virtio.c index d5da9ff646cb..f5e756132bdb 100644 --- a/drivers/misc/mic/card/mic_virtio.c +++ b/drivers/misc/mic/card/mic_virtio.c @@ -68,7 +68,7 @@ static inline struct device *mic_dev(struct mic_vdev *mvdev) } /* This gets the device's feature bits. */ -static u32 mic_get_features(struct virtio_device *vdev) +static u64 mic_get_features(struct virtio_device *vdev) { unsigned int i, bits; u32 features = 0; diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index dafaf38dac0a..62897db1331e 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -207,7 +207,7 @@ static void rproc_virtio_reset(struct virtio_device *vdev) } /* provide the vdev features as retrieved from the firmware */ -static u32 rproc_virtio_get_features(struct virtio_device *vdev) +static u64 rproc_virtio_get_features(struct virtio_device *vdev) { struct rproc_vdev *rvdev = vdev_to_rvdev(vdev); struct fw_rsc_vdev *rsc; diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index fcd312d0c018..2336c7e3b0cf 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -80,7 +80,7 @@ static unsigned desc_size(const struct kvm_device_desc *desc) } /* This gets the device's feature bits. */ -static u32 kvm_get_features(struct virtio_device *vdev) +static u64 kvm_get_features(struct virtio_device *vdev) { unsigned int i; u32 features = 0; diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c index 1dbee95838fe..56d78956a8cb 100644 --- a/drivers/s390/kvm/virtio_ccw.c +++ b/drivers/s390/kvm/virtio_ccw.c @@ -660,7 +660,7 @@ static void virtio_ccw_reset(struct virtio_device *vdev) kfree(ccw); } -static u32 virtio_ccw_get_features(struct virtio_device *vdev) +static u64 virtio_ccw_get_features(struct virtio_device *vdev) { struct virtio_ccw_device *vcdev = to_vc_device(vdev); struct virtio_feature_desc *features; diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 2b9aafb4d679..746d350c8062 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -159,7 +159,7 @@ static int virtio_dev_probe(struct device *_d) int err, i; struct virtio_device *dev = dev_to_virtio(_d); struct virtio_driver *drv = drv_to_virtio(dev->dev.driver); - u32 device_features; + u64 device_features; /* We have a driver! */ add_status(dev, VIRTIO_CONFIG_S_DRIVER); @@ -171,14 +171,14 @@ static int virtio_dev_probe(struct device *_d) 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)) + BUG_ON(f >= 64); + if (device_features & (1ULL << f)) __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)) + if (device_features & (1ULL << i)) __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 eb5b0e2b434a..c63d0efa947b 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -142,7 +142,7 @@ struct virtio_mmio_vq_info { /* Configuration interface */ -static u32 vm_get_features(struct virtio_device *vdev) +static u64 vm_get_features(struct virtio_device *vdev) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 4e112c158488..7e0efa7fc384 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -102,7 +102,7 @@ static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev) } /* virtio config->get_features() implementation */ -static u32 vp_get_features(struct virtio_device *vdev) +static u64 vp_get_features(struct virtio_device *vdev) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 7828a7f47c2c..149284e5196d 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -101,7 +101,7 @@ struct virtio_device { const struct virtio_config_ops *config; const struct vringh_config_ops *vringh_config; struct list_head vqs; - u32 features; + u64 features; void *priv; }; diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index ffc2ae04879c..f51788439574 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -66,7 +66,7 @@ struct virtio_config_ops { vq_callback_t *callbacks[], const char *names[]); void (*del_vqs)(struct virtio_device *); - u32 (*get_features)(struct virtio_device *vdev); + u64 (*get_features)(struct virtio_device *vdev); void (*finalize_features)(struct virtio_device *vdev); const char *(*bus_name)(struct virtio_device *vdev); int (*set_vq_affinity)(struct virtqueue *vq, int cpu); @@ -88,11 +88,11 @@ static inline bool __virtio_test_bit(const struct virtio_device *vdev, { /* Did you forget to fix assumptions on max features? */ if (__builtin_constant_p(fbit)) - BUILD_BUG_ON(fbit >= 32); + BUILD_BUG_ON(fbit >= 64); else - BUG_ON(fbit >= 32); + BUG_ON(fbit >= 64); - return vdev->features & BIT(fbit); + return vdev->features & BIT_ULL(fbit); } /** @@ -105,11 +105,11 @@ static inline void __virtio_set_bit(struct virtio_device *vdev, { /* Did you forget to fix assumptions on max features? */ if (__builtin_constant_p(fbit)) - BUILD_BUG_ON(fbit >= 32); + BUILD_BUG_ON(fbit >= 64); else - BUG_ON(fbit >= 32); + BUG_ON(fbit >= 64); - vdev->features |= BIT(fbit); + vdev->features |= BIT_ULL(fbit); } /** @@ -122,11 +122,11 @@ static inline void __virtio_clear_bit(struct virtio_device *vdev, { /* Did you forget to fix assumptions on max features? */ if (__builtin_constant_p(fbit)) - BUILD_BUG_ON(fbit >= 32); + BUILD_BUG_ON(fbit >= 64); else - BUG_ON(fbit >= 32); + BUG_ON(fbit >= 64); - vdev->features &= ~BIT(fbit); + vdev->features &= ~BIT_ULL(fbit); } /** diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h index 72bff70bfeeb..8eb6421761cd 100644 --- a/tools/virtio/linux/virtio.h +++ b/tools/virtio/linux/virtio.h @@ -10,7 +10,7 @@ struct virtio_device { void *dev; - u32 features; + u64 features; }; struct virtqueue { -- cgit v1.2.3