Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 00/20] qemu: towards virtio-1 host support
And yet another iteration of virtio-1 support in qemu, tested with the latest virtio kernel patches. Find it at git://github.com/cohuck/qemu virtio-1 Changes from v5: - fixed stupid bug in "virtio: support more feature bits": we need to define a proper prop backend for 64 bit wide handling... - don't negotiate revision 1 unless VERSION_1 is offered - use 64 bit wide features in the whole of vhost (patch can probably be merged into the 64 bit extension for the rest of the code) The bug I suspected in the revision-specific feature handling turned out to be a messup on my side. There still seems to be something missing for vhost-net to support virtio-1. Cornelia Huck (17): virtio: cull virtio_bus_set_vdev_features virtio: feature bit manipulation helpers virtio: add feature checking helpers virtio: support more feature bits virtio: endianness checks for virtio 1.0 devices virtio: allow virtio-1 queue layout dataplane: allow virtio-1 devices s390x/virtio-ccw: support virtio-1 set_vq format virtio: disallow late feature changes for virtio-1 virtio: allow to fail setting status s390x/virtio-ccw: enable virtio 1.0 virtio-net: no writeable mac for virtio-1 virtio-net: support longer header virtio-net: enable virtio 1.0 virtio: support revision-specific features virtio-blk: revision specific feature bits vhost: 64 bit features Thomas Huth (3): linux-headers/virtio_config: Update with VIRTIO_F_VERSION_1 s390x/css: Add a callback for when subchannel gets disabled s390x/virtio-ccw: add virtio set-revision call hw/9pfs/virtio-9p-device.c | 4 +- hw/block/dataplane/virtio-blk.c | 4 +- hw/block/virtio-blk.c | 44 +++-- hw/char/virtio-serial-bus.c | 6 +- hw/core/qdev-properties.c | 58 ++++++ hw/net/vhost_net.c | 12 +- hw/net/virtio-net.c | 100 ++++++----- hw/s390x/css.c | 12 ++ hw/s390x/css.h | 1 + hw/s390x/s390-virtio-bus.c | 3 +- hw/s390x/s390-virtio-bus.h | 2 +- hw/s390x/virtio-ccw.c | 234 ++++++++++++++++++------- hw/s390x/virtio-ccw.h | 13 +- hw/scsi/vhost-scsi.c | 3 +- hw/scsi/virtio-scsi-dataplane.c | 2 +- hw/scsi/virtio-scsi.c | 12 +- hw/virtio/Makefile.objs | 2 +- hw/virtio/dataplane/Makefile.objs | 2 +- hw/virtio/dataplane/vring.c | 96 +++++----- hw/virtio/vhost.c | 14 +- hw/virtio/virtio-balloon.c | 4 +- hw/virtio/virtio-bus.c | 24 ++- hw/virtio/virtio-mmio.c | 9 +- hw/virtio/virtio-pci.c | 7 +- hw/virtio/virtio-pci.h | 2 +- hw/virtio/virtio-rng.c | 2 +- hw/virtio/virtio.c | 120 +++++++++---- include/hw/qdev-properties.h | 12 ++ include/hw/virtio/dataplane/vring-accessors.h | 75 ++++++++ include/hw/virtio/dataplane/vring.h | 14 +- include/hw/virtio/vhost.h | 6 +- include/hw/virtio/virtio-access.h | 4 + include/hw/virtio/virtio-bus.h | 14 +- include/hw/virtio/virtio-net.h | 46 ++--- include/hw/virtio/virtio-scsi.h | 6 +- include/hw/virtio/virtio.h | 62 +++++-- include/net/vhost_net.h | 4 +- linux-headers/linux/virtio_config.h | 3 + 38 files changed, 727 insertions(+), 311 deletions(-) create mode 100644 include/hw/virtio/dataplane/vring-accessors.h -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 01/20] linux-headers/virtio_config: Update with VIRTIO_F_VERSION_1
From: Thomas Huth <thuth at linux.vnet.ibm.com> Add the new VIRTIO_F_VERSION_1 definition to the virtio_config.h linux header. Signed-off-by: Thomas Huth <thuth at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- linux-headers/linux/virtio_config.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/linux-headers/linux/virtio_config.h b/linux-headers/linux/virtio_config.h index 75dc20b..16aa289 100644 --- a/linux-headers/linux/virtio_config.h +++ b/linux-headers/linux/virtio_config.h @@ -54,4 +54,7 @@ /* Can the device handle any descriptor layout? */ #define VIRTIO_F_ANY_LAYOUT 27 +/* v1.0 compliant. */ +#define VIRTIO_F_VERSION_1 32 + #endif /* _LINUX_VIRTIO_CONFIG_H */ -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 02/20] virtio: cull virtio_bus_set_vdev_features
The only user of this function was virtio-ccw, and it should use virtio_set_features() like everybody else: We need to make sure that bad features are masked out properly, which this function did not do. Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/virtio-ccw.c | 3 +-- hw/virtio/virtio-bus.c | 14 -------------- include/hw/virtio/virtio-bus.h | 3 --- 3 files changed, 1 insertion(+), 19 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index ea236c9..84f17bc 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -400,8 +400,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) ccw.cda + sizeof(features.features)); features.features = ldl_le_phys(&address_space_memory, ccw.cda); if (features.index < ARRAY_SIZE(dev->host_features)) { - virtio_bus_set_vdev_features(&dev->bus, features.features); - vdev->guest_features = features.features; + virtio_set_features(vdev, features.features); } else { /* * If the guest supports more feature bits, assert that it diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index eb77019..a8ffa07 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -109,20 +109,6 @@ uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus, return k->get_features(vdev, requested_features); } -/* Set the features of the plugged device. */ -void virtio_bus_set_vdev_features(VirtioBusState *bus, - uint32_t requested_features) -{ - VirtIODevice *vdev = virtio_bus_get_device(bus); - VirtioDeviceClass *k; - - assert(vdev != NULL); - k = VIRTIO_DEVICE_GET_CLASS(vdev); - if (k->set_features != NULL) { - k->set_features(vdev, requested_features); - } -} - /* Get bad features of the plugged device. */ uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) { diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 0756545..0d2e7b4 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -84,9 +84,6 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); /* Get the features of the plugged device. */ uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus, uint32_t requested_features); -/* Set the features of the plugged device. */ -void virtio_bus_set_vdev_features(VirtioBusState *bus, - uint32_t requested_features); /* Get bad features of the plugged device. */ uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); /* Get config of the plugged device. */ -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 03/20] virtio: feature bit manipulation helpers
Add virtio_{add,clear}_feature helper functions for manipulating a feature bits variable. This has some benefits over open coding: - add check that the bit is in a sane range - make it obvious at a glance what is going on - have a central point to change when we want to extend feature bits Convert existing code manipulating features to use the new helpers. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/9pfs/virtio-9p-device.c | 2 +- hw/block/virtio-blk.c | 16 ++++++++-------- hw/char/virtio-serial-bus.c | 2 +- hw/net/virtio-net.c | 34 +++++++++++++++++----------------- hw/s390x/virtio-ccw.c | 4 ++-- hw/virtio/virtio-mmio.c | 2 +- hw/virtio/virtio-pci.c | 4 ++-- include/hw/virtio/virtio.h | 12 ++++++++++++ 8 files changed, 44 insertions(+), 32 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 2572747..30492ec 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -23,7 +23,7 @@ static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features) { - features |= 1 << VIRTIO_9P_MOUNT_TAG; + virtio_add_feature(&features, VIRTIO_9P_MOUNT_TAG); return features; } diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index b19b102..3f76e2a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -568,20 +568,20 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features) { VirtIOBlock *s = VIRTIO_BLK(vdev); - features |= (1 << VIRTIO_BLK_F_SEG_MAX); - features |= (1 << VIRTIO_BLK_F_GEOMETRY); - features |= (1 << VIRTIO_BLK_F_TOPOLOGY); - features |= (1 << VIRTIO_BLK_F_BLK_SIZE); - features |= (1 << VIRTIO_BLK_F_SCSI); + virtio_add_feature(&features, VIRTIO_BLK_F_SEG_MAX); + virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY); + virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY); + virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE); + virtio_add_feature(&features, VIRTIO_BLK_F_SCSI); if (s->conf.config_wce) { - features |= (1 << VIRTIO_BLK_F_CONFIG_WCE); + virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); } if (blk_enable_write_cache(s->blk)) { - features |= (1 << VIRTIO_BLK_F_WCE); + virtio_add_feature(&features, VIRTIO_BLK_F_WCE); } if (blk_is_read_only(s->blk)) { - features |= 1 << VIRTIO_BLK_F_RO; + virtio_add_feature(&features, VIRTIO_BLK_F_RO); } return features; diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index a7b1b68..0f637db 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -474,7 +474,7 @@ static uint32_t get_features(VirtIODevice *vdev, uint32_t features) vser = VIRTIO_SERIAL(vdev); if (vser->bus.max_nr_ports > 1) { - features |= (1 << VIRTIO_CONSOLE_F_MULTIPORT); + virtio_add_feature(&features, VIRTIO_CONSOLE_F_MULTIPORT); } return features; } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index e574bd4..f1aa100 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -446,23 +446,23 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features) VirtIONet *n = VIRTIO_NET(vdev); NetClientState *nc = qemu_get_queue(n->nic); - features |= (1 << VIRTIO_NET_F_MAC); + virtio_add_feature(&features, VIRTIO_NET_F_MAC); if (!peer_has_vnet_hdr(n)) { - features &= ~(0x1 << VIRTIO_NET_F_CSUM); - features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4); - features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO6); - features &= ~(0x1 << VIRTIO_NET_F_HOST_ECN); + virtio_clear_feature(&features, VIRTIO_NET_F_CSUM); + virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO4); + virtio_clear_feature(&features, VIRTIO_NET_F_HOST_TSO6); + virtio_clear_feature(&features, VIRTIO_NET_F_HOST_ECN); - features &= ~(0x1 << VIRTIO_NET_F_GUEST_CSUM); - features &= ~(0x1 << VIRTIO_NET_F_GUEST_TSO4); - features &= ~(0x1 << VIRTIO_NET_F_GUEST_TSO6); - features &= ~(0x1 << VIRTIO_NET_F_GUEST_ECN); + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_CSUM); + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO4); + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_TSO6); + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_ECN); } if (!peer_has_vnet_hdr(n) || !peer_has_ufo(n)) { - features &= ~(0x1 << VIRTIO_NET_F_GUEST_UFO); - features &= ~(0x1 << VIRTIO_NET_F_HOST_UFO); + virtio_clear_feature(&features, VIRTIO_NET_F_GUEST_UFO); + virtio_clear_feature(&features, VIRTIO_NET_F_HOST_UFO); } if (!get_vhost_net(nc->peer)) { @@ -477,11 +477,11 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev) /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but also these: */ - features |= (1 << VIRTIO_NET_F_MAC); - features |= (1 << VIRTIO_NET_F_CSUM); - features |= (1 << VIRTIO_NET_F_HOST_TSO4); - features |= (1 << VIRTIO_NET_F_HOST_TSO6); - features |= (1 << VIRTIO_NET_F_HOST_ECN); + virtio_add_feature(&features, VIRTIO_NET_F_MAC); + virtio_add_feature(&features, VIRTIO_NET_F_CSUM); + virtio_add_feature(&features, VIRTIO_NET_F_HOST_TSO4); + virtio_add_feature(&features, VIRTIO_NET_F_HOST_TSO6); + virtio_add_feature(&features, VIRTIO_NET_F_HOST_ECN); return features; } @@ -1560,7 +1560,7 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) { int i, config_size = 0; - host_features |= (1 << VIRTIO_NET_F_MAC); + virtio_add_feature(&host_features, VIRTIO_NET_F_MAC); for (i = 0; feature_sizes[i].flags != 0; i++) { if (host_features & feature_sizes[i].flags) { config_size = MAX(feature_sizes[i].end, config_size); diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 84f17bc..3fee4aa 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -743,8 +743,8 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus, dev->host_features[0]); - dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; - dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE; + virtio_add_feature(&dev->host_features[0], VIRTIO_F_NOTIFY_ON_EMPTY); + virtio_add_feature(&dev->host_features[0], VIRTIO_F_BAD_FEATURE); css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, parent->hotplugged, 1); diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 2450c13..10123f3 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -349,7 +349,7 @@ static void virtio_mmio_device_plugged(DeviceState *opaque) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); - proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); + virtio_add_feature(&proxy->host_features, VIRTIO_F_NOTIFY_ON_EMPTY); proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, proxy->host_features); } diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index dde1d73..e7969bf 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -996,8 +996,8 @@ static void virtio_pci_device_plugged(DeviceState *d) proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD; } - proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; - proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; + virtio_add_feature(&proxy->host_features, VIRTIO_F_NOTIFY_ON_EMPTY); + virtio_add_feature(&proxy->host_features, VIRTIO_F_BAD_FEATURE); proxy->host_features = virtio_bus_get_vdev_features(bus, proxy->host_features); } diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 0726d76..2fede2e 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -266,6 +266,18 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, void virtio_queue_notify_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); +static inline void virtio_add_feature(uint32_t *features, unsigned int fbit) +{ + assert(fbit < 32); + *features |= (1 << fbit); +} + +static inline void virtio_clear_feature(uint32_t *features, unsigned int fbit) +{ + assert(fbit < 32); + *features &= ~(1 << fbit); +} + static inline bool virtio_is_big_endian(VirtIODevice *vdev) { assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 04/20] virtio: add feature checking helpers
Add a helper function for checking whether a bit is set in the guest features for a vdev as well as one that works on a feature bit set. Convert code that open-coded this: It cleans up the code and makes it easier to extend the guest feature bits. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/block/virtio-blk.c | 7 ++----- hw/char/virtio-serial-bus.c | 2 +- hw/net/virtio-net.c | 23 +++++++++++++---------- hw/scsi/virtio-scsi.c | 8 ++++---- hw/virtio/dataplane/vring.c | 10 +++++----- hw/virtio/virtio-balloon.c | 2 +- hw/virtio/virtio.c | 10 +++++----- include/hw/virtio/virtio.h | 11 +++++++++++ 8 files changed, 42 insertions(+), 31 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 3f76e2a..27f263a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -590,7 +590,6 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features) static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) { VirtIOBlock *s = VIRTIO_BLK(vdev); - uint32_t features; if (s->dataplane && !(status & (VIRTIO_CONFIG_S_DRIVER | VIRTIO_CONFIG_S_DRIVER_OK))) { @@ -601,8 +600,6 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) return; } - features = vdev->guest_features; - /* A guest that supports VIRTIO_BLK_F_CONFIG_WCE must be able to send * cache flushes. Thus, the "auto writethrough" behavior is never * necessary for guests that support the VIRTIO_BLK_F_CONFIG_WCE feature. @@ -618,10 +615,10 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) * * s->blk would erroneously be placed in writethrough mode. */ - if (!(features & (1 << VIRTIO_BLK_F_CONFIG_WCE))) { + if (!virtio_has_feature(vdev, VIRTIO_BLK_F_CONFIG_WCE)) { aio_context_acquire(blk_get_aio_context(s->blk)); blk_set_enable_write_cache(s->blk, - !!(features & (1 << VIRTIO_BLK_F_WCE))); + virtio_has_feature(vdev, VIRTIO_BLK_F_WCE)); aio_context_release(blk_get_aio_context(s->blk)); } } diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index 0f637db..d49883f 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -75,7 +75,7 @@ static VirtIOSerialPort *find_port_by_name(char *name) static bool use_multiport(VirtIOSerial *vser) { VirtIODevice *vdev = VIRTIO_DEVICE(vser); - return vdev->guest_features & (1 << VIRTIO_CONSOLE_F_MULTIPORT); + return virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT); } static size_t write_to_port(VirtIOSerialPort *port, diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index f1aa100..9f3c58a 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -86,7 +86,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) memcpy(&netcfg, config, n->config_size); - if (!(vdev->guest_features >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) && + if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR) && memcmp(netcfg.mac, n->mac, ETH_ALEN)) { memcpy(n->mac, netcfg.mac, ETH_ALEN); qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac); @@ -305,7 +305,7 @@ static RxFilterInfo *virtio_net_query_rxfilter(NetClientState *nc) info->multicast_table = str_list; info->vlan_table = get_vlan_table(n); - if (!((1 << VIRTIO_NET_F_CTRL_VLAN) & vdev->guest_features)) { + if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VLAN)) { info->vlan = RX_STATE_ALL; } else if (!info->vlan_table) { info->vlan = RX_STATE_NONE; @@ -519,9 +519,12 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) VirtIONet *n = VIRTIO_NET(vdev); int i; - virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ))); + virtio_net_set_multiqueue(n, + __virtio_has_feature(features, VIRTIO_NET_F_MQ)); - virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF))); + virtio_net_set_mrg_rx_bufs(n, + __virtio_has_feature(features, + VIRTIO_NET_F_MRG_RXBUF)); if (n->has_vnet_hdr) { n->curr_guest_offloads @@ -538,7 +541,7 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) vhost_net_ack_features(get_vhost_net(nc->peer), features); } - if ((1 << VIRTIO_NET_F_CTRL_VLAN) & features) { + if (__virtio_has_feature(features, VIRTIO_NET_F_CTRL_VLAN)) { memset(n->vlans, 0, MAX_VLAN >> 3); } else { memset(n->vlans, 0xff, MAX_VLAN >> 3); @@ -585,7 +588,7 @@ static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd, uint64_t offloads; size_t s; - if (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features)) { + if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { return VIRTIO_NET_ERR; } @@ -1378,7 +1381,7 @@ static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f) } } - if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features) { + if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { qemu_put_be64(f, n->curr_guest_offloads); } } @@ -1486,7 +1489,7 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, } } - if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features) { + if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS)) { n->curr_guest_offloads = qemu_get_be64(f); } else { n->curr_guest_offloads = virtio_net_supported_guest_offloads(n); @@ -1513,8 +1516,8 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, qemu_get_subqueue(n->nic, i)->link_down = link_down; } - if (vdev->guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE) && - vdev->guest_features & (0x1 << VIRTIO_NET_F_CTRL_VQ)) { + if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ANNOUNCE) && + virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ)) { n->announce_counter = SELF_ANNOUNCE_ROUNDS; timer_mod(n->announce_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL)); } diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index ef48550..56c92fb 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -144,7 +144,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req, * * TODO: always disable this workaround for virtio 1.0 devices. */ - if ((vdev->guest_features & VIRTIO_F_ANY_LAYOUT) == 0) { + if (!virtio_has_feature(vdev, VIRTIO_F_ANY_LAYOUT)) { req_size = req->elem.out_sg[0].iov_len; resp_size = req->elem.in_sg[0].iov_len; } @@ -748,7 +748,7 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); VirtIODevice *vdev = VIRTIO_DEVICE(s); - if (((vdev->guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) && + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_CHANGE) && dev->type != TYPE_ROM) { virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE, sense.asc | (sense.ascq << 8)); @@ -769,7 +769,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, blk_op_block_all(sd->conf.blk, s->blocker); } - if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { virtio_scsi_push_event(s, sd, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_RESCAN); @@ -783,7 +783,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, VirtIOSCSI *s = VIRTIO_SCSI(vdev); SCSIDevice *sd = SCSI_DEVICE(dev); - if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { virtio_scsi_push_event(s, sd, VIRTIO_SCSI_T_TRANSPORT_RESET, VIRTIO_SCSI_EVT_RESET_REMOVED); diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index 61f6d83..6e283fc 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -103,7 +103,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) /* Disable guest->host notifies */ void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { - if (!(vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX))) { + if (!virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY; } } @@ -114,7 +114,7 @@ void vring_disable_notification(VirtIODevice *vdev, Vring *vring) */ bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) { - if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { + if (virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(&vring->vr) = vring->vr.avail->idx; } else { vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY; @@ -133,12 +133,12 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) * interrupts. */ smp_mb(); - if ((vdev->guest_features & VIRTIO_F_NOTIFY_ON_EMPTY) && + if (virtio_has_feature(vdev, VIRTIO_F_NOTIFY_ON_EMPTY) && unlikely(vring->vr.avail->idx == vring->last_avail_idx)) { return true; } - if (!(vdev->guest_features & VIRTIO_RING_F_EVENT_IDX)) { + if (!virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { return !(vring->vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); } old = vring->signalled_used; @@ -388,7 +388,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* On success, increment avail index. */ vring->last_avail_idx++; - if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { + if (virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(&vring->vr) = vring->last_avail_idx; } diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 7bfbb75..21e449a 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -69,7 +69,7 @@ static inline void reset_stats(VirtIOBalloon *dev) static bool balloon_stats_supported(const VirtIOBalloon *s) { VirtIODevice *vdev = VIRTIO_DEVICE(s); - return vdev->guest_features & (1 << VIRTIO_BALLOON_F_STATS_VQ); + return virtio_has_feature(vdev, VIRTIO_BALLOON_F_STATS_VQ); } static bool balloon_stats_enabled(const VirtIOBalloon *s) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 013979a..5814433 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -217,7 +217,7 @@ static inline void vring_avail_event(VirtQueue *vq, uint16_t val) void virtio_queue_set_notification(VirtQueue *vq, int enable) { vq->notification = enable; - if (vq->vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { + if (virtio_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(vq, vring_avail_idx(vq)); } else if (enable) { vring_used_flags_unset_bit(vq, VRING_USED_F_NO_NOTIFY); @@ -468,7 +468,7 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem) max = vq->vring.num; i = head = virtqueue_get_head(vq, vq->last_avail_idx++); - if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { + if (virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(vq, vq->last_avail_idx); } @@ -839,12 +839,12 @@ static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq) /* We need to expose used array entries before checking used event. */ smp_mb(); /* Always notify when queue is empty (when feature acknowledge) */ - if (((vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) && - !vq->inuse && vring_avail_idx(vq) == vq->last_avail_idx)) { + if (virtio_has_feature(vdev, VIRTIO_F_NOTIFY_ON_EMPTY) && + !vq->inuse && vring_avail_idx(vq) == vq->last_avail_idx) { return true; } - if (!(vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX))) { + if (!virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { return !(vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT); } diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 2fede2e..f6c0379 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -278,6 +278,17 @@ static inline void virtio_clear_feature(uint32_t *features, unsigned int fbit) *features &= ~(1 << fbit); } +static inline bool __virtio_has_feature(uint32_t features, unsigned int fbit) +{ + assert(fbit < 32); + return !!(features & (1 << fbit)); +} + +static inline bool virtio_has_feature(VirtIODevice *vdev, unsigned int fbit) +{ + return __virtio_has_feature(vdev->guest_features, fbit); +} + static inline bool virtio_is_big_endian(VirtIODevice *vdev) { assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 05/20] virtio: support more feature bits
With virtio-1, we support more than 32 feature bits. Let's extend both host and guest features to 64, which should suffice for a while. vhost and migration have been ignored for now. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/9pfs/virtio-9p-device.c | 2 +- hw/block/virtio-blk.c | 2 +- hw/char/virtio-serial-bus.c | 2 +- hw/core/qdev-properties.c | 58 +++++++++++++++++++++++++++++++++++++++ hw/net/virtio-net.c | 22 +++++++-------- hw/s390x/s390-virtio-bus.c | 3 +- hw/s390x/s390-virtio-bus.h | 2 +- hw/s390x/virtio-ccw.c | 39 +++++++++++++++----------- hw/s390x/virtio-ccw.h | 5 +--- hw/scsi/vhost-scsi.c | 3 +- hw/scsi/virtio-scsi.c | 4 +-- hw/virtio/virtio-balloon.c | 2 +- hw/virtio/virtio-bus.c | 6 ++-- hw/virtio/virtio-mmio.c | 4 +-- hw/virtio/virtio-pci.c | 3 +- hw/virtio/virtio-pci.h | 2 +- hw/virtio/virtio-rng.c | 2 +- hw/virtio/virtio.c | 13 +++++---- include/hw/qdev-properties.h | 12 ++++++++ include/hw/virtio/virtio-bus.h | 8 +++--- include/hw/virtio/virtio-net.h | 46 +++++++++++++++---------------- include/hw/virtio/virtio-scsi.h | 6 ++-- include/hw/virtio/virtio.h | 38 ++++++++++++++----------- 23 files changed, 184 insertions(+), 100 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 30492ec..60f9ff9 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -21,7 +21,7 @@ #include "virtio-9p-coth.h" #include "hw/virtio/virtio-access.h" -static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features) +static uint64_t virtio_9p_get_features(VirtIODevice *vdev, uint64_t features) { virtio_add_feature(&features, VIRTIO_9P_MOUNT_TAG); return features; diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 27f263a..9cfae66 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -564,7 +564,7 @@ static void virtio_blk_set_config(VirtIODevice *vdev, const uint8_t *config) aio_context_release(blk_get_aio_context(s->blk)); } -static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features) +static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features) { VirtIOBlock *s = VIRTIO_BLK(vdev); diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index d49883f..2d2ed9c 100644 --- a/hw/char/virtio-serial-bus.c +++ b/hw/char/virtio-serial-bus.c @@ -467,7 +467,7 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq) { } -static uint32_t get_features(VirtIODevice *vdev, uint32_t features) +static uint64_t get_features(VirtIODevice *vdev, uint64_t features) { VirtIOSerial *vser; diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 2e47f70..1b24818 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -125,6 +125,64 @@ PropertyInfo qdev_prop_bit = { .set = prop_set_bit, }; +/* Bit on a 64 bit value*/ + +static uint64_t qdev_get_prop_mask64(Property *prop) +{ + assert(prop->info == &qdev_prop_bit64); + return 0x1ULL << prop->bitnr; +} + +static void bit64_prop_set(DeviceState *dev, Property *props, bool val) +{ + uint64_t *p = qdev_get_prop_ptr(dev, props); + uint64_t mask = qdev_get_prop_mask64(props); + if (val) { + *p |= mask; + } else { + *p &= ~mask; + } +} + +static void prop_get_bit64(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + uint64_t *p = qdev_get_prop_ptr(dev, prop); + bool value = (*p & qdev_get_prop_mask64(prop)) != 0; + + visit_type_bool(v, &value, name, errp); +} + +static void prop_set_bit64(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ + DeviceState *dev = DEVICE(obj); + Property *prop = opaque; + Error *local_err = NULL; + bool value; + + if (dev->realized) { + qdev_prop_set_after_realize(dev, name, errp); + return; + } + + visit_type_bool(v, &value, name, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + bit64_prop_set(dev, prop, value); +} + +PropertyInfo qdev_prop_bit64 = { + .name = "bool", + .description = "on/off", + .get = prop_get_bit64, + .set = prop_set_bit64, +}; + /* --- bool --- */ static void get_bool(Object *obj, Visitor *v, void *opaque, diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 9f3c58a..d6d1b98 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -38,16 +38,16 @@ (offsetof(container, field) + sizeof(((container *)0)->field)) typedef struct VirtIOFeature { - uint32_t flags; + uint64_t flags; size_t end; } VirtIOFeature; static VirtIOFeature feature_sizes[] = { - {.flags = 1 << VIRTIO_NET_F_MAC, + {.flags = 1ULL << VIRTIO_NET_F_MAC, .end = endof(struct virtio_net_config, mac)}, - {.flags = 1 << VIRTIO_NET_F_STATUS, + {.flags = 1ULL << VIRTIO_NET_F_STATUS, .end = endof(struct virtio_net_config, status)}, - {.flags = 1 << VIRTIO_NET_F_MQ, + {.flags = 1ULL << VIRTIO_NET_F_MQ, .end = endof(struct virtio_net_config, max_virtqueue_pairs)}, {} }; @@ -441,7 +441,7 @@ static void virtio_net_set_queues(VirtIONet *n) static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue); -static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features) +static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features) { VirtIONet *n = VIRTIO_NET(vdev); NetClientState *nc = qemu_get_queue(n->nic); @@ -471,9 +471,9 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features) return vhost_net_get_features(get_vhost_net(nc->peer), features); } -static uint32_t virtio_net_bad_features(VirtIODevice *vdev) +static uint64_t virtio_net_bad_features(VirtIODevice *vdev) { - uint32_t features = 0; + uint64_t features = 0; /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but also these: */ @@ -496,7 +496,7 @@ static void virtio_net_apply_guest_offloads(VirtIONet *n) !!(n->curr_guest_offloads & (1ULL << VIRTIO_NET_F_GUEST_UFO))); } -static uint64_t virtio_net_guest_offloads_by_features(uint32_t features) +static uint64_t virtio_net_guest_offloads_by_features(uint64_t features) { static const uint64_t guest_offloads_mask (1ULL << VIRTIO_NET_F_GUEST_CSUM) | @@ -514,7 +514,7 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) return virtio_net_guest_offloads_by_features(vdev->guest_features); } -static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) { VirtIONet *n = VIRTIO_NET(vdev); int i; @@ -1036,7 +1036,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t return -1; error_report("virtio-net unexpected empty queue: " "i %zd mergeable %d offset %zd, size %zd, " - "guest hdr len %zd, host hdr len %zd guest features 0x%x", + "guest hdr len %zd, host hdr len %zd guest features 0x%lx", i, n->mergeable_rx_bufs, offset, size, n->guest_hdr_len, n->host_hdr_len, vdev->guest_features); exit(1); @@ -1560,7 +1560,7 @@ static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx, vdev, idx, mask); } -void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features) +void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features) { int i, config_size = 0; virtio_add_feature(&host_features, VIRTIO_NET_F_MAC); diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 39dc201..3635909 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -490,9 +490,10 @@ static void virtio_s390_notify(DeviceState *d, uint16_t vector) s390_virtio_irq(0, token); } -static unsigned virtio_s390_get_features(DeviceState *d) +static uint64_t virtio_s390_get_features(DeviceState *d) { VirtIOS390Device *dev = to_virtio_s390_device(d); + return dev->host_features; } diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h index ffd0df7..e49d4ba 100644 --- a/hw/s390x/s390-virtio-bus.h +++ b/hw/s390x/s390-virtio-bus.h @@ -90,7 +90,7 @@ struct VirtIOS390Device { ram_addr_t feat_offs; uint8_t feat_len; VirtIODevice *vdev; - uint32_t host_features; + uint64_t host_features; VirtioBusState bus; }; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 3fee4aa..fbd909d 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -371,8 +371,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) } else { features.index = ldub_phys(&address_space_memory, ccw.cda + sizeof(features.features)); - if (features.index < ARRAY_SIZE(dev->host_features)) { - features.features = dev->host_features[features.index]; + if (features.index == 0) { + features.features = (uint32_t)dev->host_features; + } else if (features.index == 1) { + features.features = (uint32_t)(dev->host_features >> 32); } else { /* Return zeroes if the guest supports more feature bits. */ features.features = 0; @@ -399,8 +401,14 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) features.index = ldub_phys(&address_space_memory, ccw.cda + sizeof(features.features)); features.features = ldl_le_phys(&address_space_memory, ccw.cda); - if (features.index < ARRAY_SIZE(dev->host_features)) { - virtio_set_features(vdev, features.features); + if (features.index == 0) { + virtio_set_features(vdev, + (vdev->guest_features & 0xffffffff00000000) | + features.features); + } else if (features.index == 1) { + virtio_set_features(vdev, + (vdev->guest_features & 0x00000000ffffffff) | + ((uint64_t)features.features << 32)); } else { /* * If the guest supports more feature bits, assert that it @@ -739,12 +747,12 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) sch->id.cu_type = VIRTIO_CCW_CU_TYPE; sch->id.cu_model = vdev->device_id; - /* Only the first 32 feature bits are used. */ - dev->host_features[0] = virtio_bus_get_vdev_features(&dev->bus, - dev->host_features[0]); + /* Set default feature bits that are offered by the host. */ + virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY); + virtio_add_feature(&dev->host_features, VIRTIO_F_BAD_FEATURE); - virtio_add_feature(&dev->host_features[0], VIRTIO_F_NOTIFY_ON_EMPTY); - virtio_add_feature(&dev->host_features[0], VIRTIO_F_BAD_FEATURE); + dev->host_features = virtio_bus_get_vdev_features(&dev->bus, + dev->host_features); css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, parent->hotplugged, 1); @@ -777,7 +785,7 @@ static int virtio_ccw_net_init(VirtioCcwDevice *ccw_dev) VirtIONetCcw *dev = VIRTIO_NET_CCW(ccw_dev); DeviceState *vdev = DEVICE(&dev->vdev); - virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features[0]); + virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features); virtio_net_set_netclient_name(&dev->vdev, qdev->id, object_get_typename(OBJECT(qdev))); qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus)); @@ -1063,12 +1071,11 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector) } } -static unsigned virtio_ccw_get_features(DeviceState *d) +static uint64_t virtio_ccw_get_features(DeviceState *d) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); - /* Only the first 32 feature bits are used. */ - return dev->host_features[0]; + return dev->host_features; } static void virtio_ccw_reset(DeviceState *d) @@ -1381,7 +1388,7 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f) static Property virtio_ccw_net_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), - DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features[0]), + DEFINE_VIRTIO_NET_FEATURES(VirtioCcwDevice, host_features), DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags, VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_END_OF_LIST(), @@ -1486,7 +1493,7 @@ static const TypeInfo virtio_ccw_balloon = { static Property virtio_ccw_scsi_properties[] = { DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id), - DEFINE_VIRTIO_SCSI_FEATURES(VirtioCcwDevice, host_features[0]), + DEFINE_VIRTIO_SCSI_FEATURES(VirtioCcwDevice, host_features), DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags, VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true), DEFINE_PROP_END_OF_LIST(), @@ -1614,7 +1621,7 @@ static void virtio_ccw_busdev_unplug(HotplugHandler *hotplug_dev, } static Property virtio_ccw_properties[] = { - DEFINE_VIRTIO_COMMON_FEATURES(VirtioCcwDevice, host_features[0]), + DEFINE_VIRTIO_COMMON_FEATURES(VirtioCcwDevice, host_features), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 5a1f16e..9087f7a 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -68,9 +68,6 @@ typedef struct VirtIOCCWDeviceClass { int (*exit)(VirtioCcwDevice *dev); } VirtIOCCWDeviceClass; -/* Change here if we want to support more feature bits. */ -#define VIRTIO_CCW_FEATURE_SIZE 1 - /* Performance improves when virtqueue kick processing is decoupled from the * vcpu thread using ioeventfd for some devices. */ #define VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT 1 @@ -88,7 +85,7 @@ struct VirtioCcwDevice { DeviceState parent_obj; SubchDev *sch; char *bus_id; - uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE]; + uint64_t host_features; VirtioBusState bus; bool ioeventfd_started; bool ioeventfd_disabled; diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index dcb2bc5..b5d7959 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -150,8 +150,7 @@ static void vhost_scsi_stop(VHostSCSI *s) vhost_dev_disable_notifiers(&s->dev, vdev); } -static uint32_t vhost_scsi_get_features(VirtIODevice *vdev, - uint32_t features) +static uint64_t vhost_scsi_get_features(VirtIODevice *vdev, uint64_t features) { VHostSCSI *s = VHOST_SCSI(vdev); diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 56c92fb..fbac794 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -627,8 +627,8 @@ static void virtio_scsi_set_config(VirtIODevice *vdev, vs->cdb_size = virtio_ldl_p(vdev, &scsiconf->cdb_size); } -static uint32_t virtio_scsi_get_features(VirtIODevice *vdev, - uint32_t requested_features) +static uint64_t virtio_scsi_get_features(VirtIODevice *vdev, + uint64_t requested_features) { return requested_features; } diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index 21e449a..d2d7c3e 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -303,7 +303,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, } } -static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f) +static uint64_t virtio_balloon_get_features(VirtIODevice *vdev, uint64_t f) { f |= (1 << VIRTIO_BALLOON_F_STATS_VQ); return f; diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index a8ffa07..32e3fab 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -97,8 +97,8 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) } /* Get the features of the plugged device. */ -uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus, - uint32_t requested_features) +uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, + uint64_t requested_features) { VirtIODevice *vdev = virtio_bus_get_device(bus); VirtioDeviceClass *k; @@ -110,7 +110,7 @@ uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus, } /* Get bad features of the plugged device. */ -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) { VirtIODevice *vdev = virtio_bus_get_device(bus); VirtioDeviceClass *k; diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 10123f3..43b7e02 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -80,7 +80,7 @@ typedef struct { SysBusDevice parent_obj; MemoryRegion iomem; qemu_irq irq; - uint32_t host_features; + uint64_t host_features; /* Guest accessible state needing migration and reset */ uint32_t host_features_sel; uint32_t guest_features_sel; @@ -306,7 +306,7 @@ static void virtio_mmio_update_irq(DeviceState *opaque, uint16_t vector) qemu_set_irq(proxy->irq, level); } -static unsigned int virtio_mmio_get_features(DeviceState *opaque) +static uint64_t virtio_mmio_get_features(DeviceState *opaque) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index e7969bf..7382705 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -478,9 +478,10 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, } } -static unsigned virtio_pci_get_features(DeviceState *d) +static uint64_t virtio_pci_get_features(DeviceState *d) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); + return proxy->host_features; } diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 8873b6d..85f102d 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -91,7 +91,7 @@ struct VirtIOPCIProxy { uint32_t flags; uint32_t class_code; uint32_t nvectors; - uint32_t host_features; + uint64_t host_features; bool ioeventfd_disabled; bool ioeventfd_started; VirtIOIRQFD *vector_irqfd; diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index 473c044..edd39cc 100644 --- a/hw/virtio/virtio-rng.c +++ b/hw/virtio/virtio-rng.c @@ -99,7 +99,7 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq) virtio_rng_process(vrng); } -static uint32_t get_features(VirtIODevice *vdev, uint32_t f) +static uint64_t get_features(VirtIODevice *vdev, uint64_t f) { return f; } diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 5814433..7f74ae5 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -593,6 +593,7 @@ void virtio_reset(void *opaque) } vdev->guest_features = 0; + vdev->queue_sel = 0; vdev->status = 0; vdev->isr = 0; @@ -924,7 +925,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) qemu_put_8s(f, &vdev->status); qemu_put_8s(f, &vdev->isr); qemu_put_be16s(f, &vdev->queue_sel); - qemu_put_be32s(f, &vdev->guest_features); + /* XXX features >= 32 */ + qemu_put_be32s(f, (uint32_t *)&vdev->guest_features); qemu_put_be32(f, vdev->config_len); qemu_put_buffer(f, vdev->config, vdev->config_len); @@ -958,12 +960,12 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) vmstate_save_state(f, &vmstate_virtio, vdev); } -int virtio_set_features(VirtIODevice *vdev, uint32_t val) +int virtio_set_features(VirtIODevice *vdev, uint64_t val) { BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *vbusk = VIRTIO_BUS_GET_CLASS(qbus); VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); - uint32_t supported_features = vbusk->get_features(qbus->parent); + uint64_t supported_features = vbusk->get_features(qbus->parent); bool bad = (val & ~supported_features) != 0; val &= supported_features; @@ -980,7 +982,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) int32_t config_len; uint32_t num; uint32_t features; - uint32_t supported_features; + uint64_t supported_features; BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev); @@ -1005,9 +1007,10 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) } qemu_get_be32s(f, &features); + /* XXX features >= 32 */ if (virtio_set_features(vdev, features) < 0) { supported_features = k->get_features(qbus->parent); - error_report("Features 0x%x unsupported. Allowed features: 0x%x", + error_report("Features 0x%x unsupported. Allowed features: 0x%lx", features, supported_features); return -1; } diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 070006c..81e5d0b 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -6,6 +6,7 @@ /*** qdev-properties.c ***/ extern PropertyInfo qdev_prop_bit; +extern PropertyInfo qdev_prop_bit64; extern PropertyInfo qdev_prop_bool; extern PropertyInfo qdev_prop_uint8; extern PropertyInfo qdev_prop_uint16; @@ -51,6 +52,17 @@ extern PropertyInfo qdev_prop_arraylen; .defval = (bool)_defval, \ } +#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) { \ + .name = (_name), \ + .info = &(qdev_prop_bit64), \ + .bitnr = (_bit), \ + .offset = offsetof(_state, _field) \ + + type_check(uint64_t,typeof_field(_state, _field)), \ + .qtype = QTYPE_QBOOL, \ + .defval = (bool)_defval, \ + } + + #define DEFINE_PROP_BOOL(_name, _state, _field, _defval) { \ .name = (_name), \ .info = &(qdev_prop_bool), \ diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 0d2e7b4..0a4dde1 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -47,7 +47,7 @@ typedef struct VirtioBusClass { int (*load_config)(DeviceState *d, QEMUFile *f); int (*load_queue)(DeviceState *d, int n, QEMUFile *f); int (*load_done)(DeviceState *d, QEMUFile *f); - unsigned (*get_features)(DeviceState *d); + uint64_t (*get_features)(DeviceState *d); bool (*query_guest_notifiers)(DeviceState *d); int (*set_guest_notifiers)(DeviceState *d, int nvqs, bool assign); int (*set_host_notifier)(DeviceState *d, int n, bool assigned); @@ -82,10 +82,10 @@ uint16_t virtio_bus_get_vdev_id(VirtioBusState *bus); /* Get the config_len field of the plugged device. */ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); /* Get the features of the plugged device. */ -uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus, - uint32_t requested_features); +uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, + uint64_t requested_features); /* Get bad features of the plugged device. */ -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); +uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); /* Get config of the plugged device. */ void virtio_bus_get_vdev_config(VirtioBusState *bus, uint8_t *config); /* Set config of the plugged device. */ diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h index 6ceb5aa..5c58b4b 100644 --- a/include/hw/virtio/virtio-net.h +++ b/include/hw/virtio/virtio-net.h @@ -258,35 +258,35 @@ struct virtio_net_ctrl_mq { #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0 #define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \ - DEFINE_PROP_BIT("any_layout", _state, _field, VIRTIO_F_ANY_LAYOUT, true), \ - DEFINE_PROP_BIT("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \ - DEFINE_PROP_BIT("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \ - DEFINE_PROP_BIT("gso", _state, _field, VIRTIO_NET_F_GSO, true), \ - DEFINE_PROP_BIT("guest_tso4", _state, _field, VIRTIO_NET_F_GUEST_TSO4, true), \ - DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \ - DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \ - DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \ - DEFINE_PROP_BIT("guest_announce", _state, _field, VIRTIO_NET_F_GUEST_ANNOUNCE, true), \ - DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \ - DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \ - DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \ - DEFINE_PROP_BIT("host_ufo", _state, _field, VIRTIO_NET_F_HOST_UFO, true), \ - DEFINE_PROP_BIT("mrg_rxbuf", _state, _field, VIRTIO_NET_F_MRG_RXBUF, true), \ - DEFINE_PROP_BIT("status", _state, _field, VIRTIO_NET_F_STATUS, true), \ - DEFINE_PROP_BIT("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \ - DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \ - DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \ - DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \ - DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \ - DEFINE_PROP_BIT("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \ - DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, false) + DEFINE_PROP_BIT64("any_layout", _state, _field, VIRTIO_F_ANY_LAYOUT, true), \ + DEFINE_PROP_BIT64("csum", _state, _field, VIRTIO_NET_F_CSUM, true), \ + DEFINE_PROP_BIT64("guest_csum", _state, _field, VIRTIO_NET_F_GUEST_CSUM, true), \ + DEFINE_PROP_BIT64("gso", _state, _field, VIRTIO_NET_F_GSO, true), \ + DEFINE_PROP_BIT64("guest_tso4", _state, _field, VIRTIO_NET_F_GUEST_TSO4, true), \ + DEFINE_PROP_BIT64("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \ + DEFINE_PROP_BIT64("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \ + DEFINE_PROP_BIT64("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \ + DEFINE_PROP_BIT64("guest_announce", _state, _field, VIRTIO_NET_F_GUEST_ANNOUNCE, true), \ + DEFINE_PROP_BIT64("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \ + DEFINE_PROP_BIT64("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \ + DEFINE_PROP_BIT64("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \ + DEFINE_PROP_BIT64("host_ufo", _state, _field, VIRTIO_NET_F_HOST_UFO, true), \ + DEFINE_PROP_BIT64("mrg_rxbuf", _state, _field, VIRTIO_NET_F_MRG_RXBUF, true), \ + DEFINE_PROP_BIT64("status", _state, _field, VIRTIO_NET_F_STATUS, true), \ + DEFINE_PROP_BIT64("ctrl_vq", _state, _field, VIRTIO_NET_F_CTRL_VQ, true), \ + DEFINE_PROP_BIT64("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, true), \ + DEFINE_PROP_BIT64("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, true), \ + DEFINE_PROP_BIT64("ctrl_rx_extra", _state, _field, VIRTIO_NET_F_CTRL_RX_EXTRA, true), \ + DEFINE_PROP_BIT64("ctrl_mac_addr", _state, _field, VIRTIO_NET_F_CTRL_MAC_ADDR, true), \ + DEFINE_PROP_BIT64("ctrl_guest_offloads", _state, _field, VIRTIO_NET_F_CTRL_GUEST_OFFLOADS, true), \ + DEFINE_PROP_BIT64("mq", _state, _field, VIRTIO_NET_F_MQ, false) #define DEFINE_VIRTIO_NET_PROPERTIES(_state, _field) \ DEFINE_PROP_UINT32("x-txtimer", _state, _field.txtimer, TX_TIMER_INTERVAL),\ DEFINE_PROP_INT32("x-txburst", _state, _field.txburst, TX_BURST), \ DEFINE_PROP_STRING("tx", _state, _field.tx) -void virtio_net_set_config_size(VirtIONet *n, uint32_t host_features); +void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features); void virtio_net_set_netclient_name(VirtIONet *n, const char *name, const char *type); diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index bf17cc9..f0c0a6e 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -253,11 +253,11 @@ QEMU_BUILD_BUG_ON(offsetof(VirtIOSCSIReq, req.cdb) ! DEFINE_PROP_UINT32("cmd_per_lun", _state, _conf_field.cmd_per_lun, 128) #define DEFINE_VIRTIO_SCSI_FEATURES(_state, _feature_field) \ - DEFINE_PROP_BIT("any_layout", _state, _feature_field, \ + DEFINE_PROP_BIT64("any_layout", _state, _feature_field, \ VIRTIO_F_ANY_LAYOUT, true), \ - DEFINE_PROP_BIT("hotplug", _state, _feature_field, VIRTIO_SCSI_F_HOTPLUG, \ + DEFINE_PROP_BIT64("hotplug", _state, _feature_field, VIRTIO_SCSI_F_HOTPLUG, \ true), \ - DEFINE_PROP_BIT("param_change", _state, _feature_field, \ + DEFINE_PROP_BIT64("param_change", _state, _feature_field, \ VIRTIO_SCSI_F_CHANGE, true) typedef void (*HandleOutput)(VirtIODevice *, VirtQueue *); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index f6c0379..08141c7 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -55,6 +55,12 @@ /* A guest should never accept this. It implies negotiation is broken. */ #define VIRTIO_F_BAD_FEATURE 30 +/* v1.0 compliant. */ +#define VIRTIO_F_VERSION_1 32 + +/* The highest feature bit that we support */ +#define VIRTIO_HIGHEST_FEATURE_BIT 32 + /* from Linux's linux/virtio_ring.h */ /* This marks a buffer as continuing via the next field. */ @@ -117,7 +123,7 @@ struct VirtIODevice uint8_t status; uint8_t isr; uint16_t queue_sel; - uint32_t guest_features; + uint64_t guest_features; size_t config_len; void *config; uint16_t config_vector; @@ -138,9 +144,9 @@ typedef struct VirtioDeviceClass { /* This is what a VirtioDevice must implement */ DeviceRealize realize; DeviceUnrealize unrealize; - uint32_t (*get_features)(VirtIODevice *vdev, uint32_t requested_features); - uint32_t (*bad_features)(VirtIODevice *vdev); - void (*set_features)(VirtIODevice *vdev, uint32_t val); + uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features); + uint64_t (*bad_features)(VirtIODevice *vdev); + void (*set_features)(VirtIODevice *vdev, uint64_t val); void (*get_config)(VirtIODevice *vdev, uint8_t *config); void (*set_config)(VirtIODevice *vdev, const uint8_t *config); void (*reset)(VirtIODevice *vdev); @@ -225,7 +231,7 @@ void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector); void virtio_set_status(VirtIODevice *vdev, uint8_t val); void virtio_reset(void *opaque); void virtio_update_irq(VirtIODevice *vdev); -int virtio_set_features(VirtIODevice *vdev, uint32_t val); +int virtio_set_features(VirtIODevice *vdev, uint64_t val); /* Base devices. */ typedef struct VirtIOBlkConf VirtIOBlkConf; @@ -238,9 +244,9 @@ typedef struct VirtIOSCSIConf VirtIOSCSIConf; typedef struct VirtIORNGConf VirtIORNGConf; #define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \ - DEFINE_PROP_BIT("indirect_desc", _state, _field, \ + DEFINE_PROP_BIT64("indirect_desc", _state, _field, \ VIRTIO_RING_F_INDIRECT_DESC, true), \ - DEFINE_PROP_BIT("event_idx", _state, _field, \ + DEFINE_PROP_BIT64("event_idx", _state, _field, \ VIRTIO_RING_F_EVENT_IDX, true) hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n); @@ -266,22 +272,22 @@ void virtio_queue_set_host_notifier_fd_handler(VirtQueue *vq, bool assign, void virtio_queue_notify_vq(VirtQueue *vq); void virtio_irq(VirtQueue *vq); -static inline void virtio_add_feature(uint32_t *features, unsigned int fbit) +static inline void virtio_add_feature(uint64_t *features, unsigned int fbit) { - assert(fbit < 32); - *features |= (1 << fbit); + assert(fbit < 64); + *features |= (1ULL << fbit); } -static inline void virtio_clear_feature(uint32_t *features, unsigned int fbit) +static inline void virtio_clear_feature(uint64_t *features, unsigned int fbit) { - assert(fbit < 32); - *features &= ~(1 << fbit); + assert(fbit < 64); + *features &= ~(1ULL << fbit); } -static inline bool __virtio_has_feature(uint32_t features, unsigned int fbit) +static inline bool __virtio_has_feature(uint64_t features, unsigned int fbit) { - assert(fbit < 32); - return !!(features & (1 << fbit)); + assert(fbit < 64); + return !!(features & (1ULL << fbit)); } static inline bool virtio_has_feature(VirtIODevice *vdev, unsigned int fbit) -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 06/20] virtio: endianness checks for virtio 1.0 devices
Add code that checks for the VERSION_1 feature bit in order to make decisions about the device's endianness. This allows us to support transitional devices. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/virtio/virtio.c | 6 +++++- include/hw/virtio/virtio-access.h | 4 ++++ include/hw/virtio/virtio.h | 8 ++++++-- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 7f74ae5..8f69ffa 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -881,7 +881,11 @@ static bool virtio_device_endian_needed(void *opaque) VirtIODevice *vdev = opaque; assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); - return vdev->device_endian != virtio_default_endian(); + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + return vdev->device_endian != virtio_default_endian(); + } + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ + return vdev->device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE; } static const VMStateDescription vmstate_virtio_device_endian = { diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h index 46456fd..ee28c21 100644 --- a/include/hw/virtio/virtio-access.h +++ b/include/hw/virtio/virtio-access.h @@ -19,6 +19,10 @@ static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) { + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ + return false; + } #if defined(TARGET_IS_BIENDIAN) return virtio_is_big_endian(vdev); #elif defined(TARGET_WORDS_BIGENDIAN) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 08141c7..68c40db 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -297,7 +297,11 @@ static inline bool virtio_has_feature(VirtIODevice *vdev, unsigned int fbit) static inline bool virtio_is_big_endian(VirtIODevice *vdev) { - assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); - return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG; + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); + return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG; + } + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ + return false; } #endif -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 07/20] virtio: allow virtio-1 queue layout
For virtio-1 devices, we allow a more complex queue layout that doesn't require descriptor table and rings on a physically-contigous memory area: add virtio_queue_set_rings() to allow transports to set this up. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/virtio/virtio-mmio.c | 3 +++ hw/virtio/virtio.c | 53 ++++++++++++++++++++++++++++---------------- include/hw/virtio/virtio.h | 3 +++ 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 43b7e02..0c9b63b 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -244,8 +244,11 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value, case VIRTIO_MMIO_QUEUENUM: DPRINTF("mmio_queue write %d max %d\n", (int)value, VIRTQUEUE_MAX_SIZE); virtio_queue_set_num(vdev, vdev->queue_sel, value); + /* Note: only call this function for legacy devices */ + virtio_queue_update_rings(vdev, vdev->queue_sel); break; case VIRTIO_MMIO_QUEUEALIGN: + /* Note: this is only valid for legacy devices */ virtio_queue_set_align(vdev, vdev->queue_sel, value); break; case VIRTIO_MMIO_QUEUEPFN: diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 8f69ffa..57190ba 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -69,7 +69,6 @@ typedef struct VRing struct VirtQueue { VRing vring; - hwaddr pa; uint16_t last_avail_idx; /* Last used index value we have signalled on */ uint16_t signalled_used; @@ -92,15 +91,18 @@ struct VirtQueue }; /* virt queue functions */ -static void virtqueue_init(VirtQueue *vq) +void virtio_queue_update_rings(VirtIODevice *vdev, int n) { - hwaddr pa = vq->pa; + VRing *vring = &vdev->vq[n].vring; - vq->vring.desc = pa; - vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc); - vq->vring.used = vring_align(vq->vring.avail + - offsetof(VRingAvail, ring[vq->vring.num]), - vq->vring.align); + if (!vring->desc) { + /* not yet setup -> nothing to do */ + return; + } + vring->avail = vring->desc + vring->num * sizeof(VRingDesc); + vring->used = vring_align(vring->avail + + offsetof(VRingAvail, ring[vring->num]), + vring->align); } static inline uint64_t vring_desc_addr(VirtIODevice *vdev, hwaddr desc_pa, @@ -605,7 +607,6 @@ void virtio_reset(void *opaque) vdev->vq[i].vring.avail = 0; vdev->vq[i].vring.used = 0; vdev->vq[i].last_avail_idx = 0; - vdev->vq[i].pa = 0; vdev->vq[i].vector = VIRTIO_NO_VECTOR; vdev->vq[i].signalled_used = 0; vdev->vq[i].signalled_used_valid = false; @@ -708,13 +709,21 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) { - vdev->vq[n].pa = addr; - virtqueue_init(&vdev->vq[n]); + vdev->vq[n].vring.desc = addr; + virtio_queue_update_rings(vdev, n); } hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n) { - return vdev->vq[n].pa; + return vdev->vq[n].vring.desc; +} + +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, + hwaddr avail, hwaddr used) +{ + vdev->vq[n].vring.desc = desc; + vdev->vq[n].vring.avail = avail; + vdev->vq[n].vring.used = used; } void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) @@ -728,7 +737,6 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) return; } vdev->vq[n].vring.num = num; - virtqueue_init(&vdev->vq[n]); } int virtio_queue_get_num(VirtIODevice *vdev, int n) @@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + /* virtio-1 compliant devices cannot change the aligment */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + error_report("tried to modify queue alignment for virtio-1 device"); + return; + } /* Check that the transport told us it was going to do this * (so a buggy transport will immediately assert rather than * silently failing to migrate this state) @@ -755,7 +768,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) assert(k->has_variable_vring_alignment); vdev->vq[n].vring.align = align; - virtqueue_init(&vdev->vq[n]); + virtio_queue_update_rings(vdev, n); } void virtio_queue_notify_vq(VirtQueue *vq) @@ -949,7 +962,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) if (k->has_variable_vring_alignment) { qemu_put_be32(f, vdev->vq[i].vring.align); } - qemu_put_be64(f, vdev->vq[i].pa); + /* XXX virtio-1 devices */ + qemu_put_be64(f, vdev->vq[i].vring.desc); qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); if (k->save_queue) { k->save_queue(qbus->parent, i, f); @@ -1044,13 +1058,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) if (k->has_variable_vring_alignment) { vdev->vq[i].vring.align = qemu_get_be32(f); } - vdev->vq[i].pa = qemu_get_be64(f); + vdev->vq[i].vring.desc = qemu_get_be64(f); qemu_get_be16s(f, &vdev->vq[i].last_avail_idx); vdev->vq[i].signalled_used_valid = false; vdev->vq[i].notification = true; - if (vdev->vq[i].pa) { - virtqueue_init(&vdev->vq[i]); + if (vdev->vq[i].vring.desc) { + /* XXX virtio-1 devices */ + virtio_queue_update_rings(vdev, i); } else if (vdev->vq[i].last_avail_idx) { error_report("VQ %d address 0x0 " "inconsistent with Host index 0x%x", @@ -1084,7 +1099,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) } for (i = 0; i < num; i++) { - if (vdev->vq[i].pa) { + if (vdev->vq[i].vring.desc) { uint16_t nheads; nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; /* Check it isn't doing strange things with descriptor numbers. */ diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 68c40db..b63ced3 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -224,6 +224,9 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr); hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n); void virtio_queue_set_num(VirtIODevice *vdev, int n, int num); int virtio_queue_get_num(VirtIODevice *vdev, int n); +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, + hwaddr avail, hwaddr used); +void virtio_queue_update_rings(VirtIODevice *vdev, int n); void virtio_queue_set_align(VirtIODevice *vdev, int n, int align); void virtio_queue_notify(VirtIODevice *vdev, int n); uint16_t virtio_queue_vector(VirtIODevice *vdev, int n); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 08/20] dataplane: allow virtio-1 devices
Handle endianness conversion for virtio-1 virtqueues correctly. Note that dataplane now needs to be built per-target. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/block/dataplane/virtio-blk.c | 4 +- hw/scsi/virtio-scsi-dataplane.c | 2 +- hw/virtio/Makefile.objs | 2 +- hw/virtio/dataplane/Makefile.objs | 2 +- hw/virtio/dataplane/vring.c | 86 ++++++++++++++----------- include/hw/virtio/dataplane/vring-accessors.h | 75 +++++++++++++++++++++ include/hw/virtio/dataplane/vring.h | 14 +--- 7 files changed, 131 insertions(+), 54 deletions(-) create mode 100644 include/hw/virtio/dataplane/vring-accessors.h diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 1222a37..2d8cc15 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -16,7 +16,9 @@ #include "qemu/iov.h" #include "qemu/thread.h" #include "qemu/error-report.h" +#include "hw/virtio/virtio-access.h" #include "hw/virtio/dataplane/vring.h" +#include "hw/virtio/dataplane/vring-accessors.h" #include "sysemu/block-backend.h" #include "hw/virtio/virtio-blk.h" #include "virtio-blk.h" @@ -75,7 +77,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) VirtIOBlockDataPlane *s = req->dev->dataplane; stb_p(&req->in->status, status); - vring_push(&req->dev->dataplane->vring, &req->elem, + vring_push(s->vdev, &req->dev->dataplane->vring, &req->elem, req->qiov.size + sizeof(*req->in)); /* Suppress notification to guest by BH and its scheduled diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 03a1e8c..418d73b 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -94,7 +94,7 @@ void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) { VirtIODevice *vdev = VIRTIO_DEVICE(req->vring->parent); - vring_push(&req->vring->vring, &req->elem, + vring_push(vdev, &req->vring->vring, &req->elem, req->qsgl.size + req->resp_iov.size); if (vring_should_notify(vdev, &req->vring->vring)) { diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d21c397..19b224a 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o -common-obj-$(CONFIG_VIRTIO) += dataplane/ +obj-$(CONFIG_VIRTIO) += dataplane/ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs index 9a8cfc0..753a9ca 100644 --- a/hw/virtio/dataplane/Makefile.objs +++ b/hw/virtio/dataplane/Makefile.objs @@ -1 +1 @@ -common-obj-y += vring.o +obj-y += vring.o diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index 6e283fc..a44c8c8 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -18,7 +18,9 @@ #include "hw/hw.h" #include "exec/memory.h" #include "exec/address-spaces.h" +#include "hw/virtio/virtio-access.h" #include "hw/virtio/dataplane/vring.h" +#include "hw/virtio/dataplane/vring-accessors.h" #include "qemu/error-report.h" /* vring_map can be coupled with vring_unmap or (if you still have the @@ -83,7 +85,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring_init(&vring->vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096); vring->last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n); - vring->last_used_idx = vring->vr.used->idx; + vring->last_used_idx = vring_get_used_idx(vdev, vring); vring->signalled_used = 0; vring->signalled_used_valid = false; @@ -104,7 +106,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { if (!virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { - vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY; + vring_set_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } } @@ -117,10 +119,10 @@ bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) if (virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(&vring->vr) = vring->vr.avail->idx; } else { - vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY; + vring_clear_used_flags(vdev, vring, VRING_USED_F_NO_NOTIFY); } smp_mb(); /* ensure update is seen before reading avail_idx */ - return !vring_more_avail(vring); + return !vring_more_avail(vdev, vring); } /* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify() */ @@ -134,12 +136,13 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) smp_mb(); if (virtio_has_feature(vdev, VIRTIO_F_NOTIFY_ON_EMPTY) && - unlikely(vring->vr.avail->idx == vring->last_avail_idx)) { + unlikely(!vring_more_avail(vdev, vring))) { return true; } if (!virtio_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) { - return !(vring->vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); + return !(vring_get_avail_flags(vdev, vring) & + VRING_AVAIL_F_NO_INTERRUPT); } old = vring->signalled_used; v = vring->signalled_used_valid; @@ -154,15 +157,18 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) } -static int get_desc(Vring *vring, VirtQueueElement *elem, +static int get_desc(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, struct vring_desc *desc) { unsigned *num; struct iovec *iov; hwaddr *addr; MemoryRegion *mr; + int is_write = virtio_tswap16(vdev, desc->flags) & VRING_DESC_F_WRITE; + uint32_t len = virtio_tswap32(vdev, desc->len); + uint64_t desc_addr = virtio_tswap64(vdev, desc->addr); - if (desc->flags & VRING_DESC_F_WRITE) { + if (is_write) { num = &elem->in_num; iov = &elem->in_sg[*num]; addr = &elem->in_addr[*num]; @@ -186,44 +192,45 @@ static int get_desc(Vring *vring, VirtQueueElement *elem, } /* TODO handle non-contiguous memory across region boundaries */ - iov->iov_base = vring_map(&mr, desc->addr, desc->len, - desc->flags & VRING_DESC_F_WRITE); + iov->iov_base = vring_map(&mr, desc_addr, len, is_write); if (!iov->iov_base) { error_report("Failed to map descriptor addr %#" PRIx64 " len %u", - (uint64_t)desc->addr, desc->len); + (uint64_t)desc_addr, len); return -EFAULT; } /* The MemoryRegion is looked up again and unref'ed later, leave the * ref in place. */ - iov->iov_len = desc->len; - *addr = desc->addr; + iov->iov_len = len; + *addr = desc_addr; *num += 1; return 0; } /* This is stolen from linux/drivers/vhost/vhost.c. */ -static int get_indirect(Vring *vring, VirtQueueElement *elem, - struct vring_desc *indirect) +static int get_indirect(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, struct vring_desc *indirect) { struct vring_desc desc; unsigned int i = 0, count, found = 0; int ret; + uint32_t len = virtio_tswap32(vdev, indirect->len); + uint64_t addr = virtio_tswap64(vdev, indirect->addr); /* Sanity check */ - if (unlikely(indirect->len % sizeof(desc))) { + if (unlikely(len % sizeof(desc))) { error_report("Invalid length in indirect descriptor: " "len %#x not multiple of %#zx", - indirect->len, sizeof(desc)); + len, sizeof(desc)); vring->broken = true; return -EFAULT; } - count = indirect->len / sizeof(desc); + count = len / sizeof(desc); /* Buffers are chained via a 16 bit next field, so * we can have at most 2^16 of these. */ if (unlikely(count > USHRT_MAX + 1)) { - error_report("Indirect buffer length too big: %d", indirect->len); + error_report("Indirect buffer length too big: %d", len); vring->broken = true; return -EFAULT; } @@ -234,12 +241,12 @@ static int get_indirect(Vring *vring, VirtQueueElement *elem, /* Translate indirect descriptor */ desc_ptr = vring_map(&mr, - indirect->addr + found * sizeof(desc), + addr + found * sizeof(desc), sizeof(desc), false); if (!desc_ptr) { error_report("Failed to map indirect descriptor " "addr %#" PRIx64 " len %zu", - (uint64_t)indirect->addr + found * sizeof(desc), + (uint64_t)addr + found * sizeof(desc), sizeof(desc)); vring->broken = true; return -EFAULT; @@ -257,19 +264,20 @@ static int get_indirect(Vring *vring, VirtQueueElement *elem, return -EFAULT; } - if (unlikely(desc.flags & VRING_DESC_F_INDIRECT)) { + if (unlikely(virtio_tswap16(vdev, desc.flags) + & VRING_DESC_F_INDIRECT)) { error_report("Nested indirect descriptor"); vring->broken = true; return -EFAULT; } - ret = get_desc(vring, elem, &desc); + ret = get_desc(vdev, vring, elem, &desc); if (ret < 0) { vring->broken |= (ret == -EFAULT); return ret; } - i = desc.next; - } while (desc.flags & VRING_DESC_F_NEXT); + i = virtio_tswap16(vdev, desc.next); + } while (virtio_tswap16(vdev, desc.flags) & VRING_DESC_F_NEXT); return 0; } @@ -320,7 +328,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vring->last_avail_idx; - avail_idx = vring->vr.avail->idx; + avail_idx = vring_get_avail_idx(vdev, vring); barrier(); /* load indices now and not again later */ if (unlikely((uint16_t)(avail_idx - last_avail_idx) > num)) { @@ -341,7 +349,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Grab the next descriptor number they're advertising, and increment * the index we've seen. */ - head = vring->vr.avail->ring[last_avail_idx % num]; + head = vring_get_avail_ring(vdev, vring, last_avail_idx % num); elem->index = head; @@ -370,21 +378,21 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Ensure descriptor is loaded before accessing fields */ barrier(); - if (desc.flags & VRING_DESC_F_INDIRECT) { - ret = get_indirect(vring, elem, &desc); + if (virtio_tswap16(vdev, desc.flags) & VRING_DESC_F_INDIRECT) { + ret = get_indirect(vdev, vring, elem, &desc); if (ret < 0) { goto out; } continue; } - ret = get_desc(vring, elem, &desc); + ret = get_desc(vdev, vring, elem, &desc); if (ret < 0) { goto out; } - i = desc.next; - } while (desc.flags & VRING_DESC_F_NEXT); + i = virtio_tswap16(vdev, desc.next); + } while (virtio_tswap16(vdev, desc.flags) & VRING_DESC_F_NEXT); /* On success, increment avail index. */ vring->last_avail_idx++; @@ -407,9 +415,9 @@ out: * * Stolen from linux/drivers/vhost/vhost.c. */ -void vring_push(Vring *vring, VirtQueueElement *elem, int len) +void vring_push(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, + int len) { - struct vring_used_elem *used; unsigned int head = elem->index; uint16_t new; @@ -422,14 +430,16 @@ void vring_push(Vring *vring, VirtQueueElement *elem, int len) /* The virtqueue contains a ring of used buffers. Get a pointer to the * next entry in that used ring. */ - used = &vring->vr.used->ring[vring->last_used_idx % vring->vr.num]; - used->id = head; - used->len = len; + vring_set_used_ring_id(vdev, vring, vring->last_used_idx % vring->vr.num, + head); + vring_set_used_ring_len(vdev, vring, vring->last_used_idx % vring->vr.num, + len); /* Make sure buffer is written before we update index. */ smp_wmb(); - new = vring->vr.used->idx = ++vring->last_used_idx; + new = ++vring->last_used_idx; + vring_set_used_idx(vdev, vring, new); if (unlikely((int16_t)(new - vring->signalled_used) < (uint16_t)1)) { vring->signalled_used_valid = false; } diff --git a/include/hw/virtio/dataplane/vring-accessors.h b/include/hw/virtio/dataplane/vring-accessors.h new file mode 100644 index 0000000..b508b87 --- /dev/null +++ b/include/hw/virtio/dataplane/vring-accessors.h @@ -0,0 +1,75 @@ +#ifndef VRING_ACCESSORS_H +#define VRING_ACCESSORS_H + +#include "hw/virtio/virtio_ring.h" +#include "hw/virtio/virtio.h" +#include "hw/virtio/virtio-access.h" + +static inline uint16_t vring_get_used_idx(VirtIODevice *vdev, Vring *vring) +{ + return virtio_tswap16(vdev, vring->vr.used->idx); +} + +static inline void vring_set_used_idx(VirtIODevice *vdev, Vring *vring, + uint16_t idx) +{ + vring->vr.used->idx = virtio_tswap16(vdev, idx); +} + +static inline uint16_t vring_get_avail_idx(VirtIODevice *vdev, Vring *vring) +{ + return virtio_tswap16(vdev, vring->vr.avail->idx); +} + +static inline uint16_t vring_get_avail_ring(VirtIODevice *vdev, Vring *vring, + int i) +{ + return virtio_tswap16(vdev, vring->vr.avail->ring[i]); +} + +static inline void vring_set_used_ring_id(VirtIODevice *vdev, Vring *vring, + int i, uint32_t id) +{ + vring->vr.used->ring[i].id = virtio_tswap32(vdev, id); +} + +static inline void vring_set_used_ring_len(VirtIODevice *vdev, Vring *vring, + int i, uint32_t len) +{ + vring->vr.used->ring[i].len = virtio_tswap32(vdev, len); +} + +static inline uint16_t vring_get_used_flags(VirtIODevice *vdev, Vring *vring) +{ + return virtio_tswap16(vdev, vring->vr.used->flags); +} + +static inline uint16_t vring_get_avail_flags(VirtIODevice *vdev, Vring *vring) +{ + return virtio_tswap16(vdev, vring->vr.avail->flags); +} + +static inline void vring_set_used_flags(VirtIODevice *vdev, Vring *vring, + uint16_t flags) +{ + vring->vr.used->flags |= virtio_tswap16(vdev, flags); +} + +static inline void vring_clear_used_flags(VirtIODevice *vdev, Vring *vring, + uint16_t flags) +{ + vring->vr.used->flags &= virtio_tswap16(vdev, ~flags); +} + +static inline unsigned int vring_get_num(Vring *vring) +{ + return vring->vr.num; +} + +/* Are there more descriptors available? */ +static inline bool vring_more_avail(VirtIODevice *vdev, Vring *vring) +{ + return vring_get_avail_idx(vdev, vring) != vring->last_avail_idx; +} + +#endif diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h index d3e086a..e42c0fc 100644 --- a/include/hw/virtio/dataplane/vring.h +++ b/include/hw/virtio/dataplane/vring.h @@ -31,17 +31,6 @@ typedef struct { bool broken; /* was there a fatal error? */ } Vring; -static inline unsigned int vring_get_num(Vring *vring) -{ - return vring->vr.num; -} - -/* Are there more descriptors available? */ -static inline bool vring_more_avail(Vring *vring) -{ - return vring->vr.avail->idx != vring->last_avail_idx; -} - /* Fail future vring_pop() and vring_push() calls until reset */ static inline void vring_set_broken(Vring *vring) { @@ -54,6 +43,7 @@ void vring_disable_notification(VirtIODevice *vdev, Vring *vring); bool vring_enable_notification(VirtIODevice *vdev, Vring *vring); bool vring_should_notify(VirtIODevice *vdev, Vring *vring); int vring_pop(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem); -void vring_push(Vring *vring, VirtQueueElement *elem, int len); +void vring_push(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, + int len); #endif /* VRING_H */ -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 09/20] s390x/css: Add a callback for when subchannel gets disabled
From: Thomas Huth <thuth at linux.vnet.ibm.com> We need a possibility to run code when a subchannel gets disabled. This patch adds the necessary infrastructure. Signed-off-by: Thomas Huth <thuth at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/css.c | 12 ++++++++++++ hw/s390x/css.h | 1 + 2 files changed, 13 insertions(+) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index b67c039..735ec55 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -588,6 +588,7 @@ int css_do_msch(SubchDev *sch, SCHIB *orig_schib) { SCSW *s = &sch->curr_status.scsw; PMCW *p = &sch->curr_status.pmcw; + uint16_t oldflags; int ret; SCHIB schib; @@ -610,6 +611,7 @@ int css_do_msch(SubchDev *sch, SCHIB *orig_schib) copy_schib_from_guest(&schib, orig_schib); /* Only update the program-modifiable fields. */ p->intparm = schib.pmcw.intparm; + oldflags = p->flags; p->flags &= ~(PMCW_FLAGS_MASK_ISC | PMCW_FLAGS_MASK_ENA | PMCW_FLAGS_MASK_LM | PMCW_FLAGS_MASK_MME | PMCW_FLAGS_MASK_MP); @@ -625,6 +627,12 @@ int css_do_msch(SubchDev *sch, SCHIB *orig_schib) (PMCW_CHARS_MASK_MBFC | PMCW_CHARS_MASK_CSENSE); sch->curr_status.mba = schib.mba; + /* Has the channel been disabled? */ + if (sch->disable_cb && (oldflags & PMCW_FLAGS_MASK_ENA) != 0 + && (p->flags & PMCW_FLAGS_MASK_ENA) == 0) { + sch->disable_cb(sch); + } + ret = 0; out: @@ -1443,6 +1451,10 @@ void css_reset_sch(SubchDev *sch) { PMCW *p = &sch->curr_status.pmcw; + if ((p->flags & PMCW_FLAGS_MASK_ENA) != 0 && sch->disable_cb) { + sch->disable_cb(sch); + } + p->intparm = 0; p->flags &= ~(PMCW_FLAGS_MASK_ISC | PMCW_FLAGS_MASK_ENA | PMCW_FLAGS_MASK_LM | PMCW_FLAGS_MASK_MME | diff --git a/hw/s390x/css.h b/hw/s390x/css.h index 33104ac..7fa807b 100644 --- a/hw/s390x/css.h +++ b/hw/s390x/css.h @@ -81,6 +81,7 @@ struct SubchDev { uint8_t ccw_no_data_cnt; /* transport-provided data: */ int (*ccw_cb) (SubchDev *, CCW1); + void (*disable_cb)(SubchDev *); SenseId id; void *driver_data; }; -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 10/20] s390x/virtio-ccw: add virtio set-revision call
From: Thomas Huth <thuth at linux.vnet.ibm.com> Handle the virtio-ccw revision according to what the guest sets. When revision 1 is selected, we have a virtio-1 standard device with byteswapping for the virtio rings. When a channel gets disabled, we have to revert to the legacy behavior in case the next user of the device does not negotiate the revision 1 anymore (e.g. the boot firmware uses revision 1, but the operating system only uses the legacy mode). Note that revisions > 0 are still disabled. Signed-off-by: Thomas Huth <thuth at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/virtio-ccw.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ hw/s390x/virtio-ccw.h | 8 ++++++++ 2 files changed, 60 insertions(+) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index fbd909d..ea2c6f0 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -20,9 +20,11 @@ #include "hw/virtio/virtio-net.h" #include "hw/sysbus.h" #include "qemu/bitops.h" +#include "hw/virtio/virtio-access.h" #include "hw/virtio/virtio-bus.h" #include "hw/s390x/adapter.h" #include "hw/s390x/s390_flic.h" +#include "linux/virtio_config.h" #include "ioinst.h" #include "css.h" @@ -260,6 +262,12 @@ typedef struct VirtioThinintInfo { uint8_t isc; } QEMU_PACKED VirtioThinintInfo; +typedef struct VirtioRevInfo { + uint16_t revision; + uint16_t length; + uint8_t data[0]; +} QEMU_PACKED VirtioRevInfo; + /* Specify where the virtqueues for the subchannel are in guest memory. */ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align, uint16_t index, uint16_t num) @@ -299,6 +307,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) { int ret; VqInfoBlock info; + VirtioRevInfo revinfo; uint8_t status; VirtioFeatDesc features; void *config; @@ -375,6 +384,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) features.features = (uint32_t)dev->host_features; } else if (features.index == 1) { features.features = (uint32_t)(dev->host_features >> 32); + /* + * Don't offer version 1 to the guest if it did not + * negotiate at least revision 1. + */ + if (dev->revision <= 0) { + features.features &= ~(1 << (VIRTIO_F_VERSION_1 - 32)); + } } else { /* Return zeroes if the guest supports more feature bits. */ features.features = 0; @@ -406,6 +422,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) (vdev->guest_features & 0xffffffff00000000) | features.features); } else if (features.index == 1) { + /* + * The guest should not set version 1 if it didn't + * negotiate a revision >= 1. + */ + if (dev->revision <= 0) { + features.features &= ~(1 << (VIRTIO_F_VERSION_1 - 32)); + } virtio_set_features(vdev, (vdev->guest_features & 0x00000000ffffffff) | ((uint64_t)features.features << 32)); @@ -608,6 +631,25 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) } } break; + case CCW_CMD_SET_VIRTIO_REV: + len = sizeof(revinfo); + if (ccw.count < len || (check_len && ccw.count > len)) { + ret = -EINVAL; + break; + } + if (!ccw.cda) { + ret = -EFAULT; + break; + } + cpu_physical_memory_read(ccw.cda, &revinfo, len); + if (dev->revision >= 0 || + revinfo.revision > virtio_ccw_rev_max(dev)) { + ret = -ENOSYS; + break; + } + ret = 0; + dev->revision = revinfo.revision; + break; default: ret = -ENOSYS; break; @@ -615,6 +657,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) return ret; } +static void virtio_sch_disable_cb(SubchDev *sch) +{ + VirtioCcwDevice *dev = sch->driver_data; + + dev->revision = -1; +} + static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) { unsigned int cssid = 0; @@ -740,6 +789,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) css_sch_build_virtual_schib(sch, 0, VIRTIO_CCW_CHPID_TYPE); sch->ccw_cb = virtio_ccw_cb; + sch->disable_cb = virtio_sch_disable_cb; /* Build senseid data. */ memset(&sch->id, 0, sizeof(SenseId)); @@ -747,6 +797,8 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) sch->id.cu_type = VIRTIO_CCW_CU_TYPE; sch->id.cu_model = vdev->device_id; + dev->revision = -1; + /* Set default feature bits that are offered by the host. */ virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY); virtio_add_feature(&dev->host_features, VIRTIO_F_BAD_FEATURE); diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 9087f7a..778ccb9 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -40,6 +40,7 @@ #define CCW_CMD_SET_CONF_IND 0x53 #define CCW_CMD_READ_VQ_CONF 0x32 #define CCW_CMD_SET_IND_ADAPTER 0x73 +#define CCW_CMD_SET_VIRTIO_REV 0x83 #define TYPE_VIRTIO_CCW_DEVICE "virtio-ccw-device" #define VIRTIO_CCW_DEVICE(obj) \ @@ -86,6 +87,7 @@ struct VirtioCcwDevice { SubchDev *sch; char *bus_id; uint64_t host_features; + int revision; VirtioBusState bus; bool ioeventfd_started; bool ioeventfd_disabled; @@ -99,6 +101,12 @@ struct VirtioCcwDevice { uint64_t ind_bit; }; +/* The maximum virtio revision we support. */ +static inline int virtio_ccw_rev_max(VirtioCcwDevice *dev) +{ + return 0; +} + /* virtual css bus type */ typedef struct VirtualCssBus { BusState parent_obj; -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 11/20] s390x/virtio-ccw: support virtio-1 set_vq format
Support the new CCW_CMD_SET_VQ format for virtio-1 devices. While we're at it, refactor the code a bit and enforce big endian fields (which had always been required, even for legacy). Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/virtio-ccw.c | 114 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 80 insertions(+), 34 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index ea2c6f0..e09e0da 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -238,11 +238,20 @@ VirtualCssBus *virtual_css_bus_init(void) } /* Communication blocks used by several channel commands. */ -typedef struct VqInfoBlock { +typedef struct VqInfoBlockLegacy { uint64_t queue; uint32_t align; uint16_t index; uint16_t num; +} QEMU_PACKED VqInfoBlockLegacy; + +typedef struct VqInfoBlock { + uint64_t desc; + uint32_t res0; + uint16_t index; + uint16_t num; + uint64_t avail; + uint64_t used; } QEMU_PACKED VqInfoBlock; typedef struct VqConfigBlock { @@ -269,17 +278,20 @@ typedef struct VirtioRevInfo { } QEMU_PACKED VirtioRevInfo; /* Specify where the virtqueues for the subchannel are in guest memory. */ -static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align, - uint16_t index, uint16_t num) +static int virtio_ccw_set_vqs(SubchDev *sch, VqInfoBlock *info, + VqInfoBlockLegacy *linfo) { VirtIODevice *vdev = virtio_ccw_get_vdev(sch); + uint16_t index = info ? info->index : linfo->index; + uint16_t num = info ? info->num : linfo->num; + uint64_t desc = info ? info->desc : linfo->queue; if (index > VIRTIO_PCI_QUEUE_MAX) { return -EINVAL; } /* Current code in virtio.c relies on 4K alignment. */ - if (addr && (align != 4096)) { + if (linfo && desc && (linfo->align != 4096)) { return -EINVAL; } @@ -287,8 +299,12 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align, return -EINVAL; } - virtio_queue_set_addr(vdev, index, addr); - if (!addr) { + if (info) { + virtio_queue_set_rings(vdev, index, desc, info->avail, info->used); + } else { + virtio_queue_set_addr(vdev, index, desc); + } + if (!desc) { virtio_queue_set_vector(vdev, index, 0); } else { /* Fail if we don't have a big enough queue. */ @@ -303,10 +319,66 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align, return 0; } -static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) +static int virtio_ccw_handle_set_vq(SubchDev *sch, CCW1 ccw, bool check_len, + bool is_legacy) { int ret; VqInfoBlock info; + VqInfoBlockLegacy linfo; + size_t info_len = is_legacy ? sizeof(linfo) : sizeof(info); + + if (check_len) { + if (ccw.count != info_len) { + return -EINVAL; + } + } else if (ccw.count < info_len) { + /* Can't execute command. */ + return -EINVAL; + } + if (!ccw.cda) { + return -EFAULT; + } + if (is_legacy) { + linfo.queue = ldq_be_phys(&address_space_memory, ccw.cda); + linfo.align = ldl_be_phys(&address_space_memory, + ccw.cda + sizeof(linfo.queue)); + linfo.index = lduw_be_phys(&address_space_memory, + ccw.cda + sizeof(linfo.queue) + + sizeof(linfo.align)); + linfo.num = lduw_be_phys(&address_space_memory, + ccw.cda + sizeof(linfo.queue) + + sizeof(linfo.align) + + sizeof(linfo.index)); + ret = virtio_ccw_set_vqs(sch, NULL, &linfo); + } else { + info.desc = ldq_be_phys(&address_space_memory, ccw.cda); + info.index = lduw_be_phys(&address_space_memory, + ccw.cda + sizeof(info.desc) + + sizeof(info.res0)); + info.num = lduw_be_phys(&address_space_memory, + ccw.cda + sizeof(info.desc) + + sizeof(info.res0) + + sizeof(info.index)); + info.avail = ldq_be_phys(&address_space_memory, + ccw.cda + sizeof(info.desc) + + sizeof(info.res0) + + sizeof(info.index) + + sizeof(info.num)); + info.used = ldq_be_phys(&address_space_memory, + ccw.cda + sizeof(info.desc) + + sizeof(info.res0) + + sizeof(info.index) + + sizeof(info.num) + + sizeof(info.avail)); + ret = virtio_ccw_set_vqs(sch, &info, NULL); + } + sch->curr_status.scsw.count = 0; + return ret; +} + +static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) +{ + int ret; VirtioRevInfo revinfo; uint8_t status; VirtioFeatDesc features; @@ -331,33 +403,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) /* Look at the command. */ switch (ccw.cmd_code) { case CCW_CMD_SET_VQ: - if (check_len) { - if (ccw.count != sizeof(info)) { - ret = -EINVAL; - break; - } - } else if (ccw.count < sizeof(info)) { - /* Can't execute command. */ - ret = -EINVAL; - break; - } - if (!ccw.cda) { - ret = -EFAULT; - } else { - info.queue = ldq_phys(&address_space_memory, ccw.cda); - info.align = ldl_phys(&address_space_memory, - ccw.cda + sizeof(info.queue)); - info.index = lduw_phys(&address_space_memory, - ccw.cda + sizeof(info.queue) - + sizeof(info.align)); - info.num = lduw_phys(&address_space_memory, - ccw.cda + sizeof(info.queue) - + sizeof(info.align) - + sizeof(info.index)); - ret = virtio_ccw_set_vqs(sch, info.queue, info.align, info.index, - info.num); - sch->curr_status.scsw.count = 0; - } + ret = virtio_ccw_handle_set_vq(sch, ccw, check_len, dev->revision < 1); break; case CCW_CMD_VDEV_RESET: virtio_ccw_stop_ioeventfd(dev); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 12/20] virtio: disallow late feature changes for virtio-1
For virtio-1 devices, the driver must not attempt to set feature bits after it set FEATURES_OK in the device status. Simply reject it in that case. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/virtio/virtio.c | 16 ++++++++++++++-- include/hw/virtio/virtio.h | 2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 57190ba..a3dd67b 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -978,7 +978,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) vmstate_save_state(f, &vmstate_virtio, vdev); } -int virtio_set_features(VirtIODevice *vdev, uint64_t val) +static int __virtio_set_features(VirtIODevice *vdev, uint64_t val) { BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *vbusk = VIRTIO_BUS_GET_CLASS(qbus); @@ -994,6 +994,18 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) return bad ? -1 : 0; } +int virtio_set_features(VirtIODevice *vdev, uint64_t val) +{ + /* + * The driver must not attempt to set features after feature negotiation + * has finished. + */ + if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { + return -EINVAL; + } + return __virtio_set_features(vdev, val); +} + int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) { int i, ret; @@ -1026,7 +1038,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) qemu_get_be32s(f, &features); /* XXX features >= 32 */ - if (virtio_set_features(vdev, features) < 0) { + if (__virtio_set_features(vdev, features) < 0) { supported_features = k->get_features(qbus->parent); error_report("Features 0x%x unsupported. Allowed features: 0x%lx", features, supported_features); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index b63ced3..a24e403 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -32,6 +32,8 @@ #define VIRTIO_CONFIG_S_DRIVER 2 /* Driver has used its parts of the config, and is happy */ #define VIRTIO_CONFIG_S_DRIVER_OK 4 +/* Driver has finished configuring features */ +#define VIRTIO_CONFIG_S_FEATURES_OK 8 /* We've given up on this device. */ #define VIRTIO_CONFIG_S_FAILED 0x80 -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 13/20] virtio: allow to fail setting status
virtio-1 allow setting of the FEATURES_OK status bit to fail if the negotiated feature bits are inconsistent: let's fail virtio_set_status() in that case and update virtio-ccw to post an error to the guest. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/virtio-ccw.c | 20 ++++++++++++-------- hw/virtio/virtio.c | 24 +++++++++++++++++++++++- include/hw/virtio/virtio.h | 3 ++- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index e09e0da..a55e851 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -555,15 +555,19 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) { virtio_ccw_stop_ioeventfd(dev); } - virtio_set_status(vdev, status); - if (vdev->status == 0) { - virtio_reset(vdev); - } - if (status & VIRTIO_CONFIG_S_DRIVER_OK) { - virtio_ccw_start_ioeventfd(dev); + if (virtio_set_status(vdev, status) == 0) { + if (vdev->status == 0) { + virtio_reset(vdev); + } + if (status & VIRTIO_CONFIG_S_DRIVER_OK) { + virtio_ccw_start_ioeventfd(dev); + } + sch->curr_status.scsw.count = ccw.count - sizeof(status); + ret = 0; + } else { + /* Trigger a command reject. */ + ret = -ENOSYS; } - sch->curr_status.scsw.count = ccw.count - sizeof(status); - ret = 0; } break; case CCW_CMD_SET_IND: diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index a3dd67b..90eedd3 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -543,15 +543,37 @@ void virtio_update_irq(VirtIODevice *vdev) virtio_notify_vector(vdev, VIRTIO_NO_VECTOR); } -void virtio_set_status(VirtIODevice *vdev, uint8_t val) +static int virtio_validate_features(VirtIODevice *vdev) +{ + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); + + if (k->validate_features) { + return k->validate_features(vdev); + } else { + return 0; + } +} + +int virtio_set_status(VirtIODevice *vdev, uint8_t val) { VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); trace_virtio_set_status(vdev, val); + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + if (!(vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) && + val & VIRTIO_CONFIG_S_FEATURES_OK) { + int ret = virtio_validate_features(vdev); + + if (ret) { + return ret; + } + } + } if (k->set_status) { k->set_status(vdev, val); } vdev->status = val; + return 0; } bool target_words_bigendian(void); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index a24e403..068211e 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -149,6 +149,7 @@ typedef struct VirtioDeviceClass { uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features); uint64_t (*bad_features)(VirtIODevice *vdev); void (*set_features)(VirtIODevice *vdev, uint64_t val); + int (*validate_features)(VirtIODevice *vdev); void (*get_config)(VirtIODevice *vdev, uint8_t *config); void (*set_config)(VirtIODevice *vdev, const uint8_t *config); void (*reset)(VirtIODevice *vdev); @@ -233,7 +234,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align); void virtio_queue_notify(VirtIODevice *vdev, int n); uint16_t virtio_queue_vector(VirtIODevice *vdev, int n); void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector); -void virtio_set_status(VirtIODevice *vdev, uint8_t val); +int virtio_set_status(VirtIODevice *vdev, uint8_t val); void virtio_reset(void *opaque); void virtio_update_irq(VirtIODevice *vdev); int virtio_set_features(VirtIODevice *vdev, uint64_t val); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 14/20] s390x/virtio-ccw: enable virtio 1.0
virtio-ccw should now have everything in place to operate virtio 1.0 devices, so let's enable revision 1. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/virtio-ccw.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 778ccb9..37a9840 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -104,7 +104,7 @@ struct VirtioCcwDevice { /* The maximum virtio revision we support. */ static inline int virtio_ccw_rev_max(VirtioCcwDevice *dev) { - return 0; + return dev->host_features & (1ULL << VIRTIO_F_VERSION_1) ? 1 : 0; } /* virtual css bus type */ -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 15/20] virtio-net: no writeable mac for virtio-1
Devices operating as virtio 1.0 may not allow writes to the mac address in config space. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/net/virtio-net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index d6d1b98..ebbea60 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -87,6 +87,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) memcpy(&netcfg, config, n->config_size); if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR) && + !virtio_has_feature(vdev, VIRTIO_F_VERSION_1) && memcmp(netcfg.mac, n->mac, ETH_ALEN)) { memcpy(n->mac, netcfg.mac, ETH_ALEN); qemu_format_nic_info_str(qemu_get_queue(n->nic), n->mac); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 16/20] virtio-net: support longer header
virtio-1 devices always use num_buffers in the header, even if mergeable rx buffers have not been negotiated. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/net/virtio-net.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index ebbea60..7ee2bd6 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -373,15 +373,21 @@ static int peer_has_ufo(VirtIONet *n) return n->has_ufo; } -static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs) +static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, + int version_1) { int i; NetClientState *nc; n->mergeable_rx_bufs = mergeable_rx_bufs; - n->guest_hdr_len = n->mergeable_rx_bufs ? - sizeof(struct virtio_net_hdr_mrg_rxbuf) : sizeof(struct virtio_net_hdr); + if (version_1) { + n->guest_hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf); + } else { + n->guest_hdr_len = n->mergeable_rx_bufs ? + sizeof(struct virtio_net_hdr_mrg_rxbuf) : + sizeof(struct virtio_net_hdr); + } for (i = 0; i < n->max_queues; i++) { nc = qemu_get_subqueue(n->nic, i); @@ -525,7 +531,9 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) virtio_net_set_mrg_rx_bufs(n, __virtio_has_feature(features, - VIRTIO_NET_F_MRG_RXBUF)); + VIRTIO_NET_F_MRG_RXBUF), + __virtio_has_feature(features, + VIRTIO_F_VERSION_1)); if (n->has_vnet_hdr) { n->curr_guest_offloads @@ -1407,7 +1415,8 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, qemu_get_buffer(f, n->mac, ETH_ALEN); n->vqs[0].tx_waiting = qemu_get_be32(f); - virtio_net_set_mrg_rx_bufs(n, qemu_get_be32(f)); + virtio_net_set_mrg_rx_bufs(n, qemu_get_be32(f), + virtio_has_feature(vdev, VIRTIO_F_VERSION_1)); if (version_id >= 3) n->status = qemu_get_be16(f); @@ -1653,7 +1662,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) n->vqs[0].tx_waiting = 0; n->tx_burst = n->net_conf.txburst; - virtio_net_set_mrg_rx_bufs(n, 0); + virtio_net_set_mrg_rx_bufs(n, 0, 0); n->promisc = 1; /* for compatibility */ n->mac_table.macs = g_malloc0(MAC_TABLE_ENTRIES * ETH_ALEN); -- 1.7.9.5
virtio-net (non-vhost) now should have everything in place to support virtio 1.0: let's enable the feature bit for it. Note that VIRTIO_F_VERSION_1 is technically a transport feature; once every device is ready for virtio 1.0, we can move setting this feature bit out of the individual devices. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/net/virtio-net.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 7ee2bd6..b5dd356 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -473,6 +473,7 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features) } if (!get_vhost_net(nc->peer)) { + virtio_add_feature(&features, VIRTIO_F_VERSION_1); return features; } return vhost_net_get_features(get_vhost_net(nc->peer), features); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 18/20] virtio: support revision-specific features
Devices may support different sets of feature bits depending on which revision they're operating at. Let's give the transport a way to re-query the device about its features when the revision has been changed. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/s390x/virtio-ccw.c | 12 ++++++++++-- hw/virtio/virtio-bus.c | 14 ++++++++++++-- include/hw/virtio/virtio-bus.h | 3 +++ include/hw/virtio/virtio.h | 3 +++ 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index a55e851..8b6b2ab 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -699,6 +699,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) } ret = 0; dev->revision = revinfo.revision; + /* Re-evaluate which features the device wants to offer. */ + dev->host_features + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, + dev->revision >= 1 ? 1 : 0); break; default: ret = -ENOSYS; @@ -712,6 +716,9 @@ static void virtio_sch_disable_cb(SubchDev *sch) VirtioCcwDevice *dev = sch->driver_data; dev->revision = -1; + /* Reset the device's features to legacy. */ + dev->host_features + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, 0); } static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) @@ -853,8 +860,9 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY); virtio_add_feature(&dev->host_features, VIRTIO_F_BAD_FEATURE); - dev->host_features = virtio_bus_get_vdev_features(&dev->bus, - dev->host_features); + /* All devices start in legacy mode. */ + dev->host_features + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, 0); css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, parent->hotplugged, 1); diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c index 32e3fab..a30826c 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -97,18 +97,28 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) } /* Get the features of the plugged device. */ -uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, - uint64_t requested_features) +uint64_t virtio_bus_get_vdev_features_rev(VirtioBusState *bus, + uint64_t requested_features, + unsigned int revision) { VirtIODevice *vdev = virtio_bus_get_device(bus); VirtioDeviceClass *k; assert(vdev != NULL); k = VIRTIO_DEVICE_GET_CLASS(vdev); + if (revision > 0 && k->get_features_rev) { + return k->get_features_rev(vdev, requested_features, revision); + } assert(k->get_features != NULL); return k->get_features(vdev, requested_features); } +uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, + uint64_t requested_features) +{ + return virtio_bus_get_vdev_features_rev(bus, requested_features, 0); +} + /* Get bad features of the plugged device. */ uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) { diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 0a4dde1..f0916ef 100644 --- a/include/hw/virtio/virtio-bus.h +++ b/include/hw/virtio/virtio-bus.h @@ -84,6 +84,9 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); /* Get the features of the plugged device. */ uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, uint64_t requested_features); +uint64_t virtio_bus_get_vdev_features_rev(VirtioBusState *bus, + uint64_t requested_features, + unsigned int revision); /* Get bad features of the plugged device. */ uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); /* Get config of the plugged device. */ diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 068211e..1338540 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -147,6 +147,9 @@ typedef struct VirtioDeviceClass { DeviceRealize realize; DeviceUnrealize unrealize; uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features); + uint64_t (*get_features_rev)(VirtIODevice *vdev, + uint64_t requested_features, + unsigned int revision); uint64_t (*bad_features)(VirtIODevice *vdev); void (*set_features)(VirtIODevice *vdev, uint64_t val); int (*validate_features)(VirtIODevice *vdev); -- 1.7.9.5
Cornelia Huck
2014-Dec-11 13:25 UTC
[PATCH RFC v6 19/20] virtio-blk: revision specific feature bits
Wire up virtio-blk to provide different feature bit sets depending on whether legacy or v1.0 has been requested. Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/block/virtio-blk.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9cfae66..fdc236a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -587,6 +587,24 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features) return features; } +static uint64_t virtio_blk_get_features_rev(VirtIODevice *vdev, + uint64_t features, + unsigned int revision) +{ + if (revision == 0) { + /* legacy */ + virtio_clear_feature(&features, VIRTIO_F_VERSION_1); + return virtio_blk_get_features(vdev, features); + } + /* virtio 1.0 or later */ + virtio_clear_feature(&features, VIRTIO_BLK_F_SCSI); + virtio_clear_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); + virtio_clear_feature(&features, VIRTIO_BLK_F_WCE); + /* we're still missing ANY_LAYOUT */ + /* virtio_add_feature(&features, VIRTIO_F_VERSION_1); */ + return features; +} + static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) { VirtIOBlock *s = VIRTIO_BLK(vdev); @@ -821,6 +839,7 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data) vdc->get_config = virtio_blk_update_config; vdc->set_config = virtio_blk_set_config; vdc->get_features = virtio_blk_get_features; + vdc->get_features_rev = virtio_blk_get_features_rev; vdc->set_status = virtio_blk_set_status; vdc->reset = virtio_blk_reset; vdc->save = virtio_blk_save_device; -- 1.7.9.5
Make sure that all vhost interfaces use 64 bit features, as the virtio core does, and make sure to use ULL everywhere possible to be on the safe side. Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/net/vhost_net.c | 12 ++++++------ hw/virtio/vhost.c | 14 +++++++------- include/hw/virtio/vhost.h | 6 +++--- include/net/vhost_net.h | 4 ++-- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 4e3a061..a6d4ef2 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -107,13 +107,13 @@ static const int *vhost_net_get_feature_bits(struct vhost_net *net) return feature_bits; } -unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) +uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features) { return vhost_get_features(&net->dev, vhost_net_get_feature_bits(net), features); } -void vhost_net_ack_features(struct vhost_net *net, unsigned features) +void vhost_net_ack_features(struct vhost_net *net, uint64_t features) { net->dev.acked_features = net->dev.backend_features; vhost_ack_features(&net->dev, vhost_net_get_feature_bits(net), features); @@ -147,7 +147,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) goto fail; } net->dev.backend_features = qemu_has_vnet_hdr(options->net_backend) - ? 0 : (1 << VHOST_NET_F_VIRTIO_NET_HDR); + ? 0 : (1ULL << VHOST_NET_F_VIRTIO_NET_HDR); net->backend = r; } else { net->dev.backend_features = 0; @@ -166,7 +166,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) if (backend_kernel) { if (!qemu_has_vnet_hdr_len(options->net_backend, sizeof(struct virtio_net_hdr_mrg_rxbuf))) { - net->dev.features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF); + net->dev.features &= ~(1ULL << VIRTIO_NET_F_MRG_RXBUF); } if (~net->dev.features & net->dev.backend_features) { fprintf(stderr, "vhost lacks feature mask %" PRIu64 @@ -423,11 +423,11 @@ void vhost_net_cleanup(struct vhost_net *net) { } -unsigned vhost_net_get_features(struct vhost_net *net, unsigned features) +uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features) { return features; } -void vhost_net_ack_features(struct vhost_net *net, unsigned features) +void vhost_net_ack_features(struct vhost_net *net, uint64_t features) { } diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5a12861..9e6e9cc 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -551,7 +551,7 @@ static int vhost_dev_set_features(struct vhost_dev *dev, bool enable_log) uint64_t features = dev->acked_features; int r; if (enable_log) { - features |= 0x1 << VHOST_F_LOG_ALL; + features |= 0x1ULL << VHOST_F_LOG_ALL; } r = dev->vhost_ops->vhost_call(dev, VHOST_SET_FEATURES, &features); return r < 0 ? -errno : 0; @@ -860,7 +860,7 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, .priority = 10 }; hdev->migration_blocker = NULL; - if (!(hdev->features & (0x1 << VHOST_F_LOG_ALL))) { + if (!(hdev->features & (0x1ULL << VHOST_F_LOG_ALL))) { error_setg(&hdev->migration_blocker, "Migration disabled: vhost lacks VHOST_F_LOG_ALL feature."); migrate_add_blocker(hdev->migration_blocker); @@ -1003,12 +1003,12 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, assert(r >= 0); } -unsigned vhost_get_features(struct vhost_dev *hdev, const int *feature_bits, - unsigned features) +uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits, + uint64_t features) { const int *bit = feature_bits; while (*bit != VHOST_INVALID_FEATURE_BIT) { - unsigned bit_mask = (1 << *bit); + uint64_t bit_mask = (1ULL << *bit); if (!(hdev->features & bit_mask)) { features &= ~bit_mask; } @@ -1018,11 +1018,11 @@ unsigned vhost_get_features(struct vhost_dev *hdev, const int *feature_bits, } void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, - unsigned features) + uint64_t features) { const int *bit = feature_bits; while (*bit != VHOST_INVALID_FEATURE_BIT) { - unsigned bit_mask = (1 << *bit); + uint64_t bit_mask = (1ULL << *bit); if (features & bit_mask) { hdev->acked_features |= bit_mask; } diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index d5593d1..71ef18f 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -72,8 +72,8 @@ bool vhost_virtqueue_pending(struct vhost_dev *hdev, int n); */ void vhost_virtqueue_mask(struct vhost_dev *hdev, VirtIODevice *vdev, int n, bool mask); -unsigned vhost_get_features(struct vhost_dev *hdev, const int *feature_bits, - unsigned features); +uint64_t vhost_get_features(struct vhost_dev *hdev, const int *feature_bits, + uint64_t features); void vhost_ack_features(struct vhost_dev *hdev, const int *feature_bits, - unsigned features); + uint64_t features); #endif diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h index b1c18a3..9eb493e 100644 --- a/include/net/vhost_net.h +++ b/include/net/vhost_net.h @@ -22,8 +22,8 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, int total_queues); void vhost_net_cleanup(VHostNetState *net); -unsigned vhost_net_get_features(VHostNetState *net, unsigned features); -void vhost_net_ack_features(VHostNetState *net, unsigned features); +uint64_t vhost_net_get_features(VHostNetState *net, uint64_t features); +void vhost_net_ack_features(VHostNetState *net, uint64_t features); bool vhost_net_virtqueue_pending(VHostNetState *net, int n); void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev, -- 1.7.9.5
Thomas Huth
2014-Dec-11 14:29 UTC
[PATCH RFC v6 03/20] virtio: feature bit manipulation helpers
On Thu, 11 Dec 2014 14:25:05 +0100 Cornelia Huck <cornelia.huck at de.ibm.com> wrote:> Add virtio_{add,clear}_feature helper functions for manipulating a > feature bits variable. This has some benefits over open coding: > - add check that the bit is in a sane range > - make it obvious at a glance what is going on > - have a central point to change when we want to extend feature bits > > Convert existing code manipulating features to use the new helpers. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/9pfs/virtio-9p-device.c | 2 +- > hw/block/virtio-blk.c | 16 ++++++++-------- > hw/char/virtio-serial-bus.c | 2 +- > hw/net/virtio-net.c | 34 +++++++++++++++++----------------- > hw/s390x/virtio-ccw.c | 4 ++-- > hw/virtio/virtio-mmio.c | 2 +- > hw/virtio/virtio-pci.c | 4 ++-- > include/hw/virtio/virtio.h | 12 ++++++++++++ > 8 files changed, 44 insertions(+), 32 deletions(-)Patch looks fine to me. Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com>
Thomas Huth
2014-Dec-11 14:46 UTC
[PATCH RFC v6 04/20] virtio: add feature checking helpers
On Thu, 11 Dec 2014 14:25:06 +0100 Cornelia Huck <cornelia.huck at de.ibm.com> wrote:> Add a helper function for checking whether a bit is set in the guest > features for a vdev as well as one that works on a feature bit set. > > Convert code that open-coded this: It cleans up the code and makes it > easier to extend the guest feature bits. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>...> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c > index ef48550..56c92fb 100644 > --- a/hw/scsi/virtio-scsi.c > +++ b/hw/scsi/virtio-scsi.c > @@ -144,7 +144,7 @@ static int virtio_scsi_parse_req(VirtIOSCSIReq *req, > * > * TODO: always disable this workaround for virtio 1.0 devices. > */ > - if ((vdev->guest_features & VIRTIO_F_ANY_LAYOUT) == 0) { > + if (!virtio_has_feature(vdev, VIRTIO_F_ANY_LAYOUT)) {Wait ... this does not only look like a clean-up, but also like a bug-fix to me, since it should have been "(1 << VIRTIO_F_ANY_LAYOUT)" instead of "VIRTIO_F_ANY_LAYOUT" in the original code instead? So in case this patch queue takes a little bit longer 'til it gets upstream, do we might want to submit a separate patch for fixing this issue first?> req_size = req->elem.out_sg[0].iov_len; > resp_size = req->elem.in_sg[0].iov_len; > } > @@ -748,7 +748,7 @@ static void virtio_scsi_change(SCSIBus *bus, SCSIDevice *dev, SCSISense sense) > VirtIOSCSI *s = container_of(bus, VirtIOSCSI, bus); > VirtIODevice *vdev = VIRTIO_DEVICE(s); > > - if (((vdev->guest_features >> VIRTIO_SCSI_F_CHANGE) & 1) && > + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_CHANGE) && > dev->type != TYPE_ROM) { > virtio_scsi_push_event(s, dev, VIRTIO_SCSI_T_PARAM_CHANGE, > sense.asc | (sense.ascq << 8)); > @@ -769,7 +769,7 @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, > blk_op_block_all(sd->conf.blk, s->blocker); > } > > - if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { > + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { > virtio_scsi_push_event(s, sd, > VIRTIO_SCSI_T_TRANSPORT_RESET, > VIRTIO_SCSI_EVT_RESET_RESCAN); > @@ -783,7 +783,7 @@ static void virtio_scsi_hotunplug(HotplugHandler *hotplug_dev, DeviceState *dev, > VirtIOSCSI *s = VIRTIO_SCSI(vdev); > SCSIDevice *sd = SCSI_DEVICE(dev); > > - if ((vdev->guest_features >> VIRTIO_SCSI_F_HOTPLUG) & 1) { > + if (virtio_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) { > virtio_scsi_push_event(s, sd, > VIRTIO_SCSI_T_TRANSPORT_RESET, > VIRTIO_SCSI_EVT_RESET_REMOVED);...> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 2fede2e..f6c0379 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -278,6 +278,17 @@ static inline void virtio_clear_feature(uint32_t *features, unsigned int fbit) > *features &= ~(1 << fbit); > } > > +static inline bool __virtio_has_feature(uint32_t features, unsigned int fbit) > +{ > + assert(fbit < 32); > + return !!(features & (1 << fbit)); > +} > + > +static inline bool virtio_has_feature(VirtIODevice *vdev, unsigned int fbit) > +{ > + return __virtio_has_feature(vdev->guest_features, fbit); > +} > +I've got to say that I'm a little bit unhappy with the naming of the functions - and in contrast to the Linux kernel code, I think it is also quite uncommon in the QEMU sources to use function names with double underscores at the beginning. Could you maybe rename the second function to "virtio_vdev_has_feature" instead? And then remove the double underscores from the first function? Thomas
On Thu, 11 Dec 2014 14:25:07 +0100 Cornelia Huck <cornelia.huck at de.ibm.com> wrote:> With virtio-1, we support more than 32 feature bits. Let's extend both > host and guest features to 64, which should suffice for a while. > > vhost and migration have been ignored for now. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/9pfs/virtio-9p-device.c | 2 +- > hw/block/virtio-blk.c | 2 +- > hw/char/virtio-serial-bus.c | 2 +- > hw/core/qdev-properties.c | 58 +++++++++++++++++++++++++++++++++++++++ > hw/net/virtio-net.c | 22 +++++++-------- > hw/s390x/s390-virtio-bus.c | 3 +- > hw/s390x/s390-virtio-bus.h | 2 +- > hw/s390x/virtio-ccw.c | 39 +++++++++++++++----------- > hw/s390x/virtio-ccw.h | 5 +--- > hw/scsi/vhost-scsi.c | 3 +- > hw/scsi/virtio-scsi.c | 4 +-- > hw/virtio/virtio-balloon.c | 2 +- > hw/virtio/virtio-bus.c | 6 ++-- > hw/virtio/virtio-mmio.c | 4 +-- > hw/virtio/virtio-pci.c | 3 +- > hw/virtio/virtio-pci.h | 2 +- > hw/virtio/virtio-rng.c | 2 +- > hw/virtio/virtio.c | 13 +++++---- > include/hw/qdev-properties.h | 12 ++++++++ > include/hw/virtio/virtio-bus.h | 8 +++--- > include/hw/virtio/virtio-net.h | 46 +++++++++++++++---------------- > include/hw/virtio/virtio-scsi.h | 6 ++-- > include/hw/virtio/virtio.h | 38 ++++++++++++++----------- > 23 files changed, 184 insertions(+), 100 deletions(-)...> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 9f3c58a..d6d1b98 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c...> @@ -514,7 +514,7 @@ static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) > return virtio_net_guest_offloads_by_features(vdev->guest_features); > } > > -static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) > +static void virtio_net_set_features(VirtIODevice *vdev, uint64_t features) > { > VirtIONet *n = VIRTIO_NET(vdev); > int i; > @@ -1036,7 +1036,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t > return -1; > error_report("virtio-net unexpected empty queue: " > "i %zd mergeable %d offset %zd, size %zd, " > - "guest hdr len %zd, host hdr len %zd guest features 0x%x", > + "guest hdr len %zd, host hdr len %zd guest features 0x%lx",I think you need a different format string like PRIx64 here so that the code also works right on a 32-bit system (where long is only 32-bit).> i, n->mergeable_rx_bufs, offset, size, > n->guest_hdr_len, n->host_hdr_len, vdev->guest_features); > exit(1);...> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index 3fee4aa..fbd909d 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -371,8 +371,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > } else { > features.index = ldub_phys(&address_space_memory, > ccw.cda + sizeof(features.features)); > - if (features.index < ARRAY_SIZE(dev->host_features)) { > - features.features = dev->host_features[features.index]; > + if (features.index == 0) { > + features.features = (uint32_t)dev->host_features; > + } else if (features.index == 1) { > + features.features = (uint32_t)(dev->host_features >> 32); > } else { > /* Return zeroes if the guest supports more feature bits. */ > features.features = 0; > @@ -399,8 +401,14 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > features.index = ldub_phys(&address_space_memory, > ccw.cda + sizeof(features.features)); > features.features = ldl_le_phys(&address_space_memory, ccw.cda); > - if (features.index < ARRAY_SIZE(dev->host_features)) { > - virtio_set_features(vdev, features.features); > + if (features.index == 0) { > + virtio_set_features(vdev, > + (vdev->guest_features & 0xffffffff00000000) | > + features.features); > + } else if (features.index == 1) { > + virtio_set_features(vdev, > + (vdev->guest_features & 0x00000000ffffffff) | > + ((uint64_t)features.features << 32));The long constants 0xffffffff00000000 and 0x00000000ffffffff should probably get an ULL suffix. ...> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 5814433..7f74ae5 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -593,6 +593,7 @@ void virtio_reset(void *opaque) > } > > vdev->guest_features = 0; > +Unnecessary white space change?> vdev->queue_sel = 0; > vdev->status = 0; > vdev->isr = 0; > @@ -924,7 +925,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) > qemu_put_8s(f, &vdev->status); > qemu_put_8s(f, &vdev->isr); > qemu_put_be16s(f, &vdev->queue_sel); > - qemu_put_be32s(f, &vdev->guest_features); > + /* XXX features >= 32 */ > + qemu_put_be32s(f, (uint32_t *)&vdev->guest_features);Casting a uint64_t* to a uint32_t* here sounds very wrong - this likely only works on little endian sytems, but certainly not on big-endian systems. If you do not want to extend this for 64-bit right from the beginning, I think you've got to use a temporary 32-bit variable to get this right. ...> @@ -1005,9 +1007,10 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > } > qemu_get_be32s(f, &features); > > + /* XXX features >= 32 */ > if (virtio_set_features(vdev, features) < 0) { > supported_features = k->get_features(qbus->parent); > - error_report("Features 0x%x unsupported. Allowed features: 0x%x", > + error_report("Features 0x%x unsupported. Allowed features: 0x%lx", > features, supported_features);You'll likely need the PRIx64 format here, too.> return -1; > } > diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h > index 070006c..81e5d0b 100644 > --- a/include/hw/qdev-properties.h > +++ b/include/hw/qdev-properties.h > @@ -6,6 +6,7 @@ > /*** qdev-properties.c ***/ > > extern PropertyInfo qdev_prop_bit; > +extern PropertyInfo qdev_prop_bit64; > extern PropertyInfo qdev_prop_bool; > extern PropertyInfo qdev_prop_uint8; > extern PropertyInfo qdev_prop_uint16; > @@ -51,6 +52,17 @@ extern PropertyInfo qdev_prop_arraylen; > .defval = (bool)_defval, \ > } > > +#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) { \ > + .name = (_name), \ > + .info = &(qdev_prop_bit64), \No need for the paranthesis around qdev_prop_bit64 here?> + .bitnr = (_bit), \ > + .offset = offsetof(_state, _field) \ > + + type_check(uint64_t,typeof_field(_state, _field)), \ > + .qtype = QTYPE_QBOOL, \ > + .defval = (bool)_defval, \ > + }Thomas
Thomas Huth
2014-Dec-12 10:55 UTC
[PATCH RFC v6 12/20] virtio: disallow late feature changes for virtio-1
On Thu, 11 Dec 2014 14:25:14 +0100 Cornelia Huck <cornelia.huck at de.ibm.com> wrote:> For virtio-1 devices, the driver must not attempt to set feature bits > after it set FEATURES_OK in the device status. Simply reject it in > that case. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/virtio/virtio.c | 16 ++++++++++++++-- > include/hw/virtio/virtio.h | 2 ++ > 2 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 57190ba..a3dd67b 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -978,7 +978,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) > vmstate_save_state(f, &vmstate_virtio, vdev); > } > > -int virtio_set_features(VirtIODevice *vdev, uint64_t val) > +static int __virtio_set_features(VirtIODevice *vdev, uint64_t val)Maybe avoid the double underscores here? But unfortunately, I also fail to come up with a better suggestion for a name here ...> { > BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > VirtioBusClass *vbusk = VIRTIO_BUS_GET_CLASS(qbus); > @@ -994,6 +994,18 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val) > return bad ? -1 : 0; > } > > +int virtio_set_features(VirtIODevice *vdev, uint64_t val) > +{ > + /* > + * The driver must not attempt to set features after feature negotiation > + * has finished. > + */ > + if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) { > + return -EINVAL; > + }Hmm, according to your patch description, the FEATURES_OK check only applies to virtio-1.0 devices ... so shouldn't there be a check for virtio-1 here? Or did I miss something?> + return __virtio_set_features(vdev, val); > +}Thomas
Michael S. Tsirkin
2014-Dec-16 13:10 UTC
[PATCH RFC v6 17/20] virtio-net: enable virtio 1.0
On Thu, Dec 11, 2014 at 02:25:19PM +0100, Cornelia Huck wrote:> virtio-net (non-vhost) now should have everything in place to support > virtio 1.0: let's enable the feature bit for it. > > Note that VIRTIO_F_VERSION_1 is technically a transport feature; once > every device is ready for virtio 1.0, we can move setting this > feature bit out of the individual devices. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>So to use this with e.g. tun, you need to make tun device LE. I posted a kernel patch 1418732988-3535-1-git-send-email-mst at redhat.com with TUNSETVNETLE/TUNGETVNETLE ioctls to support it. But you still need to call them in qemu, and disable virtio-1.0 if not there.> --- > hw/net/virtio-net.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index 7ee2bd6..b5dd356 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -473,6 +473,7 @@ static uint64_t virtio_net_get_features(VirtIODevice *vdev, uint64_t features) > } > > if (!get_vhost_net(nc->peer)) { > + virtio_add_feature(&features, VIRTIO_F_VERSION_1); > return features; > } > return vhost_net_get_features(get_vhost_net(nc->peer), features); > -- > 1.7.9.5
Michael S. Tsirkin
2014-Dec-28 08:32 UTC
[PATCH RFC v6 18/20] virtio: support revision-specific features
On Thu, Dec 11, 2014 at 02:25:20PM +0100, Cornelia Huck wrote:> Devices may support different sets of feature bits depending on which > revision they're operating at. Let's give the transport a way to > re-query the device about its features when the revision has been > changed. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>So now we have both get_features and get_features_rev, and it's never clear which revision does host_features refer to. IMHO that's just too messy. Let's add get_legacy_features and host_legacy_features instead?> --- > hw/s390x/virtio-ccw.c | 12 ++++++++++-- > hw/virtio/virtio-bus.c | 14 ++++++++++++-- > include/hw/virtio/virtio-bus.h | 3 +++ > include/hw/virtio/virtio.h | 3 +++ > 4 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index a55e851..8b6b2ab 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -699,6 +699,10 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > } > ret = 0; > dev->revision = revinfo.revision; > + /* Re-evaluate which features the device wants to offer. */ > + dev->host_features > + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, > + dev->revision >= 1 ? 1 : 0); > break; > default: > ret = -ENOSYS; > @@ -712,6 +716,9 @@ static void virtio_sch_disable_cb(SubchDev *sch) > VirtioCcwDevice *dev = sch->driver_data; > > dev->revision = -1; > + /* Reset the device's features to legacy. */ > + dev->host_features > + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, 0); > } > > static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) > @@ -853,8 +860,9 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) > virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY); > virtio_add_feature(&dev->host_features, VIRTIO_F_BAD_FEATURE); > > - dev->host_features = virtio_bus_get_vdev_features(&dev->bus, > - dev->host_features); > + /* All devices start in legacy mode. */ > + dev->host_features > + virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features, 0); > > css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid, > parent->hotplugged, 1); > diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c > index 32e3fab..a30826c 100644 > --- a/hw/virtio/virtio-bus.c > +++ b/hw/virtio/virtio-bus.c > @@ -97,18 +97,28 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus) > } > > /* Get the features of the plugged device. */ > -uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, > - uint64_t requested_features) > +uint64_t virtio_bus_get_vdev_features_rev(VirtioBusState *bus, > + uint64_t requested_features, > + unsigned int revision) > { > VirtIODevice *vdev = virtio_bus_get_device(bus); > VirtioDeviceClass *k; > > assert(vdev != NULL); > k = VIRTIO_DEVICE_GET_CLASS(vdev); > + if (revision > 0 && k->get_features_rev) { > + return k->get_features_rev(vdev, requested_features, revision); > + } > assert(k->get_features != NULL); > return k->get_features(vdev, requested_features); > } > > +uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, > + uint64_t requested_features) > +{ > + return virtio_bus_get_vdev_features_rev(bus, requested_features, 0); > +} > + > /* Get bad features of the plugged device. */ > uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) > { > diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h > index 0a4dde1..f0916ef 100644 > --- a/include/hw/virtio/virtio-bus.h > +++ b/include/hw/virtio/virtio-bus.h > @@ -84,6 +84,9 @@ size_t virtio_bus_get_vdev_config_len(VirtioBusState *bus); > /* Get the features of the plugged device. */ > uint64_t virtio_bus_get_vdev_features(VirtioBusState *bus, > uint64_t requested_features); > +uint64_t virtio_bus_get_vdev_features_rev(VirtioBusState *bus, > + uint64_t requested_features, > + unsigned int revision); > /* Get bad features of the plugged device. */ > uint64_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); > /* Get config of the plugged device. */ > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 068211e..1338540 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -147,6 +147,9 @@ typedef struct VirtioDeviceClass { > DeviceRealize realize; > DeviceUnrealize unrealize; > uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features); > + uint64_t (*get_features_rev)(VirtIODevice *vdev, > + uint64_t requested_features, > + unsigned int revision); > uint64_t (*bad_features)(VirtIODevice *vdev); > void (*set_features)(VirtIODevice *vdev, uint64_t val); > int (*validate_features)(VirtIODevice *vdev); > -- > 1.7.9.5
Michael S. Tsirkin
2014-Dec-28 10:24 UTC
[PATCH RFC v6 19/20] virtio-blk: revision specific feature bits
On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote:> Wire up virtio-blk to provide different feature bit sets depending > on whether legacy or v1.0 has been requested. > > Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>So we need some way for devices to tell transports not to negotiate rev 1. Does clearing VERSION_1 have this effect?> --- > hw/block/virtio-blk.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 9cfae66..fdc236a 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -587,6 +587,24 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features) > return features; > } > > +static uint64_t virtio_blk_get_features_rev(VirtIODevice *vdev, > + uint64_t features, > + unsigned int revision) > +{ > + if (revision == 0) { > + /* legacy */ > + virtio_clear_feature(&features, VIRTIO_F_VERSION_1); > + return virtio_blk_get_features(vdev, features); > + } > + /* virtio 1.0 or later */ > + virtio_clear_feature(&features, VIRTIO_BLK_F_SCSI); > + virtio_clear_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); > + virtio_clear_feature(&features, VIRTIO_BLK_F_WCE); > + /* we're still missing ANY_LAYOUT */ > + /* virtio_add_feature(&features, VIRTIO_F_VERSION_1); */ > + return features; > +} > + > static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) > { > VirtIOBlock *s = VIRTIO_BLK(vdev); > @@ -821,6 +839,7 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data) > vdc->get_config = virtio_blk_update_config; > vdc->set_config = virtio_blk_set_config; > vdc->get_features = virtio_blk_get_features; > + vdc->get_features_rev = virtio_blk_get_features_rev; > vdc->set_status = virtio_blk_set_status; > vdc->reset = virtio_blk_reset; > vdc->save = virtio_blk_save_device; > -- > 1.7.9.5
Michael S. Tsirkin
2014-Dec-30 12:25 UTC
[PATCH RFC v6 13/20] virtio: allow to fail setting status
On Thu, Dec 11, 2014 at 02:25:15PM +0100, Cornelia Huck wrote:> virtio-1 allow setting of the FEATURES_OK status bit to fail if > the negotiated feature bits are inconsistent: let's fail > virtio_set_status() in that case and update virtio-ccw to post an > error to the guest. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>Right but a separate validate_features call is awkward. How about we defer virtio_set_features until FEATURES_OK, and teach virtio_set_features that it can fail?> --- > hw/s390x/virtio-ccw.c | 20 ++++++++++++-------- > hw/virtio/virtio.c | 24 +++++++++++++++++++++++- > include/hw/virtio/virtio.h | 3 ++- > 3 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index e09e0da..a55e851 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -555,15 +555,19 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > if (!(status & VIRTIO_CONFIG_S_DRIVER_OK)) { > virtio_ccw_stop_ioeventfd(dev); > } > - virtio_set_status(vdev, status); > - if (vdev->status == 0) { > - virtio_reset(vdev); > - } > - if (status & VIRTIO_CONFIG_S_DRIVER_OK) { > - virtio_ccw_start_ioeventfd(dev); > + if (virtio_set_status(vdev, status) == 0) { > + if (vdev->status == 0) { > + virtio_reset(vdev); > + } > + if (status & VIRTIO_CONFIG_S_DRIVER_OK) { > + virtio_ccw_start_ioeventfd(dev); > + } > + sch->curr_status.scsw.count = ccw.count - sizeof(status); > + ret = 0; > + } else { > + /* Trigger a command reject. */ > + ret = -ENOSYS; > } > - sch->curr_status.scsw.count = ccw.count - sizeof(status); > - ret = 0; > } > break; > case CCW_CMD_SET_IND: > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index a3dd67b..90eedd3 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -543,15 +543,37 @@ void virtio_update_irq(VirtIODevice *vdev) > virtio_notify_vector(vdev, VIRTIO_NO_VECTOR); > } > > -void virtio_set_status(VirtIODevice *vdev, uint8_t val) > +static int virtio_validate_features(VirtIODevice *vdev) > +{ > + VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); > + > + if (k->validate_features) { > + return k->validate_features(vdev); > + } else { > + return 0; > + } > +} > + > +int virtio_set_status(VirtIODevice *vdev, uint8_t val) > { > VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev); > trace_virtio_set_status(vdev, val); > > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + if (!(vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) && > + val & VIRTIO_CONFIG_S_FEATURES_OK) { > + int ret = virtio_validate_features(vdev); > + > + if (ret) { > + return ret; > + } > + } > + } > if (k->set_status) { > k->set_status(vdev, val); > } > vdev->status = val; > + return 0; > } > > bool target_words_bigendian(void); > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index a24e403..068211e 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -149,6 +149,7 @@ typedef struct VirtioDeviceClass { > uint64_t (*get_features)(VirtIODevice *vdev, uint64_t requested_features); > uint64_t (*bad_features)(VirtIODevice *vdev); > void (*set_features)(VirtIODevice *vdev, uint64_t val); > + int (*validate_features)(VirtIODevice *vdev); > void (*get_config)(VirtIODevice *vdev, uint8_t *config); > void (*set_config)(VirtIODevice *vdev, const uint8_t *config); > void (*reset)(VirtIODevice *vdev); > @@ -233,7 +234,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align); > void virtio_queue_notify(VirtIODevice *vdev, int n); > uint16_t virtio_queue_vector(VirtIODevice *vdev, int n); > void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector); > -void virtio_set_status(VirtIODevice *vdev, uint8_t val); > +int virtio_set_status(VirtIODevice *vdev, uint8_t val); > void virtio_reset(void *opaque); > void virtio_update_irq(VirtIODevice *vdev); > int virtio_set_features(VirtIODevice *vdev, uint64_t val); > -- > 1.7.9.5
Stefan Hajnoczi
2015-Jan-20 10:29 UTC
[PATCH RFC v6 06/20] virtio: endianness checks for virtio 1.0 devices
On Thu, Dec 11, 2014 at 02:25:08PM +0100, Cornelia Huck wrote:> Add code that checks for the VERSION_1 feature bit in order to make > decisions about the device's endianness. This allows us to support > transitional devices. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/virtio/virtio.c | 6 +++++- > include/hw/virtio/virtio-access.h | 4 ++++ > include/hw/virtio/virtio.h | 8 ++++++-- > 3 files changed, 15 insertions(+), 3 deletions(-)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/f624b353/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 10:43 UTC
[PATCH RFC v6 08/20] dataplane: allow virtio-1 devices
On Thu, Dec 11, 2014 at 02:25:10PM +0100, Cornelia Huck wrote:> Handle endianness conversion for virtio-1 virtqueues correctly. > > Note that dataplane now needs to be built per-target. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/block/dataplane/virtio-blk.c | 4 +- > hw/scsi/virtio-scsi-dataplane.c | 2 +- > hw/virtio/Makefile.objs | 2 +- > hw/virtio/dataplane/Makefile.objs | 2 +- > hw/virtio/dataplane/vring.c | 86 ++++++++++++++----------- > include/hw/virtio/dataplane/vring-accessors.h | 75 +++++++++++++++++++++ > include/hw/virtio/dataplane/vring.h | 14 +--- > 7 files changed, 131 insertions(+), 54 deletions(-) > create mode 100644 include/hw/virtio/dataplane/vring-accessors.hThis patch is independent of VIRTIO 1.0 and can be merged separately (faster).> diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index 1222a37..2d8cc15 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -16,7 +16,9 @@ > #include "qemu/iov.h" > #include "qemu/thread.h" > #include "qemu/error-report.h" > +#include "hw/virtio/virtio-access.h" > #include "hw/virtio/dataplane/vring.h" > +#include "hw/virtio/dataplane/vring-accessors.h"I like your vring-accessors.h approach better than the inline virtio_ld/st_p() in my patch. Nice.> @@ -154,15 +157,18 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) > } > > > -static int get_desc(Vring *vring, VirtQueueElement *elem, > +static int get_desc(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, > struct vring_desc *desc)Since we copy in struct vring_desc anyway, it's cleaner to byteswap the fields once instead of remembering to do it each time we need to access a field. The copy_in_vring_desc() function is one thing I prefer I about my patch. -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/3b9632ea/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 10:50 UTC
[PATCH RFC v6 09/20] s390x/css: Add a callback for when subchannel gets disabled
On Thu, Dec 11, 2014 at 02:25:11PM +0100, Cornelia Huck wrote:> From: Thomas Huth <thuth at linux.vnet.ibm.com> > > We need a possibility to run code when a subchannel gets disabled. > This patch adds the necessary infrastructure. > > Signed-off-by: Thomas Huth <thuth at linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/s390x/css.c | 12 ++++++++++++ > hw/s390x/css.h | 1 + > 2 files changed, 13 insertions(+)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/8c4cfda1/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 11:00 UTC
[PATCH RFC v6 10/20] s390x/virtio-ccw: add virtio set-revision call
On Thu, Dec 11, 2014 at 02:25:12PM +0100, Cornelia Huck wrote:> From: Thomas Huth <thuth at linux.vnet.ibm.com> > > Handle the virtio-ccw revision according to what the guest sets. > When revision 1 is selected, we have a virtio-1 standard device > with byteswapping for the virtio rings. > > When a channel gets disabled, we have to revert to the legacy behavior > in case the next user of the device does not negotiate the revision 1 > anymore (e.g. the boot firmware uses revision 1, but the operating > system only uses the legacy mode). > > Note that revisions > 0 are still disabled. > > Signed-off-by: Thomas Huth <thuth at linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/s390x/virtio-ccw.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ > hw/s390x/virtio-ccw.h | 8 ++++++++ > 2 files changed, 60 insertions(+)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/ff94eba6/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 11:06 UTC
[PATCH RFC v6 11/20] s390x/virtio-ccw: support virtio-1 set_vq format
On Thu, Dec 11, 2014 at 02:25:13PM +0100, Cornelia Huck wrote:> Support the new CCW_CMD_SET_VQ format for virtio-1 devices. > > While we're at it, refactor the code a bit and enforce big endian > fields (which had always been required, even for legacy). > > Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/s390x/virtio-ccw.c | 114 ++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 80 insertions(+), 34 deletions(-)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/5f212cfa/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 11:08 UTC
[PATCH RFC v6 10/20] s390x/virtio-ccw: add virtio set-revision call
On Thu, Dec 11, 2014 at 02:25:12PM +0100, Cornelia Huck wrote:> @@ -608,6 +631,25 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > } > } > break; > + case CCW_CMD_SET_VIRTIO_REV: > + len = sizeof(revinfo); > + if (ccw.count < len || (check_len && ccw.count > len)) { > + ret = -EINVAL; > + break; > + } > + if (!ccw.cda) { > + ret = -EFAULT; > + break; > + } > + cpu_physical_memory_read(ccw.cda, &revinfo, len); > + if (dev->revision >= 0 || > + revinfo.revision > virtio_ccw_rev_max(dev)) {In the next patch virtio_ccw_handle_set_vq() uses big-endian memory access functions to load a struct from guest memory. Here you just copy the struct in without byteswaps. Are the byteswaps missing here? (I guess this normally runs big-endian guests on big-endian hosts so it's not noticable.) Stefan -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/f898dff7/attachment-0001.sig>
Stefan Hajnoczi
2015-Jan-20 11:15 UTC
[PATCH RFC v6 14/20] s390x/virtio-ccw: enable virtio 1.0
On Thu, Dec 11, 2014 at 02:25:16PM +0100, Cornelia Huck wrote:> virtio-ccw should now have everything in place to operate virtio 1.0 > devices, so let's enable revision 1. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/s390x/virtio-ccw.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-)Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/28fc6b34/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 11:19 UTC
[PATCH RFC v6 15/20] virtio-net: no writeable mac for virtio-1
On Thu, Dec 11, 2014 at 02:25:17PM +0100, Cornelia Huck wrote:> Devices operating as virtio 1.0 may not allow writes to the mac > address in config space. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/net/virtio-net.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index d6d1b98..ebbea60 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -87,6 +87,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) > memcpy(&netcfg, config, n->config_size); > > if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR) &&I don't see VIRTIO_NET_F_CTRL_MAC_ADDR (23) in the VIRTIO 1.0 "5.1.3.1 Legacy Interface: Feature bits" section. Should it be there just so people don't try to reuse bit 23 in the future? The patch itself: Reviewed-by: Stefan Hajnoczi <stefanha at redhat.com> -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/59b9a1e2/attachment.sig>
Stefan Hajnoczi
2015-Jan-20 13:18 UTC
[PATCH RFC v6 16/20] virtio-net: support longer header
On Thu, Dec 11, 2014 at 02:25:18PM +0100, Cornelia Huck wrote:> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index ebbea60..7ee2bd6 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -373,15 +373,21 @@ static int peer_has_ufo(VirtIONet *n) > return n->has_ufo; > } > > -static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs) > +static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, > + int version_1)Please use bool, it makes it 100% clear what the meaning of "version_1" is. s/int/bool/ -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150120/c43ad5eb/attachment.sig>
David Gibson
2015-Jan-22 01:24 UTC
[Qemu-devel] [PATCH RFC v6 02/20] virtio: cull virtio_bus_set_vdev_features
On Thu, Dec 11, 2014 at 02:25:04PM +0100, Cornelia Huck wrote:> The only user of this function was virtio-ccw, and it should use > virtio_set_features() like everybody else: We need to make sure > that bad features are masked out properly, which this function did > not do. > > Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>Reviewed-by: David Gibson <david at gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/e628cc2e/attachment-0001.sig>
David Gibson
2015-Jan-22 01:25 UTC
[Qemu-devel] [PATCH RFC v6 03/20] virtio: feature bit manipulation helpers
On Thu, Dec 11, 2014 at 02:25:05PM +0100, Cornelia Huck wrote:> Add virtio_{add,clear}_feature helper functions for manipulating a > feature bits variable. This has some benefits over open coding: > - add check that the bit is in a sane range > - make it obvious at a glance what is going on > - have a central point to change when we want to extend feature bits > > Convert existing code manipulating features to use the new helpers. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>Reviewed-by: David Gibson <david at gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/759d305d/attachment-0001.sig>
David Gibson
2015-Jan-22 01:28 UTC
[Qemu-devel] [PATCH RFC v6 04/20] virtio: add feature checking helpers
On Thu, Dec 11, 2014 at 02:25:06PM +0100, Cornelia Huck wrote:> Add a helper function for checking whether a bit is set in the guest > features for a vdev as well as one that works on a feature bit set. > > Convert code that open-coded this: It cleans up the code and makes it > easier to extend the guest feature bits. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>Reviewed-by: David Gibson <david at gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/0d3f2b83/attachment.sig>
David Gibson
2015-Jan-22 01:43 UTC
[Qemu-devel] [PATCH RFC v6 05/20] virtio: support more feature bits
On Thu, Dec 11, 2014 at 02:25:07PM +0100, Cornelia Huck wrote:> With virtio-1, we support more than 32 feature bits. Let's extend both > host and guest features to 64, which should suffice for a while. > > vhost and migration have been ignored for now.[snip]> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index f6c0379..08141c7 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -55,6 +55,12 @@ > /* A guest should never accept this. It implies negotiation is broken. */ > #define VIRTIO_F_BAD_FEATURE 30 > > +/* v1.0 compliant. */ > +#define VIRTIO_F_VERSION_1 32This is already in the kernel header, isn't it? -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/177edc02/attachment.sig>
David Gibson
2015-Jan-22 01:54 UTC
[Qemu-devel] [PATCH RFC v6 06/20] virtio: endianness checks for virtio 1.0 devices
On Thu, Dec 11, 2014 at 02:25:08PM +0100, Cornelia Huck wrote:> Add code that checks for the VERSION_1 feature bit in order to make > decisions about the device's endianness. This allows us to support > transitional devices. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/virtio/virtio.c | 6 +++++- > include/hw/virtio/virtio-access.h | 4 ++++ > include/hw/virtio/virtio.h | 8 ++++++-- > 3 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 7f74ae5..8f69ffa 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -881,7 +881,11 @@ static bool virtio_device_endian_needed(void *opaque) > VirtIODevice *vdev = opaque; > > assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); > - return vdev->device_endian != virtio_default_endian(); > + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + return vdev->device_endian != virtio_default_endian(); > + } > + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ > + return vdev->device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE;This doesn't seem quite right. Since virtio 1.0 is always LE, this should just assert that device_endian == LE and return false, right?> } > > static const VMStateDescription vmstate_virtio_device_endian = { > diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h > index 46456fd..ee28c21 100644 > --- a/include/hw/virtio/virtio-access.h > +++ b/include/hw/virtio/virtio-access.h > @@ -19,6 +19,10 @@ > > static inline bool virtio_access_is_big_endian(VirtIODevice *vdev) > { > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ > + return false; > + } > #if defined(TARGET_IS_BIENDIAN) > return virtio_is_big_endian(vdev); > #elif defined(TARGET_WORDS_BIGENDIAN) > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 08141c7..68c40db 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -297,7 +297,11 @@ static inline bool virtio_has_feature(VirtIODevice *vdev, unsigned int fbit) > > static inline bool virtio_is_big_endian(VirtIODevice *vdev) > { > - assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); > - return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG; > + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); > + return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG; > + } > + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ > + return false; > } > #endifAFAICT, the only real difference between virtio_is_big_endian() and virtio_access_is_big_endian() is that the latter will become compile-time constant on targets that don't do bi-endian. With virtio 1.0 support, that's no longer true, so those two macros should just be merged, I think. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/ccea1a08/attachment-0001.sig>
David Gibson
2015-Jan-22 02:06 UTC
[Qemu-devel] [PATCH RFC v6 07/20] virtio: allow virtio-1 queue layout
On Thu, Dec 11, 2014 at 02:25:09PM +0100, Cornelia Huck wrote:> For virtio-1 devices, we allow a more complex queue layout that doesn't > require descriptor table and rings on a physically-contigous memory area: > add virtio_queue_set_rings() to allow transports to set this up. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > --- > hw/virtio/virtio-mmio.c | 3 +++ > hw/virtio/virtio.c | 53 ++++++++++++++++++++++++++++---------------- > include/hw/virtio/virtio.h | 3 +++ > 3 files changed, 40 insertions(+), 19 deletions(-) > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c > index 43b7e02..0c9b63b 100644 > --- a/hw/virtio/virtio-mmio.c > +++ b/hw/virtio/virtio-mmio.c > @@ -244,8 +244,11 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value, > case VIRTIO_MMIO_QUEUENUM: > DPRINTF("mmio_queue write %d max %d\n", (int)value, VIRTQUEUE_MAX_SIZE); > virtio_queue_set_num(vdev, vdev->queue_sel, value); > + /* Note: only call this function for legacy devices */It's not clear to me if this is an assertion that this *does* only call the function for legacy devices or a fixme, that it *should* only call the function for legacy devices.> + virtio_queue_update_rings(vdev, vdev->queue_sel); > break; > case VIRTIO_MMIO_QUEUEALIGN: > + /* Note: this is only valid for legacy devices */ > virtio_queue_set_align(vdev, vdev->queue_sel, value); > break; > case VIRTIO_MMIO_QUEUEPFN: > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 8f69ffa..57190ba 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -69,7 +69,6 @@ typedef struct VRing > struct VirtQueue > { > VRing vring; > - hwaddr pa; > uint16_t last_avail_idx; > /* Last used index value we have signalled on */ > uint16_t signalled_used; > @@ -92,15 +91,18 @@ struct VirtQueue > }; > > /* virt queue functions */ > -static void virtqueue_init(VirtQueue *vq) > +void virtio_queue_update_rings(VirtIODevice *vdev, int n)Perhaps something in the name to emphasise that this is only for <v1.0 devices?> { > - hwaddr pa = vq->pa; > + VRing *vring = &vdev->vq[n].vring; > > - vq->vring.desc = pa; > - vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc); > - vq->vring.used = vring_align(vq->vring.avail + > - offsetof(VRingAvail, ring[vq->vring.num]), > - vq->vring.align); > + if (!vring->desc) { > + /* not yet setup -> nothing to do */ > + return; > + } > + vring->avail = vring->desc + vring->num * sizeof(VRingDesc); > + vring->used = vring_align(vring->avail + > + offsetof(VRingAvail, ring[vring->num]), > + vring->align);Would it make sense to implement this in terms of virtio_queue_set_rings()?> } > > static inline uint64_t vring_desc_addr(VirtIODevice *vdev, hwaddr desc_pa, > @@ -605,7 +607,6 @@ void virtio_reset(void *opaque) > vdev->vq[i].vring.avail = 0; > vdev->vq[i].vring.used = 0; > vdev->vq[i].last_avail_idx = 0; > - vdev->vq[i].pa = 0; > vdev->vq[i].vector = VIRTIO_NO_VECTOR; > vdev->vq[i].signalled_used = 0; > vdev->vq[i].signalled_used_valid = false; > @@ -708,13 +709,21 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) > > void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr) > { > - vdev->vq[n].pa = addr; > - virtqueue_init(&vdev->vq[n]); > + vdev->vq[n].vring.desc = addr; > + virtio_queue_update_rings(vdev, n); > } > > hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n) > { > - return vdev->vq[n].pa; > + return vdev->vq[n].vring.desc; > +} > + > +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, > + hwaddr avail, hwaddr used) > +{ > + vdev->vq[n].vring.desc = desc; > + vdev->vq[n].vring.avail = avail; > + vdev->vq[n].vring.used = used; > } > > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) > @@ -728,7 +737,6 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num) > return; > } > vdev->vq[n].vring.num = num; > - virtqueue_init(&vdev->vq[n]); > } > > int virtio_queue_get_num(VirtIODevice *vdev, int n) > @@ -748,6 +756,11 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) > BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); > VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); > > + /* virtio-1 compliant devices cannot change the aligment */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + error_report("tried to modify queue alignment for virtio-1 device"); > + return; > + } > /* Check that the transport told us it was going to do this > * (so a buggy transport will immediately assert rather than > * silently failing to migrate this state) > @@ -755,7 +768,7 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align) > assert(k->has_variable_vring_alignment); > > vdev->vq[n].vring.align = align; > - virtqueue_init(&vdev->vq[n]); > + virtio_queue_update_rings(vdev, n); > } > > void virtio_queue_notify_vq(VirtQueue *vq) > @@ -949,7 +962,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) > if (k->has_variable_vring_alignment) { > qemu_put_be32(f, vdev->vq[i].vring.align); > } > - qemu_put_be64(f, vdev->vq[i].pa); > + /* XXX virtio-1 devices */ > + qemu_put_be64(f, vdev->vq[i].vring.desc); > qemu_put_be16s(f, &vdev->vq[i].last_avail_idx); > if (k->save_queue) { > k->save_queue(qbus->parent, i, f); > @@ -1044,13 +1058,14 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > if (k->has_variable_vring_alignment) { > vdev->vq[i].vring.align = qemu_get_be32(f); > } > - vdev->vq[i].pa = qemu_get_be64(f); > + vdev->vq[i].vring.desc = qemu_get_be64(f); > qemu_get_be16s(f, &vdev->vq[i].last_avail_idx); > vdev->vq[i].signalled_used_valid = false; > vdev->vq[i].notification = true; > > - if (vdev->vq[i].pa) { > - virtqueue_init(&vdev->vq[i]); > + if (vdev->vq[i].vring.desc) { > + /* XXX virtio-1 devices */ > + virtio_queue_update_rings(vdev, i); > } else if (vdev->vq[i].last_avail_idx) { > error_report("VQ %d address 0x0 " > "inconsistent with Host index 0x%x", > @@ -1084,7 +1099,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) > } > > for (i = 0; i < num; i++) { > - if (vdev->vq[i].pa) { > + if (vdev->vq[i].vring.desc) { > uint16_t nheads; > nheads = vring_avail_idx(&vdev->vq[i]) - vdev->vq[i].last_avail_idx; > /* Check it isn't doing strange things with descriptor numbers. */ > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 68c40db..b63ced3 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -224,6 +224,9 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr); > hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n); > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num); > int virtio_queue_get_num(VirtIODevice *vdev, int n); > +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, > + hwaddr avail, hwaddr used); > +void virtio_queue_update_rings(VirtIODevice *vdev, int n); > void virtio_queue_set_align(VirtIODevice *vdev, int n, int align); > void virtio_queue_notify(VirtIODevice *vdev, int n); > uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/231dcc8e/attachment-0001.sig>
David Gibson
2015-Jan-22 02:12 UTC
[Qemu-devel] [PATCH RFC v6 08/20] dataplane: allow virtio-1 devices
On Thu, Dec 11, 2014 at 02:25:10PM +0100, Cornelia Huck wrote:> Handle endianness conversion for virtio-1 virtqueues correctly. > > Note that dataplane now needs to be built per-target. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>Reviewed-by: David Gibson <david at gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/c696e8c0/attachment.sig>
David Gibson
2015-Jan-22 02:15 UTC
[Qemu-devel] [PATCH RFC v6 12/20] virtio: disallow late feature changes for virtio-1
On Thu, Dec 11, 2014 at 02:25:14PM +0100, Cornelia Huck wrote:> For virtio-1 devices, the driver must not attempt to set feature bits > after it set FEATURES_OK in the device status. Simply reject it in > that case. > > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com>Reviewed-by: David Gibson <david at gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20150122/76ee814b/attachment-0001.sig>
Reasonably Related Threads
- [PATCH RFC v6 05/20] virtio: support more feature bits
- [PATCH RFC v5 05/19] virtio: support more feature bits
- [PATCH RFC v5 05/19] virtio: support more feature bits
- [PATCH RFC v4 03/16] virtio: support more feature bits
- [PATCH RFC v4 03/16] virtio: support more feature bits