Aiming these for coming merge window. Nothing should change, but get ready for a non-guest-endian config transports, and > 32 features bits. Final one is just an overdue consolidation. Cheers, Rusty. Rusty Russell (7): virtio_config: introduce size-based accessors. virtio: use size-based config accessors. virtio_config: helpers for non-converting accessors. virtio_config: make transports implement accessors. virtio: use u32, not bitmap for struct virtio_device's features virtio: add support for 64 bit features. virtio: move vring structure into struct virtqueue. drivers/block/virtio_blk.c | 77 +++++------ drivers/char/virtio_console.c | 17 +-- drivers/lguest/lguest_device.c | 36 +++-- drivers/net/caif/caif_virtio.c | 25 ++-- drivers/net/virtio_net.c | 28 ++-- drivers/remoteproc/remoteproc_virtio.c | 8 +- drivers/s390/kvm/kvm_virtio.c | 34 ++--- drivers/s390/kvm/virtio_ccw.c | 71 +++++++--- drivers/scsi/virtio_scsi.c | 12 +- drivers/virtio/virtio.c | 87 +++++++++++-- drivers/virtio/virtio_balloon.c | 10 +- drivers/virtio/virtio_mmio.c | 41 +++--- drivers/virtio/virtio_pci.c | 33 ++--- drivers/virtio/virtio_ring.c | 116 +++++++---------- include/linux/virtio.h | 11 +- include/linux/virtio_config.h | 224 ++++++++++++++++++++++++++------ net/9p/trans_virtio.c | 9 +- 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 +-- 21 files changed, 534 insertions(+), 350 deletions(-) -- 1.7.10.4
Rusty Russell
2013-Apr-05 03:36 UTC
[PATCH 1/7] virtio_config: introduce size-based accessors.
This lets the transport do endian conversion if necessary, and insulates the drivers from that change. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- include/linux/virtio_config.h | 161 ++++++++++++++++++++++++++++++++++------- 1 file changed, 134 insertions(+), 27 deletions(-) diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index 29b9104..e8f8f71 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -96,33 +96,6 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev, return test_bit(fbit, vdev->features); } -/** - * virtio_config_val - look for a feature and get a virtio config entry. - * @vdev: the virtio device - * @fbit: the feature bit - * @offset: the type to search for. - * @v: a pointer to the value to fill in. - * - * The return value is -ENOENT if the feature doesn't exist. Otherwise - * the config value is copied into whatever is pointed to by v. */ -#define virtio_config_val(vdev, fbit, offset, v) \ - virtio_config_buf((vdev), (fbit), (offset), (v), sizeof(*v)) - -#define virtio_config_val_len(vdev, fbit, offset, v, len) \ - virtio_config_buf((vdev), (fbit), (offset), (v), (len)) - -static inline int virtio_config_buf(struct virtio_device *vdev, - unsigned int fbit, - unsigned int offset, - void *buf, unsigned len) -{ - if (!virtio_has_feature(vdev, fbit)) - return -ENOENT; - - vdev->config->get(vdev, offset, buf, len); - return 0; -} - static inline struct virtqueue *virtio_find_single_vq(struct virtio_device *vdev, vq_callback_t *c, const char *n) @@ -162,5 +135,139 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) return 0; } +/* Config space accessors. */ +#define virtio_cread(vdev, structname, member, ptr) \ + do { \ + /* Must match the member's type, and be integer */ \ + if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \ + (*ptr) = 1; \ + \ + switch (sizeof(*ptr)) { \ + case 1: \ + *(ptr) = virtio_cread8(vdev, \ + offsetof(structname, member)); \ + break; \ + case 2: \ + *(ptr) = virtio_cread16(vdev, \ + offsetof(structname, member)); \ + break; \ + case 4: \ + *(ptr) = virtio_cread32(vdev, \ + offsetof(structname, member)); \ + break; \ + case 8: \ + *(ptr) = virtio_cread64(vdev, \ + offsetof(structname, member)); \ + break; \ + default: \ + BUG(); \ + } \ + } while(0) + +/* Config space accessors. */ +#define virtio_cwrite(vdev, structname, member, ptr) \ + do { \ + /* Must match the member's type, and be integer */ \ + if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \ + BUG_ON((*ptr) == 1); \ + \ + switch (sizeof(*ptr)) { \ + case 1: \ + virtio_cwrite8(vdev, \ + offsetof(structname, member), \ + *(ptr)); \ + break; \ + case 2: \ + virtio_cwrite16(vdev, \ + offsetof(structname, member), \ + *(ptr)); \ + break; \ + case 4: \ + virtio_cwrite32(vdev, \ + offsetof(structname, member), \ + *(ptr)); \ + break; \ + case 8: \ + virtio_cwrite64(vdev, \ + offsetof(structname, member), \ + *(ptr)); \ + break; \ + default: \ + BUG(); \ + } \ + } while(0) + +static inline u8 virtio_cread8(struct virtio_device *vdev, unsigned int offset) +{ + u8 ret; + vdev->config->get(vdev, offset, &ret, sizeof(ret)); + return ret; +} + +static inline void virtio_cread_bytes(struct virtio_device *vdev, + unsigned int offset, + void *buf, size_t len) +{ + vdev->config->get(vdev, offset, buf, len); +} + +static inline void virtio_cwrite8(struct virtio_device *vdev, + unsigned int offset, u8 val) +{ + vdev->config->set(vdev, offset, &val, sizeof(val)); +} + +static inline u16 virtio_cread16(struct virtio_device *vdev, + unsigned int offset) +{ + u16 ret; + vdev->config->get(vdev, offset, &ret, sizeof(ret)); + return ret; +} + +static inline void virtio_cwrite16(struct virtio_device *vdev, + unsigned int offset, u16 val) +{ + vdev->config->set(vdev, offset, &val, sizeof(val)); +} + +static inline u32 virtio_cread32(struct virtio_device *vdev, + unsigned int offset) +{ + u32 ret; + vdev->config->get(vdev, offset, &ret, sizeof(ret)); + return ret; +} + +static inline void virtio_cwrite32(struct virtio_device *vdev, + unsigned int offset, u32 val) +{ + vdev->config->set(vdev, offset, &val, sizeof(val)); +} + +static inline u64 virtio_cread64(struct virtio_device *vdev, + unsigned int offset) +{ + u64 ret; + vdev->config->get(vdev, offset, &ret, sizeof(ret)); + return ret; +} + +static inline void virtio_cwrite64(struct virtio_device *vdev, + unsigned int offset, u64 val) +{ + vdev->config->set(vdev, offset, &val, sizeof(val)); +} + +/* Conditional config space accessors. */ +#define virtio_cread_feature(vdev, fbit, structname, member, ptr) \ + ({ \ + int _r = 0; \ + if (!virtio_has_feature(vdev, fbit)) \ + _r = -ENOENT; \ + else \ + virtio_cread((vdev), structname, member, ptr); \ + _r; \ + }) #endif /* _LINUX_VIRTIO_CONFIG_H */ -- 1.7.10.4
This lets the transport do endian conversion if necessary, and insulates the drivers from the difference. Most drivers can use the simple helpers virtio_cread() and virtio_cwrite(). Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/block/virtio_blk.c | 77 +++++++++++++++++---------------------- drivers/char/virtio_console.c | 15 +++----- drivers/net/caif/caif_virtio.c | 23 ++++++------ drivers/net/virtio_net.c | 28 ++++++++------ drivers/scsi/virtio_scsi.c | 12 ++---- drivers/virtio/virtio_balloon.c | 10 ++--- net/9p/trans_virtio.c | 9 ++--- 7 files changed, 79 insertions(+), 95 deletions(-) diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 6472395..1b1bbd3 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -456,18 +456,15 @@ static int virtblk_ioctl(struct block_device *bdev, fmode_t mode, static int virtblk_getgeo(struct block_device *bd, struct hd_geometry *geo) { struct virtio_blk *vblk = bd->bd_disk->private_data; - struct virtio_blk_geometry vgeo; - int err; /* see if the host passed in geometry config */ - err = virtio_config_val(vblk->vdev, VIRTIO_BLK_F_GEOMETRY, - offsetof(struct virtio_blk_config, geometry), - &vgeo); - - if (!err) { - geo->heads = vgeo.heads; - geo->sectors = vgeo.sectors; - geo->cylinders = vgeo.cylinders; + if (virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_GEOMETRY)) { + virtio_cread(vblk->vdev, struct virtio_blk_config, + geometry.cylinders, &geo->cylinders); + virtio_cread(vblk->vdev, struct virtio_blk_config, + geometry.heads, &geo->heads); + virtio_cread(vblk->vdev, struct virtio_blk_config, + geometry.sectors, &geo->sectors); } else { /* some standard values, similar to sd */ geo->heads = 1 << 6; @@ -529,8 +526,7 @@ static void virtblk_config_changed_work(struct work_struct *work) goto done; /* Host must always specify the capacity. */ - vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity), - &capacity, sizeof(capacity)); + virtio_cread(vdev, struct virtio_blk_config, capacity, &capacity); /* If capacity is too big, truncate with warning. */ if ((sector_t)capacity != capacity) { @@ -608,9 +604,9 @@ static int virtblk_get_cache_mode(struct virtio_device *vdev) u8 writeback; int err; - err = virtio_config_val(vdev, VIRTIO_BLK_F_CONFIG_WCE, - offsetof(struct virtio_blk_config, wce), - &writeback); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE, + struct virtio_blk_config, wce, + &writeback); if (err) writeback = virtio_has_feature(vdev, VIRTIO_BLK_F_WCE); @@ -642,7 +638,6 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr, struct virtio_blk *vblk = disk->private_data; struct virtio_device *vdev = vblk->vdev; int i; - u8 writeback; BUG_ON(!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_CONFIG_WCE)); for (i = ARRAY_SIZE(virtblk_cache_types); --i >= 0; ) @@ -652,11 +647,7 @@ virtblk_cache_type_store(struct device *dev, struct device_attribute *attr, if (i < 0) return -EINVAL; - writeback = i; - vdev->config->set(vdev, - offsetof(struct virtio_blk_config, wce), - &writeback, sizeof(writeback)); - + virtio_cwrite8(vdev, offsetof(struct virtio_blk_config, wce), i); virtblk_update_cache_mode(vdev); return count; } @@ -699,9 +690,9 @@ static int virtblk_probe(struct virtio_device *vdev) index = err; /* We need to know how many segments before we allocate. */ - err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX, - offsetof(struct virtio_blk_config, seg_max), - &sg_elems); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SEG_MAX, + struct virtio_blk_config, seg_max, + &sg_elems); /* We need at least one SG element, whatever they say. */ if (err || !sg_elems) @@ -772,8 +763,7 @@ static int virtblk_probe(struct virtio_device *vdev) set_disk_ro(vblk->disk, 1); /* Host must always specify the capacity. */ - vdev->config->get(vdev, offsetof(struct virtio_blk_config, capacity), - &cap, sizeof(cap)); + virtio_cread(vdev, struct virtio_blk_config, capacity, &cap); /* If capacity is too big, truncate with warning. */ if ((sector_t)cap != cap) { @@ -794,46 +784,45 @@ static int virtblk_probe(struct virtio_device *vdev) /* Host can optionally specify maximum segment size and number of * segments. */ - err = virtio_config_val(vdev, VIRTIO_BLK_F_SIZE_MAX, - offsetof(struct virtio_blk_config, size_max), - &v); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX, + struct virtio_blk_config, size_max, &v); if (!err) blk_queue_max_segment_size(q, v); else blk_queue_max_segment_size(q, -1U); /* Host can optionally specify the block size of the device */ - err = virtio_config_val(vdev, VIRTIO_BLK_F_BLK_SIZE, - offsetof(struct virtio_blk_config, blk_size), - &blk_size); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE, + struct virtio_blk_config, blk_size, + &blk_size); if (!err) blk_queue_logical_block_size(q, blk_size); else blk_size = queue_logical_block_size(q); /* Use topology information if available */ - err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY, - offsetof(struct virtio_blk_config, physical_block_exp), - &physical_block_exp); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY, + struct virtio_blk_config, physical_block_exp, + &physical_block_exp); if (!err && physical_block_exp) blk_queue_physical_block_size(q, blk_size * (1 << physical_block_exp)); - err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY, - offsetof(struct virtio_blk_config, alignment_offset), - &alignment_offset); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY, + struct virtio_blk_config, alignment_offset, + &alignment_offset); if (!err && alignment_offset) blk_queue_alignment_offset(q, blk_size * alignment_offset); - err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY, - offsetof(struct virtio_blk_config, min_io_size), - &min_io_size); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY, + struct virtio_blk_config, min_io_size, + &min_io_size); if (!err && min_io_size) blk_queue_io_min(q, blk_size * min_io_size); - err = virtio_config_val(vdev, VIRTIO_BLK_F_TOPOLOGY, - offsetof(struct virtio_blk_config, opt_io_size), - &opt_io_size); + err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY, + struct virtio_blk_config, opt_io_size, + &opt_io_size); if (!err && opt_io_size) blk_queue_io_opt(q, blk_size * opt_io_size); diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index e6ba6b7..1735c38 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -1800,12 +1800,8 @@ static void config_intr(struct virtio_device *vdev) struct port *port; u16 rows, cols; - vdev->config->get(vdev, - offsetof(struct virtio_console_config, cols), - &cols, sizeof(u16)); - vdev->config->get(vdev, - offsetof(struct virtio_console_config, rows), - &rows, sizeof(u16)); + virtio_cread(vdev, struct virtio_console_config, cols, &cols); + virtio_cread(vdev, struct virtio_console_config, rows, &rows); port = find_port_by_id(portdev, 0); set_console_size(port, rows, cols); @@ -1977,10 +1973,9 @@ static int virtcons_probe(struct virtio_device *vdev) /* Don't test MULTIPORT at all if we're rproc: not a valid feature! */ if (!is_rproc_serial(vdev) && - virtio_config_val(vdev, VIRTIO_CONSOLE_F_MULTIPORT, - offsetof(struct virtio_console_config, - max_nr_ports), - &portdev->config.max_nr_ports) == 0) { + virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT, + struct virtio_console_config, max_nr_ports, + &portdev->config.max_nr_ports) == 0) { multiport = true; } diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c index 8308dee..ef602e3 100644 --- a/drivers/net/caif/caif_virtio.c +++ b/drivers/net/caif/caif_virtio.c @@ -682,18 +682,19 @@ static int cfv_probe(struct virtio_device *vdev) goto err; /* Get the CAIF configuration from virtio config space, if available */ -#define GET_VIRTIO_CONFIG_OPS(_v, _var, _f) \ - ((_v)->config->get(_v, offsetof(struct virtio_caif_transf_config, _f), \ - &_var, \ - FIELD_SIZEOF(struct virtio_caif_transf_config, _f))) - if (vdev->config->get) { - GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_hr, headroom); - GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_hr, headroom); - GET_VIRTIO_CONFIG_OPS(vdev, cfv->tx_tr, tailroom); - GET_VIRTIO_CONFIG_OPS(vdev, cfv->rx_tr, tailroom); - GET_VIRTIO_CONFIG_OPS(vdev, cfv->mtu, mtu); - GET_VIRTIO_CONFIG_OPS(vdev, cfv->mru, mtu); + virtio_cread(vdev, struct virtio_caif_transf_config, headroom, + &cfv->tx_hr); + virtio_cread(vdev, struct virtio_caif_transf_config, headroom, + &cfv->rx_hr); + virtio_cread(vdev, struct virtio_caif_transf_config, tailroom, + &cfv->tx_tr); + virtio_cread(vdev, struct virtio_caif_transf_config, tailroom, + &cfv->rx_tr); + virtio_cread(vdev, struct virtio_caif_transf_config, mtu, + &cfv->mtu); + virtio_cread(vdev, struct virtio_caif_transf_config, mtu, + &cfv->mru); } else { cfv->tx_hr = CFV_DEF_HEADROOM; cfv->rx_hr = CFV_DEF_HEADROOM; diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index be70487..61c1592 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -829,8 +829,13 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p) return -EINVAL; } } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { - vdev->config->set(vdev, offsetof(struct virtio_net_config, mac), - addr->sa_data, dev->addr_len); + unsigned int i; + + /* Naturally, this has an atomicity problem. */ + for (i = 0; i < dev->addr_len; i++) + virtio_cwrite8(vdev, + offsetof(struct virtio_net_config, mac) + + i, addr->sa_data[i]); } eth_commit_mac_addr_change(dev, p); @@ -1239,9 +1244,8 @@ static void virtnet_config_changed_work(struct work_struct *work) if (!vi->config_enable) goto done; - if (virtio_config_val(vi->vdev, VIRTIO_NET_F_STATUS, - offsetof(struct virtio_net_config, status), - &v) < 0) + if (virtio_cread_feature(vi->vdev, VIRTIO_NET_F_STATUS, + struct virtio_net_config, status, &v) < 0) goto done; if (v & VIRTIO_NET_S_ANNOUNCE) { @@ -1463,9 +1467,9 @@ static int virtnet_probe(struct virtio_device *vdev) u16 max_queue_pairs; /* Find if host supports multiqueue virtio_net device */ - err = virtio_config_val(vdev, VIRTIO_NET_F_MQ, - offsetof(struct virtio_net_config, - max_virtqueue_pairs), &max_queue_pairs); + err = virtio_cread_feature(vdev, VIRTIO_NET_F_MQ, + struct virtio_net_config, + max_virtqueue_pairs, &max_queue_pairs); /* We need at least 2 queue's */ if (err || max_queue_pairs < VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN || @@ -1513,9 +1517,11 @@ static int virtnet_probe(struct virtio_device *vdev) } /* Configuration may specify what MAC to use. Otherwise random. */ - if (virtio_config_val_len(vdev, VIRTIO_NET_F_MAC, - offsetof(struct virtio_net_config, mac), - dev->dev_addr, dev->addr_len) < 0) + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) + virtio_cread_bytes(vdev, + offsetof(struct virtio_net_config, mac), + dev->dev_addr, dev->addr_len); + else eth_hw_addr_random(dev); /* Set up our device-specific information */ diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index f679b8c..214d397 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -547,19 +547,15 @@ static struct scsi_host_template virtscsi_host_template = { #define virtscsi_config_get(vdev, fld) \ ({ \ typeof(((struct virtio_scsi_config *)0)->fld) __val; \ - vdev->config->get(vdev, \ - offsetof(struct virtio_scsi_config, fld), \ - &__val, sizeof(__val)); \ + virtio_cread(vdev, struct virtio_scsi_config, fld, &__val); \ __val; \ }) #define virtscsi_config_set(vdev, fld, val) \ - (void)({ \ + do { \ typeof(((struct virtio_scsi_config *)0)->fld) __val = (val); \ - vdev->config->set(vdev, \ - offsetof(struct virtio_scsi_config, fld), \ - &__val, sizeof(__val)); \ - }) + virtio_cwrite(vdev, struct virtio_scsi_config, fld, __val); \ + } while(0) static void virtscsi_init_vq(struct virtio_scsi_vq *virtscsi_vq, struct virtqueue *vq) diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 8dab163..bbab952 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -273,9 +273,8 @@ static inline s64 towards_target(struct virtio_balloon *vb) __le32 v; s64 target; - vb->vdev->config->get(vb->vdev, - offsetof(struct virtio_balloon_config, num_pages), - &v, sizeof(v)); + virtio_cread(vb->vdev, struct virtio_balloon_config, num_pages, &v); + target = le32_to_cpu(v); return target - vb->num_pages; } @@ -284,9 +283,8 @@ static void update_balloon_size(struct virtio_balloon *vb) { __le32 actual = cpu_to_le32(vb->num_pages); - vb->vdev->config->set(vb->vdev, - offsetof(struct virtio_balloon_config, actual), - &actual, sizeof(actual)); + virtio_cwrite(vb->vdev, struct virtio_balloon_config, num_pages, + &actual); } static int balloon(void *_vballoon) diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c index de2e950..d843d5d 100644 --- a/net/9p/trans_virtio.c +++ b/net/9p/trans_virtio.c @@ -514,9 +514,7 @@ static int p9_virtio_probe(struct virtio_device *vdev) chan->inuse = false; if (virtio_has_feature(vdev, VIRTIO_9P_MOUNT_TAG)) { - vdev->config->get(vdev, - offsetof(struct virtio_9p_config, tag_len), - &tag_len, sizeof(tag_len)); + virtio_cread(vdev, struct virtio_9p_config, tag_len, &tag_len); } else { err = -EINVAL; goto out_free_vq; @@ -526,8 +524,9 @@ static int p9_virtio_probe(struct virtio_device *vdev) err = -ENOMEM; goto out_free_vq; } - vdev->config->get(vdev, offsetof(struct virtio_9p_config, tag), - tag, tag_len); + + virtio_cread_bytes(vdev, offsetof(struct virtio_9p_config, tag), + tag, tag_len); chan->tag = tag; chan->tag_len = tag_len; err = sysfs_create_file(&(vdev->dev.kobj), &dev_attr_mount_tag.attr); -- 1.7.10.4
Rusty Russell
2013-Apr-05 03:37 UTC
[PATCH 3/7] virtio_config: helpers for non-converting accessors.
Simply redirect everything via the byte-at-a-time accessor. Slow, but simple and this is config reading, which mostly only happens at probe time. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/virtio/virtio.c | 69 +++++++++++++++++++++++++++++++++++++++++ include/linux/virtio_config.h | 19 ++++++++++++ 2 files changed, 88 insertions(+) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index ee59b74..2b154f3 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -219,6 +219,75 @@ out: } EXPORT_SYMBOL_GPL(register_virtio_device); +static void noconv_get(struct virtio_device *vdev, unsigned offset, + size_t len, void *p) +{ + u8 *buf = p; + while (len) { + *buf = vdev->config->get8(vdev, offset); + buf++; + offset++; + len--; + } +} + +u16 virtio_config_get_noconv16(struct virtio_device *vdev, unsigned offset) +{ + u16 v; + noconv_get(vdev, offset, sizeof(v), &v); + return v; +} +EXPORT_SYMBOL_GPL(virtio_config_get_noconv16); + +u32 virtio_config_get_noconv32(struct virtio_device *vdev, unsigned offset) +{ + u32 v; + noconv_get(vdev, offset, sizeof(v), &v); + return v; +} +EXPORT_SYMBOL_GPL(virtio_config_get_noconv32); + +u64 virtio_config_get_noconv64(struct virtio_device *vdev, unsigned offset) +{ + u64 v; + noconv_get(vdev, offset, sizeof(v), &v); + return v; +} +EXPORT_SYMBOL_GPL(virtio_config_get_noconv64); + +static void noconv_set(struct virtio_device *vdev, unsigned offset, + size_t len, const void *p) +{ + const u8 *buf = p; + while (len) { + vdev->config->set8(vdev, offset, *buf); + buf++; + offset++; + len--; + } +} + +void virtio_config_set_noconv16(struct virtio_device *vdev, + unsigned offset, u16 v) +{ + noconv_set(vdev, offset, sizeof(v), &v); +} +EXPORT_SYMBOL_GPL(virtio_config_set_noconv16); + +void virtio_config_set_noconv32(struct virtio_device *vdev, + unsigned offset, u32 v) +{ + noconv_set(vdev, offset, sizeof(v), &v); +} +EXPORT_SYMBOL_GPL(virtio_config_set_noconv32); + +void virtio_config_set_noconv64(struct virtio_device *vdev, + unsigned offset, u64 v) +{ + noconv_set(vdev, offset, sizeof(v), &v); +} +EXPORT_SYMBOL_GPL(virtio_config_set_noconv64); + void unregister_virtio_device(struct virtio_device *dev) { int index = dev->index; /* save for after device release */ diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index e8f8f71..12a777f 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -270,4 +270,23 @@ static inline void virtio_cwrite64(struct virtio_device *vdev, _r; \ }) +/* Helpers for non-endian converting transports. */ +u16 virtio_config_get_noconv16(struct virtio_device *vdev, unsigned offset); +u32 virtio_config_get_noconv32(struct virtio_device *vdev, unsigned offset); +u64 virtio_config_get_noconv64(struct virtio_device *vdev, unsigned offset); +void virtio_config_set_noconv16(struct virtio_device *vdev, + unsigned offset, u16 v); +void virtio_config_set_noconv32(struct virtio_device *vdev, + unsigned offset, u32 v); +void virtio_config_set_noconv64(struct virtio_device *vdev, + unsigned offset, u64 v); + +#define VIRTIO_CONFIG_OPS_NOCONV \ + .get16 = virtio_config_get_noconv16, \ + .set16 = virtio_config_set_noconv16, \ + .get32 = virtio_config_get_noconv32, \ + .set32 = virtio_config_set_noconv32, \ + .get64 = virtio_config_get_noconv64, \ + .set64 = virtio_config_set_noconv64 + #endif /* _LINUX_VIRTIO_CONFIG_H */ -- 1.7.10.4
Rusty Russell
2013-Apr-05 03:37 UTC
[PATCH 4/7] virtio_config: make transports implement accessors.
All transports just pass through at the moment. s390/kvm/virtio_ccw.c doesn't use the helper wrappers, since it does quite a lot of work for each config read. Cc: Ohad Ben-Cohen <ohad at wizery.com> Cc: Brian Swetland <swetland at google.com> Cc: Cornelia Huck <cornelia.huck at de.ibm.com> Cc: Pawel Moll <pawel.moll at arm.com> Cc: Christian Borntraeger <borntraeger at de.ibm.com> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/lguest/lguest_device.c | 20 ++++++------ drivers/net/caif/caif_virtio.c | 2 +- drivers/s390/kvm/kvm_virtio.c | 24 +++++++------- drivers/s390/kvm/virtio_ccw.c | 39 ++++++++++++++++++++-- drivers/virtio/virtio_mmio.c | 21 ++++-------- drivers/virtio/virtio_pci.c | 25 +++++--------- include/linux/virtio_config.h | 70 ++++++++++++++++++++++++---------------- 7 files changed, 118 insertions(+), 83 deletions(-) diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index b3256ff..baee77e 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -153,25 +153,22 @@ static void lg_finalize_features(struct virtio_device *vdev) } /* Once they've found a field, getting a copy of it is easy. */ -static void lg_get(struct virtio_device *vdev, unsigned int offset, - void *buf, unsigned len) +static u8 lg_get8(struct virtio_device *vdev, unsigned int offset) { struct lguest_device_desc *desc = to_lgdev(vdev)->desc; /* Check they didn't ask for more than the length of the config! */ - BUG_ON(offset + len > desc->config_len); - memcpy(buf, lg_config(desc) + offset, len); + BUG_ON(offset + sizeof(u8) > desc->config_len); + return *(u8 *)(lg_config(desc) + offset); } -/* Setting the contents is also trivial. */ -static void lg_set(struct virtio_device *vdev, unsigned int offset, - const void *buf, unsigned len) +static void lg_set8(struct virtio_device *vdev, unsigned int offset, u8 val) { struct lguest_device_desc *desc = to_lgdev(vdev)->desc; /* Check they didn't ask for more than the length of the config! */ - BUG_ON(offset + len > desc->config_len); - memcpy(lg_config(desc) + offset, buf, len); + BUG_ON(offset + sizeof(val) > desc->config_len); + *(u8 *)(lg_config(desc) + offset) = val; } /* @@ -399,8 +396,9 @@ static const char *lg_bus_name(struct virtio_device *vdev) static const struct virtio_config_ops lguest_config_ops = { .get_features = lg_get_features, .finalize_features = lg_finalize_features, - .get = lg_get, - .set = lg_set, + .get8 = lg_get8, + .set8 = lg_set8, + VIRTIO_CONFIG_OPS_NOCONV, .get_status = lg_get_status, .set_status = lg_set_status, .reset = lg_reset, diff --git a/drivers/net/caif/caif_virtio.c b/drivers/net/caif/caif_virtio.c index ef602e3..0f9bae0 100644 --- a/drivers/net/caif/caif_virtio.c +++ b/drivers/net/caif/caif_virtio.c @@ -682,7 +682,7 @@ static int cfv_probe(struct virtio_device *vdev) goto err; /* Get the CAIF configuration from virtio config space, if available */ - if (vdev->config->get) { + if (vdev->config->get8) { virtio_cread(vdev, struct virtio_caif_transf_config, headroom, &cfv->tx_hr); virtio_cread(vdev, struct virtio_caif_transf_config, headroom, diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index 6711e65..cde64d5 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -112,24 +112,25 @@ static void kvm_finalize_features(struct virtio_device *vdev) } /* - * Reading and writing elements in config space + * Reading and writing elements in config space. Host and guest are always + * big-endian, so no conversion necessary. */ -static void kvm_get(struct virtio_device *vdev, unsigned int offset, - void *buf, unsigned len) +static u8 kvm_get8(struct virtio_device *vdev, unsigned int offset) { struct kvm_device_desc *desc = to_kvmdev(vdev)->desc; - BUG_ON(offset + len > desc->config_len); - memcpy(buf, kvm_vq_configspace(desc) + offset, len); + /* Check they didn't ask for more than the length of the config! */ + BUG_ON(offset + sizeof(u8) > desc->config_len); + return *(u8 *)(kvm_vq_configspace(desc) + offset); } -static void kvm_set(struct virtio_device *vdev, unsigned int offset, - const void *buf, unsigned len) +static void kvm_set8(struct virtio_device *vdev, unsigned int offset, u8 val) { struct kvm_device_desc *desc = to_kvmdev(vdev)->desc; - BUG_ON(offset + len > desc->config_len); - memcpy(kvm_vq_configspace(desc) + offset, buf, len); + /* Check they didn't ask for more than the length of the config! */ + BUG_ON(offset + sizeof(val) > desc->config_len); + *(u8 *)(kvm_vq_configspace(desc) + offset) = val; } /* @@ -278,8 +279,9 @@ static const char *kvm_bus_name(struct virtio_device *vdev) static const struct virtio_config_ops kvm_vq_configspace_ops = { .get_features = kvm_get_features, .finalize_features = kvm_finalize_features, - .get = kvm_get, - .set = kvm_set, + .get8 = kvm_get8, + .set8 = kvm_set8, + VIRTIO_CONFIG_OPS_NOCONV, .get_status = kvm_get_status, .set_status = kvm_set_status, .reset = kvm_reset, diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c index 2029b6c..3652473 100644 --- a/drivers/s390/kvm/virtio_ccw.c +++ b/drivers/s390/kvm/virtio_ccw.c @@ -472,6 +472,7 @@ out_free: kfree(ccw); } +/* We don't need to do endian conversion, as it's always big endian like us */ static void virtio_ccw_get_config(struct virtio_device *vdev, unsigned int offset, void *buf, unsigned len) { @@ -505,6 +506,21 @@ out_free: kfree(ccw); } + +#define VIRTIO_CCW_GET_CONFIGx(bits) \ +static u##bits virtio_ccw_get_config##bits(struct virtio_device *vdev, \ + unsigned int offset) \ +{ \ + u##bits v; \ + virtio_ccw_get_config(vdev, offset, &v, sizeof(v)); \ + return v; \ +} + +VIRTIO_CCW_GET_CONFIGx(8) +VIRTIO_CCW_GET_CONFIGx(16) +VIRTIO_CCW_GET_CONFIGx(32) +VIRTIO_CCW_GET_CONFIGx(64) + static void virtio_ccw_set_config(struct virtio_device *vdev, unsigned int offset, const void *buf, unsigned len) @@ -535,6 +551,19 @@ out_free: kfree(ccw); } +#define VIRTIO_CCW_SET_CONFIGx(bits) \ +static void virtio_ccw_set_config##bits(struct virtio_device *vdev, \ + unsigned int offset, \ + u##bits v) \ +{ \ + virtio_ccw_set_config(vdev, offset, &v, sizeof(v)); \ +} + +VIRTIO_CCW_SET_CONFIGx(8) +VIRTIO_CCW_SET_CONFIGx(16) +VIRTIO_CCW_SET_CONFIGx(32) +VIRTIO_CCW_SET_CONFIGx(64) + static u8 virtio_ccw_get_status(struct virtio_device *vdev) { struct virtio_ccw_device *vcdev = to_vc_device(vdev); @@ -564,8 +593,14 @@ static void virtio_ccw_set_status(struct virtio_device *vdev, u8 status) static struct virtio_config_ops virtio_ccw_config_ops = { .get_features = virtio_ccw_get_features, .finalize_features = virtio_ccw_finalize_features, - .get = virtio_ccw_get_config, - .set = virtio_ccw_set_config, + .get8 = virtio_ccw_get_config8, + .set8 = virtio_ccw_set_config8, + .get16 = virtio_ccw_get_config16, + .set16 = virtio_ccw_set_config16, + .get32 = virtio_ccw_get_config32, + .set32 = virtio_ccw_set_config32, + .get64 = virtio_ccw_get_config64, + .set64 = virtio_ccw_set_config64, .get_status = virtio_ccw_get_status, .set_status = virtio_ccw_set_status, .reset = virtio_ccw_reset, diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 1ba0d68..4c2c6be 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -167,26 +167,18 @@ static void vm_finalize_features(struct virtio_device *vdev) } } -static void vm_get(struct virtio_device *vdev, unsigned offset, - void *buf, unsigned len) +static u8 vm_get8(struct virtio_device *vdev, unsigned offset) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); - u8 *ptr = buf; - int i; - for (i = 0; i < len; i++) - ptr[i] = readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i); + return readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset); } -static void vm_set(struct virtio_device *vdev, unsigned offset, - const void *buf, unsigned len) +static void vm_set8(struct virtio_device *vdev, unsigned offset, u8 v) { struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); - const u8 *ptr = buf; - int i; - for (i = 0; i < len; i++) - writeb(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i); + writeb(v, vm_dev->base + VIRTIO_MMIO_CONFIG + offset); } static u8 vm_get_status(struct virtio_device *vdev) @@ -424,8 +416,9 @@ static const char *vm_bus_name(struct virtio_device *vdev) } static const struct virtio_config_ops virtio_mmio_config_ops = { - .get = vm_get, - .set = vm_set, + .get8 = vm_get8, + .set8 = vm_set8, + VIRTIO_CONFIG_OPS_NOCONV, .get_status = vm_get_status, .set_status = vm_set_status, .reset = vm_reset, diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index a7ce730..fc01b77 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -127,33 +127,23 @@ static void vp_finalize_features(struct virtio_device *vdev) iowrite32(vdev->features[0], vp_dev->ioaddr+VIRTIO_PCI_GUEST_FEATURES); } -/* virtio config->get() implementation */ -static void vp_get(struct virtio_device *vdev, unsigned offset, - void *buf, unsigned len) +/* Device config access: we use guest endian, as per spec. */ +static u8 vp_get8(struct virtio_device *vdev, unsigned offset) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); void __iomem *ioaddr = vp_dev->ioaddr + VIRTIO_PCI_CONFIG(vp_dev) + offset; - u8 *ptr = buf; - int i; - for (i = 0; i < len; i++) - ptr[i] = ioread8(ioaddr + i); + return ioread8(ioaddr); } -/* the config->set() implementation. it's symmetric to the config->get() - * implementation */ -static void vp_set(struct virtio_device *vdev, unsigned offset, - const void *buf, unsigned len) +static void vp_set8(struct virtio_device *vdev, unsigned offset, u8 v) { struct virtio_pci_device *vp_dev = to_vp_device(vdev); void __iomem *ioaddr = vp_dev->ioaddr + VIRTIO_PCI_CONFIG(vp_dev) + offset; - const u8 *ptr = buf; - int i; - for (i = 0; i < len; i++) - iowrite8(ptr[i], ioaddr + i); + iowrite8(v, ioaddr); } /* config->{get,set}_status() implementations */ @@ -653,8 +643,9 @@ static int vp_set_vq_affinity(struct virtqueue *vq, int cpu) } static const struct virtio_config_ops virtio_pci_config_ops = { - .get = vp_get, - .set = vp_set, + .get8 = vp_get8, + .set8 = vp_set8, + VIRTIO_CONFIG_OPS_NOCONV, .get_status = vp_get_status, .set_status = vp_set_status, .reset = vp_reset, diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h index 12a777f..b7aa3b1 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -8,16 +8,30 @@ /** * virtio_config_ops - operations for configuring a virtio device - * @get: read the value of a configuration field + * @get8: read a byte from a configuration field * vdev: the virtio_device * offset: the offset of the configuration field - * buf: the buffer to write the field value into. - * len: the length of the buffer - * @set: write the value of a configuration field + * @set8: write a byte to a configuration field + * vdev: the virtio_device + * offset: the offset of the configuration field + * @get16: read a short from a configuration field + * vdev: the virtio_device + * offset: the offset of the configuration field + * @set16: write a short to a configuration field + * vdev: the virtio_device + * offset: the offset of the configuration field + * @get32: read a u32 from a configuration field + * vdev: the virtio_device + * offset: the offset of the configuration field + * @set32: write a u32 to a configuration field + * vdev: the virtio_device + * offset: the offset of the configuration field + * @get64: read a u64 from a configuration field + * vdev: the virtio_device + * offset: the offset of the configuration field + * @set64: write a u64 to a configuration field * vdev: the virtio_device * offset: the offset of the configuration field - * buf: the buffer to read the field value from. - * len: the length of the buffer * @get_status: read the status byte * vdev: the virtio_device * Returns the status byte @@ -54,10 +68,14 @@ */ typedef void vq_callback_t(struct virtqueue *); struct virtio_config_ops { - void (*get)(struct virtio_device *vdev, unsigned offset, - void *buf, unsigned len); - void (*set)(struct virtio_device *vdev, unsigned offset, - const void *buf, unsigned len); + u8 (*get8)(struct virtio_device *vdev, unsigned offset); + void (*set8)(struct virtio_device *vdev, unsigned offset, u8 val); + u16 (*get16)(struct virtio_device *vdev, unsigned offset); + void (*set16)(struct virtio_device *vdev, unsigned offset, u16 val); + u32 (*get32)(struct virtio_device *vdev, unsigned offset); + void (*set32)(struct virtio_device *vdev, unsigned offset, u32 val); + u64 (*get64)(struct virtio_device *vdev, unsigned offset); + void (*set64)(struct virtio_device *vdev, unsigned offset, u64 val); u8 (*get_status)(struct virtio_device *vdev); void (*set_status)(struct virtio_device *vdev, u8 status); void (*reset)(struct virtio_device *vdev); @@ -199,64 +217,62 @@ int virtqueue_set_affinity(struct virtqueue *vq, int cpu) static inline u8 virtio_cread8(struct virtio_device *vdev, unsigned int offset) { - u8 ret; - vdev->config->get(vdev, offset, &ret, sizeof(ret)); - return ret; + return vdev->config->get8(vdev, offset); } static inline void virtio_cread_bytes(struct virtio_device *vdev, unsigned int offset, void *buf, size_t len) { - vdev->config->get(vdev, offset, buf, len); + u8 *dst = buf; + while (len) { + *dst = vdev->config->get8(vdev, offset); + dst++; + offset++; + len--; + } } static inline void virtio_cwrite8(struct virtio_device *vdev, unsigned int offset, u8 val) { - vdev->config->set(vdev, offset, &val, sizeof(val)); + vdev->config->set8(vdev, offset, val); } static inline u16 virtio_cread16(struct virtio_device *vdev, unsigned int offset) { - u16 ret; - vdev->config->get(vdev, offset, &ret, sizeof(ret)); - return ret; + return vdev->config->get16(vdev, offset); } static inline void virtio_cwrite16(struct virtio_device *vdev, unsigned int offset, u16 val) { - vdev->config->set(vdev, offset, &val, sizeof(val)); + vdev->config->set16(vdev, offset, val); } static inline u32 virtio_cread32(struct virtio_device *vdev, unsigned int offset) { - u32 ret; - vdev->config->get(vdev, offset, &ret, sizeof(ret)); - return ret; + return vdev->config->get32(vdev, offset); } static inline void virtio_cwrite32(struct virtio_device *vdev, unsigned int offset, u32 val) { - vdev->config->set(vdev, offset, &val, sizeof(val)); + vdev->config->set32(vdev, offset, val); } static inline u64 virtio_cread64(struct virtio_device *vdev, unsigned int offset) { - u64 ret; - vdev->config->get(vdev, offset, &ret, sizeof(ret)); - return ret; + return vdev->config->get64(vdev, offset); } static inline void virtio_cwrite64(struct virtio_device *vdev, unsigned int offset, u64 val) { - vdev->config->set(vdev, offset, &val, sizeof(val)); + vdev->config->set64(vdev, offset, val); } /* Conditional config space accessors. */ -- 1.7.10.4
Rusty Russell
2013-Apr-05 03:37 UTC
[PATCH 5/7] virtio: use u32, not bitmap for struct virtio_device's features
It seemed like a good idea, but it's actually a pain when we get more than 32 feature bits. Just change it to a u32 for now. Cc: Ohad Ben-Cohen <ohad at wizery.com> Cc: Brian Swetland <swetland at google.com> Cc: Cornelia Huck <cornelia.huck at de.ibm.com> Cc: Christian Borntraeger <borntraeger at de.ibm.com> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> Acked-by: Pawel Moll <pawel.moll at arm.com> --- drivers/char/virtio_console.c | 2 +- drivers/lguest/lguest_device.c | 8 ++++---- drivers/remoteproc/remoteproc_virtio.c | 2 +- drivers/s390/kvm/kvm_virtio.c | 2 +- drivers/s390/kvm/virtio_ccw.c | 23 +++++++++-------------- drivers/virtio/virtio.c | 10 +++++----- drivers/virtio/virtio_mmio.c | 8 ++------ drivers/virtio/virtio_pci.c | 3 +-- drivers/virtio/virtio_ring.c | 2 +- include/linux/virtio.h | 3 +-- include/linux/virtio_config.h | 2 +- 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 ++++++++-------- 15 files changed, 39 insertions(+), 71 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 1735c38..9d9f717 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -351,7 +351,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 portdev->vdev->features & (1 << 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 baee77e..9528e64 100644 --- 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 (vdev->features & (1 << i)) out_features[i / 8] |= (1 << (i % 8)); } diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index afed9b7..fb8faee 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -219,7 +219,7 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) * fixed as part of a small resource table overhaul and then an * extension of the virtio resource entries. */ - rvdev->gfeatures = vdev->features[0]; + rvdev->gfeatures = vdev->features; } static const struct virtio_config_ops rproc_virtio_config_ops = { diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index cde64d5..8779224 100644 --- 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 (vdev->features & (1 << 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 3652473..8c564bf 100644 --- a/drivers/s390/kvm/virtio_ccw.c +++ b/drivers/s390/kvm/virtio_ccw.c @@ -440,7 +440,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); @@ -454,19 +453,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 2b154f3..8ebe913 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -41,9 +41,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'); + dev->features & (1ULL << i) ? '1' : '0'); len += sprintf(buf+len, "\n"); return len; } @@ -120,18 +120,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); + dev->features |= (1 << 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); + dev->features |= (1 << i); dev->config->finalize_features(dev); diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 4c2c6be..24d583a 100644 --- 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 u8 vm_get8(struct virtio_device *vdev, unsigned offset) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index fc01b77..aa85fdd 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -123,8 +123,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); } /* Device config access: we use guest endian, as per spec. */ diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 5217baf..82afdd8 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -814,7 +814,7 @@ void vring_transport_features(struct virtio_device *vdev) break; default: /* We don't understand this bit. */ - clear_bit(i, vdev->features); + vdev->features &= ~(1 << i); } } } diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 833f17b..80f55a0 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -100,8 +100,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 b7aa3b1..9a7f740 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -111,7 +111,7 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev, if (fbit < VIRTIO_TRANSPORT_F_START) virtio_check_driver_offered_feature(vdev, fbit); - return test_bit(fbit, vdev->features); + return vdev->features & (1 << fbit); } static inline diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h index 6df181a..02a38e8 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 5049967..1f1636b 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) + ((dev)->features & (1 << feature)) diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c index 814ae80..59f0706 100644 --- a/tools/virtio/virtio_test.c +++ b/tools/virtio/virtio_test.c @@ -59,7 +59,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, @@ -112,8 +112,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 88fe02a..d71fc05 100644 --- a/tools/virtio/vringh_test.c +++ b/tools/virtio/vringh_test.c @@ -299,7 +299,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; @@ -444,13 +444,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); + vdev.features |= (1 << VIRTIO_RING_F_INDIRECT_DESC); else if (strcmp(argv[1], "--eventidx") == 0) - vdev.features[0] |= (1 << VIRTIO_RING_F_EVENT_IDX); + vdev.features |= (1 << VIRTIO_RING_F_EVENT_IDX); else if (strcmp(argv[1], "--slow-range") == 0) getrange = getrange_slow; else if (strcmp(argv[1], "--fast-vringh") == 0) @@ -463,7 +463,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(); @@ -478,7 +478,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... */ @@ -646,13 +646,13 @@ int main(int argc, char *argv[]) } /* Test weird (but legal!) indirect. */ - if (vdev.features[0] & (1 << VIRTIO_RING_F_INDIRECT_DESC)) { + if (vdev.features & (1 << 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); + vdev.features &= ~(1 << VIRTIO_RING_F_INDIRECT_DESC); vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, &vdev, true, __user_addr_min, never_notify_host, -- 1.7.10.4
Change the u32 to a u64, and make sure to use 1ULL everywhere! Cc: Ohad Ben-Cohen <ohad at wizery.com> Cc: Brian Swetland <swetland at google.com> Cc: Cornelia Huck <cornelia.huck at de.ibm.com> Cc: Christian Borntraeger <borntraeger at de.ibm.com> Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> Acked-by: Pawel Moll <pawel.moll at arm.com> --- drivers/char/virtio_console.c | 2 +- drivers/lguest/lguest_device.c | 10 +++++----- drivers/remoteproc/remoteproc_virtio.c | 6 +++++- drivers/s390/kvm/kvm_virtio.c | 10 +++++----- drivers/s390/kvm/virtio_ccw.c | 13 +++++++++++-- drivers/virtio/virtio.c | 12 ++++++------ drivers/virtio/virtio_mmio.c | 14 +++++++++----- drivers/virtio/virtio_pci.c | 5 ++--- drivers/virtio/virtio_ring.c | 2 +- include/linux/virtio.h | 2 +- include/linux/virtio_config.h | 8 ++++---- tools/virtio/linux/virtio.h | 2 +- tools/virtio/linux/virtio_config.h | 2 +- 13 files changed, 52 insertions(+), 36 deletions(-) diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index 9d9f717..1a1e5da 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -351,7 +351,7 @@ static inline bool use_multiport(struct ports_device *portdev) */ if (!portdev->vdev) return 0; - return portdev->vdev->features & (1 << VIRTIO_CONSOLE_F_MULTIPORT); + return portdev->vdev->features & (1ULL << 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 9528e64..61dd97c 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -94,17 +94,17 @@ 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; + u64 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++) + for (i = 0; i < min(desc->feature_len * 8, 64); i++) if (in_features[i / 8] & (1 << (i % 8))) - features |= (1 << i); + features |= (1ULL << i); return features; } @@ -144,7 +144,7 @@ static void lg_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 (vdev->features & (1 << i)) + if (vdev->features & (1ULL << i)) out_features[i / 8] |= (1 << (i % 8)); } diff --git a/drivers/remoteproc/remoteproc_virtio.c b/drivers/remoteproc/remoteproc_virtio.c index fb8faee..aefbaae 100644 --- a/drivers/remoteproc/remoteproc_virtio.c +++ b/drivers/remoteproc/remoteproc_virtio.c @@ -196,7 +196,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); @@ -210,6 +210,9 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) /* Give virtio_ring a chance to accept features */ vring_transport_features(vdev); + /* Make sure we don't have any features > 32 bits! */ + BUG_ON((u32)vdev->features != vdev->features); + /* * Remember the finalized features of our vdev, and provide it * to the remote processor once it is powered on. @@ -220,6 +223,7 @@ static void rproc_virtio_finalize_features(struct virtio_device *vdev) * extension of the virtio resource entries. */ rvdev->gfeatures = vdev->features; + } static const struct virtio_config_ops rproc_virtio_config_ops = { diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c index 8779224..a023979 100644 --- a/drivers/s390/kvm/kvm_virtio.c +++ b/drivers/s390/kvm/kvm_virtio.c @@ -80,16 +80,16 @@ 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; + u64 features = 0; struct kvm_device_desc *desc = to_kvmdev(vdev)->desc; u8 *in_features = kvm_vq_features(desc); - for (i = 0; i < min(desc->feature_len * 8, 32); i++) + for (i = 0; i < min(desc->feature_len * 8, 64); i++) if (in_features[i / 8] & (1 << (i % 8))) - features |= (1 << i); + features |= (1ULL << i); return features; } @@ -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 (vdev->features & (1 << i)) + if (vdev->features & (1ULL << 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 8c564bf..bd67b26 100644 --- a/drivers/s390/kvm/virtio_ccw.c +++ b/drivers/s390/kvm/virtio_ccw.c @@ -454,8 +454,17 @@ static void virtio_ccw_finalize_features(struct virtio_device *vdev) vring_transport_features(vdev); features->index = 0; - features->features = cpu_to_le32(vdev->features); - /* Write the feature bits to the host. */ + features->features = cpu_to_le32((u32)vdev->features); + /* Write the first half of 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 = 1; + features->features = cpu_to_le32(vdev->features >> 32); + /* Write the second half of the feature bits to the host. */ ccw->cmd_code = CCW_CMD_WRITE_FEAT; ccw->flags = 0; ccw->count = sizeof(*features); diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 8ebe913..6ffa542 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -111,7 +111,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); @@ -123,15 +123,15 @@ 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)) - dev->features |= (1 << f); + BUG_ON(f >= 64); + if (device_features & (1ULL << f)) + dev->features |= (1ULL << 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)) - dev->features |= (1 << i); + if (device_features & (1ULL << i)) + dev->features |= (1ULL << i); dev->config->finalize_features(dev); diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 24d583a..0f1995c 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -142,14 +142,16 @@ 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); + u64 features; - /* TODO: Features > 32 bits */ writel(0, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL); - - return readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES); + features = readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES); + writel(1, vm_dev->base + VIRTIO_MMIO_HOST_FEATURES_SEL); + features |= ((u64)readl(vm_dev->base + VIRTIO_MMIO_HOST_FEATURES) << 32); + return features; } static void vm_finalize_features(struct virtio_device *vdev) @@ -160,7 +162,9 @@ static void vm_finalize_features(struct virtio_device *vdev) vring_transport_features(vdev); writel(0, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL); - writel(vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES); + writel((u32)vdev->features, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES); + writel(1, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES_SEL); + writel(vdev->features >> 32, vm_dev->base + VIRTIO_MMIO_GUEST_FEATURES); } static u8 vm_get8(struct virtio_device *vdev, unsigned offset) diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index aa85fdd..bfcce21 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -105,12 +105,11 @@ 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); - /* When someone needs more than 32 feature bits, we'll need to - * steal a bit to indicate that the rest are somewhere else. */ + /* We only support 32 feature bits. */ return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES); } diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 82afdd8..ba58e29 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -814,7 +814,7 @@ void vring_transport_features(struct virtio_device *vdev) break; default: /* We don't understand this bit. */ - vdev->features &= ~(1 << i); + vdev->features &= ~(1ULL << i); } } } diff --git a/include/linux/virtio.h b/include/linux/virtio.h index 80f55a0..a05f7c7 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -100,7 +100,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 9a7f740..9a9aaa0 100644 --- a/include/linux/virtio_config.h +++ b/include/linux/virtio_config.h @@ -84,7 +84,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); @@ -104,14 +104,14 @@ static inline bool virtio_has_feature(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); if (fbit < VIRTIO_TRANSPORT_F_START) virtio_check_driver_offered_feature(vdev, fbit); - return vdev->features & (1 << fbit); + return vdev->features & (1ULL << fbit); } static inline diff --git a/tools/virtio/linux/virtio.h b/tools/virtio/linux/virtio.h index 02a38e8..358706d 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 { diff --git a/tools/virtio/linux/virtio_config.h b/tools/virtio/linux/virtio_config.h index 1f1636b..a254c2b 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) \ - ((dev)->features & (1 << feature)) + ((dev)->features & (1ULL << feature)) -- 1.7.10.4
Rusty Russell
2013-Apr-05 03:37 UTC
[PATCH 7/7] virtio: move vring structure into struct virtqueue.
Back in 2010 (7c5e9ed0c84e7d70d887878574590638d5572659) MST removed the abstraction between virtio and the virtio ring, removing the various ops pointers, and we haven't really missed it. Now we hoist the struct vring out from the private struct vring_virtqueue into the struct virtqueue: we've already demonstrated that it's useful to be able to see the ring size, and the new virtio pci layout wants to know the location of each part of the ring. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> --- drivers/virtio/virtio_ring.c | 114 +++++++++++++++++------------------------- include/linux/virtio.h | 8 ++- 2 files changed, 54 insertions(+), 68 deletions(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index ba58e29..7f9d4e9 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -53,13 +53,9 @@ #define END_USE(vq) #endif -struct vring_virtqueue -{ +struct vring_virtqueue { struct virtqueue vq; - /* Actual memory layout for this queue */ - struct vring vring; - /* Can we use weak barriers? */ bool weak_barriers; @@ -171,12 +167,12 @@ static inline int vring_add_indirect(struct vring_virtqueue *vq, /* Use a single buffer which doesn't continue */ head = vq->free_head; - vq->vring.desc[head].flags = VRING_DESC_F_INDIRECT; - vq->vring.desc[head].addr = virt_to_phys(desc); - vq->vring.desc[head].len = i * sizeof(struct vring_desc); + vq->vq.vring.desc[head].flags = VRING_DESC_F_INDIRECT; + vq->vq.vring.desc[head].addr = virt_to_phys(desc); + vq->vq.vring.desc[head].len = i * sizeof(struct vring_desc); /* Update free pointer */ - vq->free_head = vq->vring.desc[head].next; + vq->free_head = vq->vq.vring.desc[head].next; return head; } @@ -226,7 +222,7 @@ static inline int virtqueue_add(struct virtqueue *_vq, goto add_head; } - BUG_ON(total_sg > vq->vring.num); + BUG_ON(total_sg > vq->vq.vring.num); BUG_ON(total_sg == 0); if (vq->vq.num_free < total_sg) { @@ -247,24 +243,24 @@ static inline int virtqueue_add(struct virtqueue *_vq, head = i = vq->free_head; for (n = 0; n < out_sgs; n++) { for (sg = sgs[n]; sg; sg = next(sg, &total_out)) { - vq->vring.desc[i].flags = VRING_DESC_F_NEXT; - vq->vring.desc[i].addr = sg_phys(sg); - vq->vring.desc[i].len = sg->length; + vq->vq.vring.desc[i].flags = VRING_DESC_F_NEXT; + vq->vq.vring.desc[i].addr = sg_phys(sg); + vq->vq.vring.desc[i].len = sg->length; prev = i; - i = vq->vring.desc[i].next; + i = vq->vq.vring.desc[i].next; } } for (; n < (out_sgs + in_sgs); n++) { for (sg = sgs[n]; sg; sg = next(sg, &total_in)) { - vq->vring.desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE; - vq->vring.desc[i].addr = sg_phys(sg); - vq->vring.desc[i].len = sg->length; + vq->vq.vring.desc[i].flags = VRING_DESC_F_NEXT|VRING_DESC_F_WRITE; + vq->vq.vring.desc[i].addr = sg_phys(sg); + vq->vq.vring.desc[i].len = sg->length; prev = i; - i = vq->vring.desc[i].next; + i = vq->vq.vring.desc[i].next; } } /* Last one doesn't continue. */ - vq->vring.desc[prev].flags &= ~VRING_DESC_F_NEXT; + vq->vq.vring.desc[prev].flags &= ~VRING_DESC_F_NEXT; /* Update free pointer */ vq->free_head = i; @@ -275,13 +271,13 @@ add_head: /* Put entry in available array (but don't update avail->idx until they * do sync). */ - avail = (vq->vring.avail->idx & (vq->vring.num-1)); - vq->vring.avail->ring[avail] = head; + avail = (vq->vq.vring.avail->idx & (vq->vq.vring.num-1)); + vq->vq.vring.avail->ring[avail] = head; /* Descriptors and available array need to be set before we expose the * new available array entries. */ virtio_wmb(vq->weak_barriers); - vq->vring.avail->idx++; + vq->vq.vring.avail->idx++; vq->num_added++; /* This is very unlikely, but theoretically possible. Kick @@ -431,8 +427,8 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq) * event. */ virtio_mb(vq->weak_barriers); - old = vq->vring.avail->idx - vq->num_added; - new = vq->vring.avail->idx; + old = vq->vq.vring.avail->idx - vq->num_added; + new = vq->vq.vring.avail->idx; vq->num_added = 0; #ifdef DEBUG @@ -444,10 +440,10 @@ bool virtqueue_kick_prepare(struct virtqueue *_vq) #endif if (vq->event) { - needs_kick = vring_need_event(vring_avail_event(&vq->vring), + needs_kick = vring_need_event(vring_avail_event(&vq->vq.vring), new, old); } else { - needs_kick = !(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY); + needs_kick = !(vq->vq.vring.used->flags&VRING_USED_F_NO_NOTIFY); } END_USE(vq); return needs_kick; @@ -497,15 +493,15 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) i = head; /* Free the indirect table */ - if (vq->vring.desc[i].flags & VRING_DESC_F_INDIRECT) - kfree(phys_to_virt(vq->vring.desc[i].addr)); + if (vq->vq.vring.desc[i].flags & VRING_DESC_F_INDIRECT) + kfree(phys_to_virt(vq->vq.vring.desc[i].addr)); - while (vq->vring.desc[i].flags & VRING_DESC_F_NEXT) { - i = vq->vring.desc[i].next; + while (vq->vq.vring.desc[i].flags & VRING_DESC_F_NEXT) { + i = vq->vq.vring.desc[i].next; vq->vq.num_free++; } - vq->vring.desc[i].next = vq->free_head; + vq->vq.vring.desc[i].next = vq->free_head; vq->free_head = head; /* Plus final descriptor */ vq->vq.num_free++; @@ -513,7 +509,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) static inline bool more_used(const struct vring_virtqueue *vq) { - return vq->last_used_idx != vq->vring.used->idx; + return vq->last_used_idx != vq->vq.vring.used->idx; } /** @@ -555,11 +551,11 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) /* Only get used array entries after they have been exposed by host. */ virtio_rmb(vq->weak_barriers); - last_used = (vq->last_used_idx & (vq->vring.num - 1)); - i = vq->vring.used->ring[last_used].id; - *len = vq->vring.used->ring[last_used].len; + last_used = (vq->last_used_idx & (vq->vq.vring.num - 1)); + i = vq->vq.vring.used->ring[last_used].id; + *len = vq->vq.vring.used->ring[last_used].len; - if (unlikely(i >= vq->vring.num)) { + if (unlikely(i >= vq->vq.vring.num)) { BAD_RING(vq, "id %u out of range\n", i); return NULL; } @@ -575,8 +571,8 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len) /* If we expect an interrupt for the next entry, tell host * by writing event index and flush out the write before * the read in the next get_buf call. */ - if (!(vq->vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) { - vring_used_event(&vq->vring) = vq->last_used_idx; + if (!(vq->vq.vring.avail->flags & VRING_AVAIL_F_NO_INTERRUPT)) { + vring_used_event(&vq->vq.vring) = vq->last_used_idx; virtio_mb(vq->weak_barriers); } @@ -602,7 +598,7 @@ void virtqueue_disable_cb(struct virtqueue *_vq) { struct vring_virtqueue *vq = to_vvq(_vq); - vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + vq->vq.vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; } EXPORT_SYMBOL_GPL(virtqueue_disable_cb); @@ -628,8 +624,8 @@ bool virtqueue_enable_cb(struct virtqueue *_vq) /* Depending on the VIRTIO_RING_F_EVENT_IDX feature, we need to * either clear the flags bit or point the event index at the next * entry. Always do both to keep code simple. */ - vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; - vring_used_event(&vq->vring) = vq->last_used_idx; + vq->vq.vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; + vring_used_event(&vq->vq.vring) = vq->last_used_idx; virtio_mb(vq->weak_barriers); if (unlikely(more_used(vq))) { END_USE(vq); @@ -666,12 +662,12 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *_vq) /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to * either clear the flags bit or point the event index at the next * entry. Always do both to keep code simple. */ - vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; + vq->vq.vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT; /* TODO: tune this threshold */ - bufs = (u16)(vq->vring.avail->idx - vq->last_used_idx) * 3 / 4; - vring_used_event(&vq->vring) = vq->last_used_idx + bufs; + bufs = (u16)(vq->vq.vring.avail->idx - vq->last_used_idx) * 3 / 4; + vring_used_event(&vq->vq.vring) = vq->last_used_idx + bufs; virtio_mb(vq->weak_barriers); - if (unlikely((u16)(vq->vring.used->idx - vq->last_used_idx) > bufs)) { + if (unlikely((u16)(vq->vq.vring.used->idx-vq->last_used_idx) > bufs)) { END_USE(vq); return false; } @@ -697,18 +693,18 @@ void *virtqueue_detach_unused_buf(struct virtqueue *_vq) START_USE(vq); - for (i = 0; i < vq->vring.num; i++) { + for (i = 0; i < vq->vq.vring.num; i++) { if (!vq->data[i]) continue; /* detach_buf clears data, so grab it now. */ buf = vq->data[i]; detach_buf(vq, i); - vq->vring.avail->idx--; + vq->vq.vring.avail->idx--; END_USE(vq); return buf; } /* That should have freed everything. */ - BUG_ON(vq->vq.num_free != vq->vring.num); + BUG_ON(vq->vq.num_free != vq->vq.vring.num); END_USE(vq); return NULL; @@ -758,7 +754,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, if (!vq) return NULL; - vring_init(&vq->vring, num, pages, vring_align); + vring_init(&vq->vq.vring, num, pages, vring_align); vq->vq.callback = callback; vq->vq.vdev = vdev; vq->vq.name = name; @@ -780,12 +776,12 @@ struct virtqueue *vring_new_virtqueue(unsigned int index, /* No callback? Tell other side not to bother us. */ if (!callback) - vq->vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; + vq->vq.vring.avail->flags |= VRING_AVAIL_F_NO_INTERRUPT; /* Put everything in free lists. */ vq->free_head = 0; for (i = 0; i < num-1; i++) { - vq->vring.desc[i].next = i+1; + vq->vq.vring.desc[i].next = i+1; vq->data[i] = NULL; } vq->data[i] = NULL; @@ -820,20 +816,4 @@ void vring_transport_features(struct virtio_device *vdev) } EXPORT_SYMBOL_GPL(vring_transport_features); -/** - * virtqueue_get_vring_size - return the size of the virtqueue's vring - * @vq: the struct virtqueue containing the vring of interest. - * - * Returns the size of the vring. This is mainly used for boasting to - * userspace. Unlike other operations, this need not be serialized. - */ -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) -{ - - struct vring_virtqueue *vq = to_vvq(_vq); - - return vq->vring.num; -} -EXPORT_SYMBOL_GPL(virtqueue_get_vring_size); - MODULE_LICENSE("GPL"); diff --git a/include/linux/virtio.h b/include/linux/virtio.h index a05f7c7..09883f5 100644 --- a/include/linux/virtio.h +++ b/include/linux/virtio.h @@ -9,6 +9,7 @@ #include <linux/mod_devicetable.h> #include <linux/gfp.h> #include <linux/vringh.h> +#include <uapi/linux/virtio_ring.h> /** * virtqueue - a queue to register buffers for sending or receiving. @@ -19,6 +20,7 @@ * @priv: a pointer for the virtqueue implementation to use. * @index: the zero-based ordinal number for this queue. * @num_free: number of elements we expect to be able to fit. + * @vring: the layout of the virtio ring. * * A note on @num_free: with indirect buffers, each buffer needs one * element in the queue, otherwise a buffer will need one element per @@ -31,6 +33,7 @@ struct virtqueue { struct virtio_device *vdev; unsigned int index; unsigned int num_free; + struct vring vring; void *priv; }; @@ -74,7 +77,10 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq); void *virtqueue_detach_unused_buf(struct virtqueue *vq); -unsigned int virtqueue_get_vring_size(struct virtqueue *vq); +static inline unsigned int virtqueue_get_vring_size(struct virtqueue *vq) +{ + return vq->vring.num; +} /* FIXME: Obsolete accessor, but required for virtio_net merge. */ static inline unsigned int virtqueue_get_queue_index(struct virtqueue *vq) -- 1.7.10.4
Pawel Moll
2013-Apr-05 09:52 UTC
[PATCH 4/7] virtio_config: make transports implement accessors.
On Fri, 2013-04-05 at 04:37 +0100, Rusty Russell wrote:> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index 1ba0d68..4c2c6be 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -167,26 +167,18 @@ static void vm_finalize_features(struct virtio_device *vdev) > } > } > > -static void vm_get(struct virtio_device *vdev, unsigned offset, > - void *buf, unsigned len) > +static u8 vm_get8(struct virtio_device *vdev, unsigned offset) > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > - u8 *ptr = buf; > - int i; > > - for (i = 0; i < len; i++) > - ptr[i] = readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i); > + return readb(vm_dev->base + VIRTIO_MMIO_CONFIG + offset); > } > > -static void vm_set(struct virtio_device *vdev, unsigned offset, > - const void *buf, unsigned len) > +static void vm_set8(struct virtio_device *vdev, unsigned offset, u8 v) > { > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > - const u8 *ptr = buf; > - int i; > > - for (i = 0; i < len; i++) > - writeb(ptr[i], vm_dev->base + VIRTIO_MMIO_CONFIG + offset + i); > + writeb(v, vm_dev->base + VIRTIO_MMIO_CONFIG + offset); > } > > static u8 vm_get_status(struct virtio_device *vdev) > @@ -424,8 +416,9 @@ static const char *vm_bus_name(struct virtio_device *vdev) > } > > static const struct virtio_config_ops virtio_mmio_config_ops = { > - .get = vm_get, > - .set = vm_set, > + .get8 = vm_get8, > + .set8 = vm_set8, > + VIRTIO_CONFIG_OPS_NOCONV, > .get_status = vm_get_status, > .set_status = vm_set_status, > .reset = vm_reset,Acked-by: Pawel Moll <pawel.moll at arm.com> Thanks! Pawel