From a586d4f6016f7139d8c26df0e6927131168d3b5b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Feb 2008 23:49:56 -0500 Subject: virtio: simplify config mechanism. Previously we used a type/len pair within the config space, but this seems overkill. We now simply define a structure which represents the layout in the config space: the config space can now only be extended at the end. The main driver-visible changes: 1) We indicate what fields are present with an explicit feature bit. 2) Virtqueues are explicitly numbered, and not in the config space. Signed-off-by: Rusty Russell --- drivers/block/virtio_blk.c | 35 ++++------- drivers/char/virtio_console.c | 4 +- drivers/lguest/lguest_device.c | 132 +++++++++++++++++++++++------------------ drivers/net/virtio_net.c | 25 ++++---- drivers/virtio/virtio.c | 45 -------------- 5 files changed, 100 insertions(+), 141 deletions(-) (limited to 'drivers') diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c index 924ddd8bccd..1c63d5b64c2 100644 --- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -162,8 +162,6 @@ static int virtblk_probe(struct virtio_device *vdev) { struct virtio_blk *vblk; int err, major; - void *token; - unsigned int len; u64 cap; u32 v; @@ -178,7 +176,7 @@ static int virtblk_probe(struct virtio_device *vdev) vblk->vdev = vdev; /* We expect one virtqueue, for output. */ - vblk->vq = vdev->config->find_vq(vdev, blk_done); + vblk->vq = vdev->config->find_vq(vdev, 0, blk_done); if (IS_ERR(vblk->vq)) { err = PTR_ERR(vblk->vq); goto out_free_vblk; @@ -216,15 +214,12 @@ static int virtblk_probe(struct virtio_device *vdev) vblk->disk->fops = &virtblk_fops; /* If barriers are supported, tell block layer that queue is ordered */ - token = vdev->config->find(vdev, VIRTIO_CONFIG_BLK_F, &len); - if (virtio_use_bit(vdev, token, len, VIRTIO_BLK_F_BARRIER)) + if (vdev->config->feature(vdev, VIRTIO_BLK_F_BARRIER)) blk_queue_ordered(vblk->disk->queue, QUEUE_ORDERED_TAG, NULL); - err = virtio_config_val(vdev, VIRTIO_CONFIG_BLK_F_CAPACITY, &cap); - if (err) { - dev_err(&vdev->dev, "Bad/missing capacity in config\n"); - goto out_cleanup_queue; - } + /* Host must always specify the capacity. */ + __virtio_config_val(vdev, offsetof(struct virtio_blk_config, capacity), + &cap); /* If capacity is too big, truncate with warning. */ if ((sector_t)cap != cap) { @@ -234,27 +229,23 @@ static int virtblk_probe(struct virtio_device *vdev) } set_capacity(vblk->disk, cap); - err = virtio_config_val(vdev, VIRTIO_CONFIG_BLK_F_SIZE_MAX, &v); + /* 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); if (!err) blk_queue_max_segment_size(vblk->disk->queue, v); - else if (err != -ENOENT) { - dev_err(&vdev->dev, "Bad SIZE_MAX in config\n"); - goto out_cleanup_queue; - } - err = virtio_config_val(vdev, VIRTIO_CONFIG_BLK_F_SEG_MAX, &v); + err = virtio_config_val(vdev, VIRTIO_BLK_F_SEG_MAX, + offsetof(struct virtio_blk_config, seg_max), + &v); if (!err) blk_queue_max_hw_segments(vblk->disk->queue, v); - else if (err != -ENOENT) { - dev_err(&vdev->dev, "Bad SEG_MAX in config\n"); - goto out_cleanup_queue; - } add_disk(vblk->disk); return 0; -out_cleanup_queue: - blk_cleanup_queue(vblk->disk->queue); out_put_disk: put_disk(vblk->disk); out_unregister_blkdev: diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c index e34da5c9719..dc17fe3a88b 100644 --- a/drivers/char/virtio_console.c +++ b/drivers/char/virtio_console.c @@ -158,13 +158,13 @@ static int __devinit virtcons_probe(struct virtio_device *dev) /* Find the input queue. */ /* FIXME: This is why we want to wean off hvc: we do nothing * when input comes in. */ - in_vq = vdev->config->find_vq(vdev, NULL); + in_vq = vdev->config->find_vq(vdev, 0, NULL); if (IS_ERR(in_vq)) { err = PTR_ERR(in_vq); goto free; } - out_vq = vdev->config->find_vq(vdev, NULL); + out_vq = vdev->config->find_vq(vdev, 1, NULL); if (IS_ERR(out_vq)) { err = PTR_ERR(out_vq); goto free_in_vq; diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c index e2eec38c83c..07f57a53658 100644 --- a/drivers/lguest/lguest_device.c +++ b/drivers/lguest/lguest_device.c @@ -52,57 +52,82 @@ struct lguest_device { /*D:130 * Device configurations * - * The configuration information for a device consists of a series of fields. - * We don't really care what they are: the Launcher set them up, and the driver - * will look at them during setup. + * The configuration information for a device consists of one or more + * virtqueues, a feature bitmaks, and some configuration bytes. The + * configuration bytes don't really matter to us: the Launcher set them up, and + * the driver will look at them during setup. * - * For us these fields come immediately after that device's descriptor in the - * lguest_devices page. - * - * Each field starts with a "type" byte, a "length" byte, then that number of - * bytes of configuration information. The device descriptor tells us the - * total configuration length so we know when we've reached the last field. */ + * A convenient routine to return the device's virtqueue config array: + * immediately after the descriptor. */ +static struct lguest_vqconfig *lg_vq(const struct lguest_device_desc *desc) +{ + return (void *)(desc + 1); +} -/* type + length bytes */ -#define FHDR_LEN 2 +/* The features come immediately after the virtqueues. */ +static u8 *lg_features(const struct lguest_device_desc *desc) +{ + return (void *)(lg_vq(desc) + desc->num_vq); +} -/* This finds the first field of a given type for a device's configuration. */ -static void *lg_find(struct virtio_device *vdev, u8 type, unsigned int *len) +/* The config space comes after the two feature bitmasks. */ +static u8 *lg_config(const struct lguest_device_desc *desc) { - struct lguest_device_desc *desc = to_lgdev(vdev)->desc; - int i; - - for (i = 0; i < desc->config_len; i += FHDR_LEN + desc->config[i+1]) { - if (desc->config[i] == type) { - /* Mark it used, so Host can know we looked at it, and - * also so we won't find the same one twice. */ - desc->config[i] |= 0x80; - /* Remember, the second byte is the length. */ - *len = desc->config[i+1]; - /* We return a pointer to the field header. */ - return desc->config + i; - } - } + return lg_features(desc) + desc->feature_len * 2; +} - /* Not found: return NULL for failure. */ - return NULL; +/* The total size of the config page used by this device (incl. desc) */ +static unsigned desc_size(const struct lguest_device_desc *desc) +{ + return sizeof(*desc) + + desc->num_vq * sizeof(struct lguest_vqconfig) + + desc->feature_len * 2 + + desc->config_len; +} + +/* This tests (and acknowleges) a feature bit. */ +static bool lg_feature(struct virtio_device *vdev, unsigned fbit) +{ + struct lguest_device_desc *desc = to_lgdev(vdev)->desc; + u8 *features; + + /* Obviously if they ask for a feature off the end of our feature + * bitmap, it's not set. */ + if (fbit / 8 > desc->feature_len) + return false; + + /* The feature bitmap comes after the virtqueues. */ + features = lg_features(desc); + if (!(features[fbit / 8] & (1 << (fbit % 8)))) + return false; + + /* We set the matching bit in the other half of the bitmap to tell the + * Host we want to use this feature. We don't use this yet, but we + * could in future. */ + features[desc->feature_len + fbit / 8] |= (1 << (fbit % 8)); + return true; } /* Once they've found a field, getting a copy of it is easy. */ -static void lg_get(struct virtio_device *vdev, void *token, +static void lg_get(struct virtio_device *vdev, unsigned int offset, void *buf, unsigned len) { - /* Check they didn't ask for more than the length of the field! */ - BUG_ON(len > ((u8 *)token)[1]); - memcpy(buf, token + FHDR_LEN, len); + 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); } /* Setting the contents is also trivial. */ -static void lg_set(struct virtio_device *vdev, void *token, +static void lg_set(struct virtio_device *vdev, unsigned int offset, const void *buf, unsigned len) { - BUG_ON(len > ((u8 *)token)[1]); - memcpy(token + FHDR_LEN, buf, len); + 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); } /* The operations to get and set the status word just access the status field @@ -165,39 +190,29 @@ static void lg_notify(struct virtqueue *vq) * * So we provide devices with a "find virtqueue and set it up" function. */ static struct virtqueue *lg_find_vq(struct virtio_device *vdev, + unsigned index, bool (*callback)(struct virtqueue *vq)) { + struct lguest_device *ldev = to_lgdev(vdev); struct lguest_vq_info *lvq; struct virtqueue *vq; - unsigned int len; - void *token; int err; - /* Look for a field of the correct type to mark a virtqueue. Note that - * if this succeeds, then the type will be changed so it won't be found - * again, and future lg_find_vq() calls will find the next - * virtqueue (if any). */ - token = vdev->config->find(vdev, VIRTIO_CONFIG_F_VIRTQUEUE, &len); - if (!token) + /* We must have this many virtqueues. */ + if (index >= ldev->desc->num_vq) return ERR_PTR(-ENOENT); lvq = kmalloc(sizeof(*lvq), GFP_KERNEL); if (!lvq) return ERR_PTR(-ENOMEM); - /* Note: we could use a configuration space inside here, just like we - * do for the device. This would allow expansion in future, because - * our configuration system is designed to be expansible. But this is - * way easier. */ - if (len != sizeof(lvq->config)) { - dev_err(&vdev->dev, "Unexpected virtio config len %u\n", len); - err = -EIO; - goto free_lvq; - } - /* Make a copy of the "struct lguest_vqconfig" field. We need a copy - * because the config space might not be aligned correctly. */ - vdev->config->get(vdev, token, &lvq->config, sizeof(lvq->config)); + /* Make a copy of the "struct lguest_vqconfig" entry, which sits after + * the descriptor. We need a copy because the config space might not + * be aligned correctly. */ + memcpy(&lvq->config, lg_vq(ldev->desc)+index, sizeof(lvq->config)); + printk("Mapping virtqueue %i addr %lx\n", index, + (unsigned long)lvq->config.pfn << PAGE_SHIFT); /* Figure out how many pages the ring will take, and map that memory */ lvq->pages = lguest_map((unsigned long)lvq->config.pfn << PAGE_SHIFT, DIV_ROUND_UP(vring_size(lvq->config.num, @@ -259,7 +274,7 @@ static void lg_del_vq(struct virtqueue *vq) /* The ops structure which hooks everything together. */ static struct virtio_config_ops lguest_config_ops = { - .find = lg_find, + .feature = lg_feature, .get = lg_get, .set = lg_set, .get_status = lg_get_status, @@ -329,13 +344,14 @@ static void scan_devices(void) struct lguest_device_desc *d; /* We start at the page beginning, and skip over each entry. */ - for (i = 0; i < PAGE_SIZE; i += sizeof(*d) + d->config_len) { + for (i = 0; i < PAGE_SIZE; i += desc_size(d)) { d = lguest_devices + i; /* Once we hit a zero, stop. */ if (d->type == 0) break; + printk("Device at %i has size %u\n", i, desc_size(d)); add_lguest_device(d); } } diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index a60505c8f82..4b813831275 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -311,10 +311,8 @@ static int virtnet_close(struct net_device *dev) static int virtnet_probe(struct virtio_device *vdev) { int err; - unsigned int len; struct net_device *dev; struct virtnet_info *vi; - void *token; /* Allocate ourselves a network device with room for our info */ dev = alloc_etherdev(sizeof(struct virtnet_info)); @@ -330,25 +328,24 @@ static int virtnet_probe(struct virtio_device *vdev) SET_NETDEV_DEV(dev, &vdev->dev); /* Do we support "hardware" checksums? */ - token = vdev->config->find(vdev, VIRTIO_CONFIG_NET_F, &len); - if (virtio_use_bit(vdev, token, len, VIRTIO_NET_F_NO_CSUM)) { + if (vdev->config->feature(vdev, VIRTIO_NET_F_NO_CSUM)) { /* This opens up the world of extra features. */ dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST; - if (virtio_use_bit(vdev, token, len, VIRTIO_NET_F_TSO4)) + if (vdev->config->feature(vdev, VIRTIO_NET_F_TSO4)) dev->features |= NETIF_F_TSO; - if (virtio_use_bit(vdev, token, len, VIRTIO_NET_F_UFO)) + if (vdev->config->feature(vdev, VIRTIO_NET_F_UFO)) dev->features |= NETIF_F_UFO; - if (virtio_use_bit(vdev, token, len, VIRTIO_NET_F_TSO4_ECN)) + if (vdev->config->feature(vdev, VIRTIO_NET_F_TSO4_ECN)) dev->features |= NETIF_F_TSO_ECN; - if (virtio_use_bit(vdev, token, len, VIRTIO_NET_F_TSO6)) + if (vdev->config->feature(vdev, VIRTIO_NET_F_TSO6)) dev->features |= NETIF_F_TSO6; } /* Configuration may specify what MAC to use. Otherwise random. */ - token = vdev->config->find(vdev, VIRTIO_CONFIG_NET_MAC_F, &len); - if (token) { - dev->addr_len = len; - vdev->config->get(vdev, token, dev->dev_addr, len); + if (vdev->config->feature(vdev, VIRTIO_NET_F_MAC)) { + vdev->config->get(vdev, + offsetof(struct virtio_net_config, mac), + dev->dev_addr, dev->addr_len); } else random_ether_addr(dev->dev_addr); @@ -359,13 +356,13 @@ static int virtnet_probe(struct virtio_device *vdev) vi->vdev = vdev; /* We expect two virtqueues, receive then send. */ - vi->rvq = vdev->config->find_vq(vdev, skb_recv_done); + vi->rvq = vdev->config->find_vq(vdev, 0, skb_recv_done); if (IS_ERR(vi->rvq)) { err = PTR_ERR(vi->rvq); goto free; } - vi->svq = vdev->config->find_vq(vdev, skb_xmit_done); + vi->svq = vdev->config->find_vq(vdev, 1, skb_xmit_done); if (IS_ERR(vi->svq)) { err = PTR_ERR(vi->svq); goto free_recv; diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index 69d7ea02cd4..303cb6f9010 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -148,51 +148,6 @@ void unregister_virtio_device(struct virtio_device *dev) } EXPORT_SYMBOL_GPL(unregister_virtio_device); -int __virtio_config_val(struct virtio_device *vdev, - u8 type, void *val, size_t size) -{ - void *token; - unsigned int len; - - token = vdev->config->find(vdev, type, &len); - if (!token) - return -ENOENT; - - if (len != size) - return -EIO; - - vdev->config->get(vdev, token, val, size); - return 0; -} -EXPORT_SYMBOL_GPL(__virtio_config_val); - -int virtio_use_bit(struct virtio_device *vdev, - void *token, unsigned int len, unsigned int bitnum) -{ - unsigned long bits[16]; - - /* This makes it convenient to pass-through find() results. */ - if (!token) - return 0; - - /* bit not in range of this bitfield? */ - if (bitnum * 8 >= len / 2) - return 0; - - /* Giant feature bitfields are silly. */ - BUG_ON(len > sizeof(bits)); - vdev->config->get(vdev, token, bits, len); - - if (!test_bit(bitnum, bits)) - return 0; - - /* Set acknowledge bit, and write it back. */ - set_bit(bitnum + len * 8 / 2, bits); - vdev->config->set(vdev, token, bits, len); - return 1; -} -EXPORT_SYMBOL_GPL(virtio_use_bit); - static int virtio_init(void) { if (bus_register(&virtio_bus) != 0) -- cgit v1.2.3