From c45a6816c19dee67b8f725e6646d428901a6dc24 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 2 May 2008 21:50:50 -0500 Subject: virtio: explicit advertisement of driver features A recent proposed feature addition to the virtio block driver revealed some flaws in the API: in particular, we assume that feature negotiation is complete once a driver's probe function returns. There is nothing in the API to require this, however, and even I didn't notice when it was violated. So instead, we require the driver to specify what features it supports in a table, we can then move the feature negotiation into the virtio core. The intersection of device and driver features are presented in a new 'features' bitmap in the struct virtio_device. Note that this highlights the difference between Linux unsigned-long bitmaps where each unsigned long is in native endian, and a straight-forward little-endian array of bytes. Drivers can still remove feature bits in their probe routine if they really have to. API changes: - dev->config->feature() no longer gets and acks a feature. - drivers should advertise their features in the 'feature_table' field - use virtio_has_feature() for extra sanity when checking feature bits Signed-off-by: Rusty Russell --- drivers/net/virtio_net.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'drivers/net') diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ad43421ab6f1..f926b5ab3d09 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -378,26 +378,26 @@ static int virtnet_probe(struct virtio_device *vdev) SET_NETDEV_DEV(dev, &vdev->dev); /* Do we support "hardware" checksums? */ - if (csum && vdev->config->feature(vdev, VIRTIO_NET_F_CSUM)) { + if (csum && virtio_has_feature(vdev, VIRTIO_NET_F_CSUM)) { /* This opens up the world of extra features. */ dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST; - if (gso && vdev->config->feature(vdev, VIRTIO_NET_F_GSO)) { + if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) { dev->features |= NETIF_F_TSO | NETIF_F_UFO | NETIF_F_TSO_ECN | NETIF_F_TSO6; } /* Individual feature bits: what can host handle? */ - if (gso && vdev->config->feature(vdev, VIRTIO_NET_F_HOST_TSO4)) + if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO4)) dev->features |= NETIF_F_TSO; - if (gso && vdev->config->feature(vdev, VIRTIO_NET_F_HOST_TSO6)) + if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_TSO6)) dev->features |= NETIF_F_TSO6; - if (gso && vdev->config->feature(vdev, VIRTIO_NET_F_HOST_ECN)) + if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN)) dev->features |= NETIF_F_TSO_ECN; - if (gso && vdev->config->feature(vdev, VIRTIO_NET_F_HOST_UFO)) + if (gso && virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO)) dev->features |= NETIF_F_UFO; } /* Configuration may specify what MAC to use. Otherwise random. */ - if (vdev->config->feature(vdev, VIRTIO_NET_F_MAC)) { + if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) { vdev->config->get(vdev, offsetof(struct virtio_net_config, mac), dev->dev_addr, dev->addr_len); @@ -486,7 +486,15 @@ static struct virtio_device_id id_table[] = { { 0 }, }; +static unsigned int features[] = { + VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC, + VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6, + VIRTIO_NET_F_HOST_ECN, +}; + static struct virtio_driver virtio_net = { + .feature_table = features, + .feature_table_size = ARRAY_SIZE(features), .driver.name = KBUILD_MODNAME, .driver.owner = THIS_MODULE, .id_table = id_table, -- cgit v1.2.3