Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 00/16] qemu: towards virtio-1 host support
Yet another version of the virtio-1 support patches. This one has seen some (very) light testing with the virtio-1 guest support patches currently on vhost-next. Changes from v3: - Add support for FEATURES_OK. We refuse to set features after the driver has set this in the status field, and we allow to fail setting the status if the features are inconsistent. - Add missing virtio-1 changes for virtio-net (header size and mac). - Dropped setting the VERSION_1 bit for virtio-blk: There's still some stuff missing. For virtio-blk, we need to validate the feature bits if version 1 is negotiated: some legacy features are not allowed in that case. I'm not quite sure how to handle this, though. We could use the new validate_features callback to verify that the driver negotiated a sensible feature set, but that would require us to offer a superset of legacy and version 1 bits, which feels wrong. Any ideas? Cornelia Huck (13): virtio: cull virtio_bus_set_vdev_features virtio: support more feature bits s390x/virtio-ccw: fix check for WRITE_FEAT virtio: introduce legacy virtio 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 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 | 7 +- hw/block/dataplane/virtio-blk.c | 4 +- hw/block/virtio-blk.c | 9 +- hw/char/virtio-serial-bus.c | 9 +- hw/net/virtio-net.c | 52 +++++-- hw/s390x/css.c | 12 ++ hw/s390x/css.h | 1 + hw/s390x/s390-virtio-bus.c | 9 +- hw/s390x/virtio-ccw.c | 206 +++++++++++++++++++------ hw/s390x/virtio-ccw.h | 7 +- hw/scsi/vhost-scsi.c | 7 +- hw/scsi/virtio-scsi-dataplane.c | 2 +- hw/scsi/virtio-scsi.c | 10 +- hw/virtio/Makefile.objs | 2 +- hw/virtio/dataplane/Makefile.objs | 2 +- hw/virtio/dataplane/vring.c | 96 ++++++------ hw/virtio/virtio-balloon.c | 8 +- hw/virtio/virtio-bus.c | 23 +-- hw/virtio/virtio-mmio.c | 9 +- hw/virtio/virtio-pci.c | 13 +- hw/virtio/virtio-rng.c | 2 +- hw/virtio/virtio.c | 88 +++++++++-- include/hw/virtio/dataplane/vring-accessors.h | 75 +++++++++ include/hw/virtio/dataplane/vring.h | 14 +- include/hw/virtio/virtio-access.h | 4 + include/hw/virtio/virtio-bus.h | 10 +- include/hw/virtio/virtio.h | 39 ++++- linux-headers/linux/virtio_config.h | 3 + 28 files changed, 523 insertions(+), 200 deletions(-) create mode 100644 include/hw/virtio/dataplane/vring-accessors.h -- 1.7.9.5
Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 01/16] 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-Nov-27 15:16 UTC
[PATCH RFC v4 02/16] 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-Nov-27 15:16 UTC
[PATCH RFC v4 03/16] virtio: support more feature bits
With virtio-1, we support more than 32 feature bits. Let's make vdev->guest_features depend on the number of supported feature bits, allowing us to grow the feature bits automatically. We also need to enhance the internal functions dealing with getting and setting features with an additional index field, so that all feature bits may be accessed (in chunks of 32 bits). vhost and migration have been ignored for now. Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> --- hw/9pfs/virtio-9p-device.c | 7 ++++++- hw/block/virtio-blk.c | 9 +++++++-- hw/char/virtio-serial-bus.c | 9 +++++++-- hw/net/virtio-net.c | 38 ++++++++++++++++++++++++++------------ hw/s390x/s390-virtio-bus.c | 9 +++++---- hw/s390x/virtio-ccw.c | 17 ++++++++++------- hw/scsi/vhost-scsi.c | 7 +++++-- hw/scsi/virtio-scsi.c | 10 +++++----- hw/virtio/dataplane/vring.c | 10 +++++----- hw/virtio/virtio-balloon.c | 8 ++++++-- hw/virtio/virtio-bus.c | 9 +++++---- hw/virtio/virtio-mmio.c | 9 +++++---- hw/virtio/virtio-pci.c | 13 +++++++------ hw/virtio/virtio-rng.c | 2 +- hw/virtio/virtio.c | 29 +++++++++++++++++------------ include/hw/virtio/virtio-bus.h | 7 ++++--- include/hw/virtio/virtio.h | 19 ++++++++++++++----- 17 files changed, 135 insertions(+), 77 deletions(-) diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c index 2572747..c29c8c8 100644 --- a/hw/9pfs/virtio-9p-device.c +++ b/hw/9pfs/virtio-9p-device.c @@ -21,8 +21,13 @@ #include "virtio-9p-coth.h" #include "hw/virtio/virtio-access.h" -static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features) +static uint32_t virtio_9p_get_features(VirtIODevice *vdev, unsigned int index, + uint32_t features) { + if (index > 0) { + return features; + } + features |= 1 << VIRTIO_9P_MOUNT_TAG; return features; } diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index b19b102..6d86f60 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -564,10 +564,15 @@ 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 uint32_t virtio_blk_get_features(VirtIODevice *vdev, unsigned int index, + uint32_t features) { VirtIOBlock *s = VIRTIO_BLK(vdev); + if (index > 0) { + return features; + } + features |= (1 << VIRTIO_BLK_F_SEG_MAX); features |= (1 << VIRTIO_BLK_F_GEOMETRY); features |= (1 << VIRTIO_BLK_F_TOPOLOGY); @@ -601,7 +606,7 @@ static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) return; } - features = vdev->guest_features; + features = vdev->guest_features[0]; /* A guest that supports VIRTIO_BLK_F_CONFIG_WCE must be able to send * cache flushes. Thus, the "auto writethrough" behavior is never diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c index a7b1b68..55de504 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 vdev->guest_features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT); } static size_t write_to_port(VirtIOSerialPort *port, @@ -467,10 +467,15 @@ static void handle_input(VirtIODevice *vdev, VirtQueue *vq) { } -static uint32_t get_features(VirtIODevice *vdev, uint32_t features) +static uint32_t get_features(VirtIODevice *vdev, unsigned int index, + uint32_t features) { VirtIOSerial *vser; + if (index > 0) { + return features; + } + vser = VIRTIO_SERIAL(vdev); if (vser->bus.max_nr_ports > 1) { diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 9b88775..1e214b5 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 (!(vdev->guest_features[0] >> VIRTIO_NET_F_CTRL_MAC_ADDR & 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); @@ -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 (!((1 << VIRTIO_NET_F_CTRL_VLAN) & vdev->guest_features[0])) { info->vlan = RX_STATE_ALL; } else if (!info->vlan_table) { info->vlan = RX_STATE_NONE; @@ -441,11 +441,16 @@ 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 uint32_t virtio_net_get_features(VirtIODevice *vdev, unsigned int index, + uint32_t features) { VirtIONet *n = VIRTIO_NET(vdev); NetClientState *nc = qemu_get_queue(n->nic); + if (index > 0) { + return features; + } + features |= (1 << VIRTIO_NET_F_MAC); if (!peer_has_vnet_hdr(n)) { @@ -471,10 +476,14 @@ 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 uint32_t virtio_net_bad_features(VirtIODevice *vdev, unsigned int index) { uint32_t features = 0; + if (index > 0) { + return 0; + } + /* Linux kernel 2.6.25. It understood MAC (as everyone must), * but also these: */ features |= (1 << VIRTIO_NET_F_MAC); @@ -511,14 +520,19 @@ static uint64_t virtio_net_guest_offloads_by_features(uint32_t features) static inline uint64_t virtio_net_supported_guest_offloads(VirtIONet *n) { VirtIODevice *vdev = VIRTIO_DEVICE(n); - return virtio_net_guest_offloads_by_features(vdev->guest_features); + return virtio_net_guest_offloads_by_features(vdev->guest_features[0]); } -static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) +static void virtio_net_set_features(VirtIODevice *vdev, unsigned int index, + uint32_t features) { VirtIONet *n = VIRTIO_NET(vdev); int i; + if (index > 0) { + return; + } + virtio_net_set_multiqueue(n, !!(features & (1 << VIRTIO_NET_F_MQ))); virtio_net_set_mrg_rx_bufs(n, !!(features & (1 << VIRTIO_NET_F_MRG_RXBUF))); @@ -585,7 +599,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 (!((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features[0])) { return VIRTIO_NET_ERR; } @@ -1034,7 +1048,7 @@ static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, size_t "i %zd mergeable %d offset %zd, size %zd, " "guest hdr len %zd, host hdr len %zd guest features 0x%x", i, n->mergeable_rx_bufs, offset, size, - n->guest_hdr_len, n->host_hdr_len, vdev->guest_features); + n->guest_hdr_len, n->host_hdr_len, vdev->guest_features[0]); exit(1); } @@ -1377,7 +1391,7 @@ static void virtio_net_save_device(VirtIODevice *vdev, QEMUFile *f) } } - if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features) { + if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features[0]) { qemu_put_be64(f, n->curr_guest_offloads); } } @@ -1485,7 +1499,7 @@ static int virtio_net_load_device(VirtIODevice *vdev, QEMUFile *f, } } - if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features) { + if ((1 << VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) & vdev->guest_features[0]) { n->curr_guest_offloads = qemu_get_be64(f); } else { n->curr_guest_offloads = virtio_net_supported_guest_offloads(n); @@ -1512,8 +1526,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 (vdev->guest_features[0] & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE) && + vdev->guest_features[0] & (0x1 << 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/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 39dc201..b5cc131 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -128,7 +128,7 @@ static int s390_virtio_device_init(VirtIOS390Device *dev, VirtIODevice *vdev) bus->dev_offs += dev_len; - dev->host_features = virtio_bus_get_vdev_features(&dev->bus, + dev->host_features = virtio_bus_get_vdev_features(&dev->bus, 0, dev->host_features); s390_virtio_device_sync(dev); s390_virtio_reset_idx(dev); @@ -419,7 +419,7 @@ void s390_virtio_device_update_status(VirtIOS390Device *dev) /* Update guest supported feature bitmap */ features = bswap32(ldl_be_phys(&address_space_memory, dev->feat_offs)); - virtio_set_features(vdev, features); + virtio_set_features(vdev, 0, features); } VirtIOS390Device *s390_virtio_bus_console(VirtIOS390Bus *bus) @@ -490,10 +490,11 @@ static void virtio_s390_notify(DeviceState *d, uint16_t vector) s390_virtio_irq(0, token); } -static unsigned virtio_s390_get_features(DeviceState *d) +static unsigned virtio_s390_get_features(DeviceState *d, unsigned int index) { VirtIOS390Device *dev = to_virtio_s390_device(d); - return dev->host_features; + + return index == 0 ? dev->host_features : 0; } /**************** S390 Virtio Bus Device Descriptions *******************/ diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 84f17bc..2f52b82 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -400,7 +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_set_features(vdev, features.features); + virtio_set_features(vdev, features.index, features.features); } else { /* * If the guest supports more feature bits, assert that it @@ -619,6 +619,7 @@ static int virtio_ccw_device_init(VirtioCcwDevice *dev, VirtIODevice *vdev) int ret; int num; DeviceState *parent = DEVICE(dev); + int i; sch = g_malloc0(sizeof(SubchDev)); @@ -739,9 +740,11 @@ 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. */ + for (i = 0; i < ARRAY_SIZE(dev->host_features); i++) { + dev->host_features[i] + virtio_bus_get_vdev_features(&dev->bus, i, dev->host_features[i]); + } dev->host_features[0] |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; dev->host_features[0] |= 0x1 << VIRTIO_F_BAD_FEATURE; @@ -1063,12 +1066,12 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector) } } -static unsigned virtio_ccw_get_features(DeviceState *d) +static unsigned virtio_ccw_get_features(DeviceState *d, unsigned int index) { VirtioCcwDevice *dev = VIRTIO_CCW_DEVICE(d); - /* Only the first 32 feature bits are used. */ - return dev->host_features[0]; + return index < ARRAY_SIZE(dev->host_features) ? + dev->host_features[index] : 0; } static void virtio_ccw_reset(DeviceState *d) diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 308b393..8e1afa0 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -92,7 +92,7 @@ static int vhost_scsi_start(VHostSCSI *s) return ret; } - s->dev.acked_features = vdev->guest_features; + s->dev.acked_features = vdev->guest_features[0]; ret = vhost_dev_start(&s->dev, vdev); if (ret < 0) { error_report("Error start vhost dev"); @@ -150,11 +150,14 @@ static void vhost_scsi_stop(VHostSCSI *s) vhost_dev_disable_notifiers(&s->dev, vdev); } -static uint32_t vhost_scsi_get_features(VirtIODevice *vdev, +static uint32_t vhost_scsi_get_features(VirtIODevice *vdev, unsigned int index, uint32_t features) { VHostSCSI *s = VHOST_SCSI(vdev); + if (index > 0) { + return features; + } return vhost_get_features(&s->dev, kernel_feature_bits, features); } diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index ef48550..189c59a 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 ((vdev->guest_features[0] & VIRTIO_F_ANY_LAYOUT) == 0) { req_size = req->elem.out_sg[0].iov_len; resp_size = req->elem.in_sg[0].iov_len; } @@ -627,7 +627,7 @@ 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, +static uint32_t virtio_scsi_get_features(VirtIODevice *vdev, unsigned int index, uint32_t requested_features) { return requested_features; @@ -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 (((vdev->guest_features[0] >> VIRTIO_SCSI_F_CHANGE) & 1) && 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 ((vdev->guest_features[0] >> VIRTIO_SCSI_F_HOTPLUG) & 1) { 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 ((vdev->guest_features[0] >> VIRTIO_SCSI_F_HOTPLUG) & 1) { 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..a051775 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 (!(vdev->guest_features[0] & (1 << 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 (vdev->guest_features[0] & (1 << 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 ((vdev->guest_features[0] & 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 (!(vdev->guest_features[0] & 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 (vdev->guest_features[0] & (1 << 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..c5b7a4d 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 vdev->guest_features[0] & (1 << VIRTIO_BALLOON_F_STATS_VQ); } static bool balloon_stats_enabled(const VirtIOBalloon *s) @@ -303,8 +303,12 @@ static void virtio_balloon_set_config(VirtIODevice *vdev, } } -static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f) +static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, + unsigned int index, uint32_t f) { + if (index > 0) { + return 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..503114d 100644 --- a/hw/virtio/virtio-bus.c +++ b/hw/virtio/virtio-bus.c @@ -97,7 +97,7 @@ 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 virtio_bus_get_vdev_features(VirtioBusState *bus, unsigned int index, uint32_t requested_features) { VirtIODevice *vdev = virtio_bus_get_device(bus); @@ -106,11 +106,12 @@ uint32_t virtio_bus_get_vdev_features(VirtioBusState *bus, assert(vdev != NULL); k = VIRTIO_DEVICE_GET_CLASS(vdev); assert(k->get_features != NULL); - return k->get_features(vdev, requested_features); + return k->get_features(vdev, index, requested_features); } /* Get bad features of the plugged device. */ -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) +uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus, + unsigned int index) { VirtIODevice *vdev = virtio_bus_get_device(bus); VirtioDeviceClass *k; @@ -118,7 +119,7 @@ uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus) assert(vdev != NULL); k = VIRTIO_DEVICE_GET_CLASS(vdev); if (k->bad_features != NULL) { - return k->bad_features(vdev); + return k->bad_features(vdev, index); } else { return 0; } diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c index 2450c13..e83bb47 100644 --- a/hw/virtio/virtio-mmio.c +++ b/hw/virtio/virtio-mmio.c @@ -222,7 +222,7 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value, break; case VIRTIO_MMIO_GUESTFEATURES: if (!proxy->guest_features_sel) { - virtio_set_features(vdev, value); + virtio_set_features(vdev, 0, value); } break; case VIRTIO_MMIO_GUESTFEATURESSEL: @@ -306,11 +306,12 @@ 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 unsigned int virtio_mmio_get_features(DeviceState *opaque, + unsigned int index) { VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); - return proxy->host_features; + return index == 0 ? proxy->host_features : 0; } static int virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f) @@ -350,7 +351,7 @@ static void virtio_mmio_device_plugged(DeviceState *opaque) VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque); proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY); - proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, + proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus, 0, proxy->host_features); } diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index dde1d73..00eeeb6 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -272,9 +272,9 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val) case VIRTIO_PCI_GUEST_FEATURES: /* Guest does not negotiate properly? We have to assume nothing. */ if (val & (1 << VIRTIO_F_BAD_FEATURE)) { - val = virtio_bus_get_vdev_bad_features(&proxy->bus); + val = virtio_bus_get_vdev_bad_features(&proxy->bus, 0); } - virtio_set_features(vdev, val); + virtio_set_features(vdev, 0, val); break; case VIRTIO_PCI_QUEUE_PFN: pa = (hwaddr)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT; @@ -353,7 +353,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr) ret = proxy->host_features; break; case VIRTIO_PCI_GUEST_FEATURES: - ret = vdev->guest_features; + ret = vdev->guest_features[0]; break; case VIRTIO_PCI_QUEUE_PFN: ret = virtio_queue_get_addr(vdev, vdev->queue_sel) @@ -478,10 +478,11 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address, } } -static unsigned virtio_pci_get_features(DeviceState *d) +static unsigned virtio_pci_get_features(DeviceState *d, unsigned int index) { VirtIOPCIProxy *proxy = to_virtio_pci_proxy(d); - return proxy->host_features; + + return index == 0 ? proxy->host_features : 0; } static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy, @@ -998,7 +999,7 @@ static void virtio_pci_device_plugged(DeviceState *d) proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY; proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE; - proxy->host_features = virtio_bus_get_vdev_features(bus, + proxy->host_features = virtio_bus_get_vdev_features(bus, 0, proxy->host_features); } diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c index e85a979..2814017 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 uint32_t get_features(VirtIODevice *vdev, unsigned int index, uint32_t f) { return f; } diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 013979a..2eb5d3c 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 (vq->vdev->guest_features[0] & (1 << 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 (vdev->guest_features[0] & (1 << VIRTIO_RING_F_EVENT_IDX)) { vring_avail_event(vq, vq->last_avail_idx); } @@ -592,7 +592,10 @@ void virtio_reset(void *opaque) k->reset(vdev); } - vdev->guest_features = 0; + for (i = 0; i < ARRAY_SIZE(vdev->guest_features); i++) { + vdev->guest_features[i] = 0; + } + vdev->queue_sel = 0; vdev->status = 0; vdev->isr = 0; @@ -839,12 +842,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)) && + if (((vdev->guest_features[0] & (1 << 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 (!(vdev->guest_features[0] & (1 << VIRTIO_RING_F_EVENT_IDX))) { return !(vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT); } @@ -924,7 +927,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, &vdev->guest_features[0]); qemu_put_be32(f, vdev->config_len); qemu_put_buffer(f, vdev->config, vdev->config_len); @@ -958,19 +962,19 @@ 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, unsigned int index, uint32_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); + uint32_t supported_features = vbusk->get_features(qbus->parent, index); bool bad = (val & ~supported_features) != 0; val &= supported_features; if (k->set_features) { - k->set_features(vdev, val); + k->set_features(vdev, index, val); } - vdev->guest_features = val; + vdev->guest_features[index] = val; return bad ? -1 : 0; } @@ -1005,8 +1009,9 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) } qemu_get_be32s(f, &features); - if (virtio_set_features(vdev, features) < 0) { - supported_features = k->get_features(qbus->parent); + /* XXX features >= 32 */ + if (virtio_set_features(vdev, 0, features) < 0) { + supported_features = k->get_features(qbus->parent, 0); error_report("Features 0x%x unsupported. Allowed features: 0x%x", features, supported_features); return -1; diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h index 0d2e7b4..03f4432 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); + unsigned (*get_features)(DeviceState *d, unsigned int index); 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,11 @@ 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 virtio_bus_get_vdev_features(VirtioBusState *bus, unsigned int index, uint32_t requested_features); /* Get bad features of the plugged device. */ -uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus); +uint32_t virtio_bus_get_vdev_bad_features(VirtioBusState *bus, + unsigned int index); /* 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.h b/include/hw/virtio/virtio.h index 0726d76..b408166 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. */ @@ -110,6 +116,8 @@ enum virtio_device_endian { VIRTIO_DEVICE_ENDIAN_BIG, }; +#define NR_VIRTIO_FEATURE_WORDS ((VIRTIO_HIGHEST_FEATURE_BIT + 32) / 32) + struct VirtIODevice { DeviceState parent_obj; @@ -117,7 +125,7 @@ struct VirtIODevice uint8_t status; uint8_t isr; uint16_t queue_sel; - uint32_t guest_features; + uint32_t guest_features[NR_VIRTIO_FEATURE_WORDS]; size_t config_len; void *config; uint16_t config_vector; @@ -138,9 +146,10 @@ 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); + uint32_t (*get_features)(VirtIODevice *vdev, unsigned int index, + uint32_t requested_features); + uint32_t (*bad_features)(VirtIODevice *vdev, unsigned int index); + void (*set_features)(VirtIODevice *vdev, unsigned int index, uint32_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 +234,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, unsigned int index, uint32_t val); /* Base devices. */ typedef struct VirtIOBlkConf VirtIOBlkConf; -- 1.7.9.5
Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 04/16] s390x/virtio-ccw: fix check for WRITE_FEAT
We need to check guest feature size, not host feature size to find out whether we should call virtio_set_features(). This check is possible now that vdev->guest_features is an array. 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 2f52b82..62ec852 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -399,7 +399,7 @@ 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)) { + if (features.index < ARRAY_SIZE(vdev->guest_features)) { virtio_set_features(vdev, features.index, features.features); } else { /* -- 1.7.9.5
Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 05/16] virtio: introduce legacy virtio devices
Introduce a helper function to indicate whether a virtio device is operating in legacy or virtio standard mode. It may be used to make decisions about the endianess of virtio accesses and other virtio-1 specific changes, enabling us to support transitional devices. Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> 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 | 13 +++++++++++-- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 2eb5d3c..4149f45 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -883,7 +883,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_device_is_legacy(vdev)) { + 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..c123ee0 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_device_is_legacy(vdev)) { + /* 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 b408166..40e567c 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -275,9 +275,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 bool virtio_device_is_legacy(VirtIODevice *vdev) +{ + return !(vdev->guest_features[1] & (1 << (VIRTIO_F_VERSION_1 - 32))); +} + 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_device_is_legacy(vdev)) { + 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-Nov-27 15:16 UTC
[PATCH RFC v4 06/16] 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.c | 16 ++++++++++++++++ include/hw/virtio/virtio.h | 2 ++ 2 files changed, 18 insertions(+) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 4149f45..2c6bb91 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -96,6 +96,13 @@ static void virtqueue_init(VirtQueue *vq) { hwaddr pa = vq->pa; + if (pa == -1ULL) { + /* + * This is a virtio-1 style vq that has already been setup + * in virtio_queue_set. + */ + return; + } vq->vring.desc = pa; vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc); vq->vring.used = vring_align(vq->vring.avail + @@ -719,6 +726,15 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n) return vdev->vq[n].pa; } +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc, + hwaddr avail, hwaddr used) +{ + vdev->vq[n].pa = -1ULL; + 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) { /* Don't allow guest to flip queue between existent and diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 40e567c..f840320 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -227,6 +227,8 @@ 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_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-Nov-27 15:16 UTC
[PATCH RFC v4 07/16] 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 a051775..0da8d6b 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 (!(vdev->guest_features[0] & (1 << 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 (vdev->guest_features[0] & (1 << 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 ((vdev->guest_features[0] & VIRTIO_F_NOTIFY_ON_EMPTY) && - unlikely(vring->vr.avail->idx == vring->last_avail_idx)) { + unlikely(!vring_more_avail(vdev, vring))) { return true; } if (!(vdev->guest_features[0] & 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-Nov-27 15:16 UTC
[PATCH RFC v4 08/16] 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-Nov-27 15:16 UTC
[PATCH RFC v4 09/16] 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; but we still extend the feature bit size to be able to handle the VERSION_1 bit. 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 | 7 ++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 62ec852..e79f3b8 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; @@ -373,6 +382,13 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) ccw.cda + sizeof(features.features)); if (features.index < ARRAY_SIZE(dev->host_features)) { features.features = dev->host_features[features.index]; + /* + * Don't offer version 1 to the guest if it did not + * negotiate at least revision 1. + */ + if (features.index == 1 && dev->revision <= 0) { + features.features &= ~(1 << (VIRTIO_F_VERSION_1 - 32)); + } } else { /* Return zeroes if the guest supports more feature bits. */ features.features = 0; @@ -400,6 +416,13 @@ 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(vdev->guest_features)) { + /* + * The guest should not set version 1 if it didn't + * negotiate a revision >= 1. + */ + if (features.index == 1 && dev->revision <= 0) { + features.features &= ~(1 << (VIRTIO_F_VERSION_1 - 32)); + } virtio_set_features(vdev, features.index, features.features); } else { /* @@ -600,6 +623,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) { + ret = -ENOSYS; + break; + } + ret = 0; + dev->revision = revinfo.revision; + break; default: ret = -ENOSYS; break; @@ -607,6 +649,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; @@ -733,6 +782,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)); @@ -740,6 +790,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. */ for (i = 0; i < ARRAY_SIZE(dev->host_features); i++) { dev->host_features[i] diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h index 5a1f16e..03d5955 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) \ @@ -69,7 +70,10 @@ typedef struct VirtIOCCWDeviceClass { } VirtIOCCWDeviceClass; /* Change here if we want to support more feature bits. */ -#define VIRTIO_CCW_FEATURE_SIZE 1 +#define VIRTIO_CCW_FEATURE_SIZE NR_VIRTIO_FEATURE_WORDS + +/* The maximum virtio revision we support. */ +#define VIRTIO_CCW_REV_MAX 0 /* Performance improves when virtqueue kick processing is decoupled from the * vcpu thread using ioeventfd for some devices. */ @@ -89,6 +93,7 @@ struct VirtioCcwDevice { SubchDev *sch; char *bus_id; uint32_t host_features[VIRTIO_CCW_FEATURE_SIZE]; + int revision; VirtioBusState bus; bool ioeventfd_started; bool ioeventfd_disabled; -- 1.7.9.5
Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 10/16] 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 e79f3b8..60d67a3 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-Nov-27 15:16 UTC
[PATCH RFC v4 11/16] 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 | 17 +++++++++++++++-- include/hw/virtio/virtio.h | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 2c6bb91..8cdc0cb 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -982,7 +982,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) vmstate_save_state(f, &vmstate_virtio, vdev); } -int virtio_set_features(VirtIODevice *vdev, unsigned int index, uint32_t val) +static int __virtio_set_features(VirtIODevice *vdev, unsigned int index, + uint32_t val) { BusState *qbus = qdev_get_parent_bus(DEVICE(vdev)); VirtioBusClass *vbusk = VIRTIO_BUS_GET_CLASS(qbus); @@ -998,6 +999,18 @@ int virtio_set_features(VirtIODevice *vdev, unsigned int index, uint32_t val) return bad ? -1 : 0; } +int virtio_set_features(VirtIODevice *vdev, unsigned int index, uint32_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, index, val); +} + int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) { int i, ret; @@ -1030,7 +1043,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id) qemu_get_be32s(f, &features); /* XXX features >= 32 */ - if (virtio_set_features(vdev, 0, features) < 0) { + if (__virtio_set_features(vdev, 0, features) < 0) { supported_features = k->get_features(qbus->parent, 0); error_report("Features 0x%x unsupported. Allowed features: 0x%x", features, supported_features); diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index f840320..ec1be3b 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-Nov-27 15:16 UTC
[PATCH RFC v4 12/16] 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 60d67a3..65bef4b 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -547,15 +547,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 8cdc0cb..ab5c671 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -548,15 +548,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_device_is_legacy(vdev)) { + 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 ec1be3b..601578f 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -152,6 +152,7 @@ typedef struct VirtioDeviceClass { uint32_t requested_features); uint32_t (*bad_features)(VirtIODevice *vdev, unsigned int index); void (*set_features)(VirtIODevice *vdev, unsigned int index, uint32_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); @@ -235,7 +236,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, unsigned int index, uint32_t val); -- 1.7.9.5
Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 13/16] 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 03d5955..08edd8d 100644 --- a/hw/s390x/virtio-ccw.h +++ b/hw/s390x/virtio-ccw.h @@ -73,7 +73,7 @@ typedef struct VirtIOCCWDeviceClass { #define VIRTIO_CCW_FEATURE_SIZE NR_VIRTIO_FEATURE_WORDS /* The maximum virtio revision we support. */ -#define VIRTIO_CCW_REV_MAX 0 +#define VIRTIO_CCW_REV_MAX 1 /* Performance improves when virtqueue kick processing is decoupled from the * vcpu thread using ioeventfd for some devices. */ -- 1.7.9.5
Cornelia Huck
2014-Nov-27 15:16 UTC
[PATCH RFC v4 14/16] 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 1e214b5..ad477bf 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 (!(vdev->guest_features[0] >> VIRTIO_NET_F_CTRL_MAC_ADDR & 1) && + !(vdev->guest_features[1] >> (VIRTIO_F_VERSION_1 - 32) & 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-Nov-27 15:16 UTC
[PATCH RFC v4 15/16] 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 | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index ad477bf..b31b3a4 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -380,8 +380,13 @@ static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs) 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 (n->parent_obj.guest_features[1] >> (VIRTIO_F_VERSION_1 - 32) & 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); -- 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 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index b31b3a4..9ceff02 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -453,6 +453,10 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, unsigned int index, VirtIONet *n = VIRTIO_NET(vdev); NetClientState *nc = qemu_get_queue(n->nic); + if (index == 1 && !get_vhost_net(nc->peer)) { + features |= (1 << (VIRTIO_F_VERSION_1 - 32)); + } + if (index > 0) { return features; } -- 1.7.9.5
Michael S. Tsirkin
2014-Nov-27 15:24 UTC
[PATCH RFC v4 00/16] qemu: towards virtio-1 host support
On Thu, Nov 27, 2014 at 04:16:33PM +0100, Cornelia Huck wrote:> Yet another version of the virtio-1 support patches. > > This one has seen some (very) light testing with the virtio-1 guest > support patches currently on vhost-next. > > Changes from v3: > > - Add support for FEATURES_OK. We refuse to set features after the > driver has set this in the status field, and we allow to fail > setting the status if the features are inconsistent. > - Add missing virtio-1 changes for virtio-net (header size and mac). > - Dropped setting the VERSION_1 bit for virtio-blk: There's still > some stuff missing. > > For virtio-blk, we need to validate the feature bits if version 1 is > negotiated: some legacy features are not allowed in that case. I'm not > quite sure how to handle this, though. We could use the new > validate_features callback to verify that the driver negotiated a > sensible feature set, but that would require us to offer a superset > of legacy and version 1 bits, which feels wrong. Any ideas?No, that's violating the spec. I think the simplest way is to have separate features and legacy_features fields. Present the correct one depending on which revision was negotiated.> Cornelia Huck (13): > virtio: cull virtio_bus_set_vdev_features > virtio: support more feature bits > s390x/virtio-ccw: fix check for WRITE_FEAT > virtio: introduce legacy virtio 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 > > 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 | 7 +- > hw/block/dataplane/virtio-blk.c | 4 +- > hw/block/virtio-blk.c | 9 +- > hw/char/virtio-serial-bus.c | 9 +- > hw/net/virtio-net.c | 52 +++++-- > hw/s390x/css.c | 12 ++ > hw/s390x/css.h | 1 + > hw/s390x/s390-virtio-bus.c | 9 +- > hw/s390x/virtio-ccw.c | 206 +++++++++++++++++++------ > hw/s390x/virtio-ccw.h | 7 +- > hw/scsi/vhost-scsi.c | 7 +- > hw/scsi/virtio-scsi-dataplane.c | 2 +- > hw/scsi/virtio-scsi.c | 10 +- > hw/virtio/Makefile.objs | 2 +- > hw/virtio/dataplane/Makefile.objs | 2 +- > hw/virtio/dataplane/vring.c | 96 ++++++------ > hw/virtio/virtio-balloon.c | 8 +- > hw/virtio/virtio-bus.c | 23 +-- > hw/virtio/virtio-mmio.c | 9 +- > hw/virtio/virtio-pci.c | 13 +- > hw/virtio/virtio-rng.c | 2 +- > hw/virtio/virtio.c | 88 +++++++++-- > include/hw/virtio/dataplane/vring-accessors.h | 75 +++++++++ > include/hw/virtio/dataplane/vring.h | 14 +- > include/hw/virtio/virtio-access.h | 4 + > include/hw/virtio/virtio-bus.h | 10 +- > include/hw/virtio/virtio.h | 39 ++++- > linux-headers/linux/virtio_config.h | 3 + > 28 files changed, 523 insertions(+), 200 deletions(-) > create mode 100644 include/hw/virtio/dataplane/vring-accessors.h > > -- > 1.7.9.5
Cornelia Huck
2014-Nov-27 15:31 UTC
[PATCH RFC v4 00/16] qemu: towards virtio-1 host support
On Thu, 27 Nov 2014 17:24:22 +0200 "Michael S. Tsirkin" <mst at redhat.com> wrote:> On Thu, Nov 27, 2014 at 04:16:33PM +0100, Cornelia Huck wrote: > > Yet another version of the virtio-1 support patches. > > > > This one has seen some (very) light testing with the virtio-1 guest > > support patches currently on vhost-next. > > > > Changes from v3: > > > > - Add support for FEATURES_OK. We refuse to set features after the > > driver has set this in the status field, and we allow to fail > > setting the status if the features are inconsistent. > > - Add missing virtio-1 changes for virtio-net (header size and mac). > > - Dropped setting the VERSION_1 bit for virtio-blk: There's still > > some stuff missing. > > > > For virtio-blk, we need to validate the feature bits if version 1 is > > negotiated: some legacy features are not allowed in that case. I'm not > > quite sure how to handle this, though. We could use the new > > validate_features callback to verify that the driver negotiated a > > sensible feature set, but that would require us to offer a superset > > of legacy and version 1 bits, which feels wrong. Any ideas? > > No, that's violating the spec. > I think the simplest way is to have separate features and > legacy_features fields. Present the correct one depending on which > revision was negotiated.But revisions are a virtio-ccw only thing - what can other transports do here? The basic problem is that we decide via a feature bit that needs to be negotiated which feature bits we want to present. pci and mmio don't have a way to know whether the driver wants to use 1.0 or legacy prior to feature negotiation, do they?
Michael S. Tsirkin
2014-Nov-27 15:34 UTC
[PATCH RFC v4 03/16] virtio: support more feature bits
On Thu, Nov 27, 2014 at 04:16:36PM +0100, Cornelia Huck wrote:> With virtio-1, we support more than 32 feature bits. Let's make > vdev->guest_features depend on the number of supported feature bits, > allowing us to grow the feature bits automatically. > > We also need to enhance the internal functions dealing with getting > and setting features with an additional index field, so that all feature > bits may be accessed (in chunks of 32 bits). > > vhost and migration have been ignored for now. > > Reviewed-by: Thomas Huth <thuth at linux.vnet.ibm.com> > Signed-off-by: Cornelia Huck <cornelia.huck at de.ibm.com> > @@ -117,7 +125,7 @@ struct VirtIODevice > uint8_t status; > uint8_t isr; > uint16_t queue_sel; > - uint32_t guest_features; > + uint32_t guest_features[NR_VIRTIO_FEATURE_WORDS]; > size_t config_len; > void *config; > uint16_t config_vector;Ugh. That's quite tricky to use correctly. Why don't we just make it uint64_t? The only real issue is that DEFINE_PROP_BIT wants a uint32_t. But that's easy to fix: add DEFINE_PROP64_BIT that is the same but handles a 64 bit array. -- MST
OK, I am trying to experiment with virtio 1.0 support using the latest kernel and MST's qemu tree: https://git.kernel.org/cgit/virt/kvm/mst/qemu.git/?h=virtio-1.0 The first issue is that the device config endian was wrong (see attached patch). I'm now setting up a BE guest on my x86 laptop, and a BE and LE guest on a BE powerpc machine, to check that all combinations work correctly. If others test too, that would be appreciated! Cheers, Rusty.>From 95ac91554ed602f856a2a5fcc25eaffcad1b1c8d Mon Sep 17 00:00:00 2001From: Rusty Russell <rusty at rustcorp.com.au> Date: Tue, 24 Feb 2015 14:47:44 +1030 Subject: [PATCH] virtio_config_write*/virtio_config_read*: Don't endian swap for virtio 1.0. Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 079944c..882a31b 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -662,7 +662,12 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) k->get_config(vdev, vdev->config); - val = lduw_p(vdev->config + addr); + /* Virtio 1.0 is always LE */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + val = lduw_le_p(vdev->config + addr); + } else { + val = lduw_p(vdev->config + addr); + } return val; } @@ -677,7 +682,12 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr) k->get_config(vdev, vdev->config); - val = ldl_p(vdev->config + addr); + /* Virtio 1.0 is always LE */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + val = ldl_le_p(vdev->config + addr); + } else { + val = ldl_p(vdev->config + addr); + } return val; } @@ -706,7 +716,12 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data) return; } - stw_p(vdev->config + addr, val); + /* Virtio 1.0 is always LE */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + stw_le_p(vdev->config + addr, val); + } else { + stw_p(vdev->config + addr, val); + } if (k->set_config) { k->set_config(vdev, vdev->config); @@ -722,7 +737,12 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) return; } - stl_p(vdev->config + addr, val); + /* Virtio 1.0 is always LE */ + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { + stl_le_p(vdev->config + addr, val); + } else { + stl_p(vdev->config + addr, val); + } if (k->set_config) { k->set_config(vdev, vdev->config);
On Wed, 25 Feb 2015 14:50:22 +1030 Rusty Russell <rusty at rustcorp.com.au> wrote:> OK, I am trying to experiment with virtio 1.0 support using the > latest kernel and MST's qemu tree: > > https://git.kernel.org/cgit/virt/kvm/mst/qemu.git/?h=virtio-1.0 > > The first issue is that the device config endian was wrong (see > attached patch). > > I'm now setting up a BE guest on my x86 laptop, and a BE and LE guest > on a BE powerpc machine, to check that all combinations work correctly. > If others test too, that would be appreciated!My virtio-1 work had been taking the back seat recently, but I plan to look into it again soon.> > Cheers, > Rusty. > > From 95ac91554ed602f856a2a5fcc25eaffcad1b1c8d Mon Sep 17 00:00:00 2001 > From: Rusty Russell <rusty at rustcorp.com.au> > Date: Tue, 24 Feb 2015 14:47:44 +1030 > Subject: [PATCH] virtio_config_write*/virtio_config_read*: Don't endian swap > for virtio 1.0.Ah, virtio-ccw doesn't use these config space accessors, that's why I did not look at them.> > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 079944c..882a31b 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -662,7 +662,12 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) > > k->get_config(vdev, vdev->config); > > - val = lduw_p(vdev->config + addr); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + val = lduw_le_p(vdev->config + addr); > + } else { > + val = lduw_p(vdev->config + addr); > + }Can't you use the virtio_* helpers for that?> return val; > } ><looks at the callers> It seems virtio-pci does some conditional swapping based on virtio-endianness already, which should account for virtio-1. virtio-mmio does not seem to do so. But: Device code accessing config space already does use the virtio accessors - or virtio-ccw would not work as it simply copies the whole config space. So it seems this change simply undos the endian swap in virtio-pci (while probably breaking virtio-mmio). Can we simply get rid of the endian swap in virtio-pci instead, or have I been throuroughly confused?
On Wed, Feb 25, 2015 at 02:50:22PM +1030, Rusty Russell wrote:> OK, I am trying to experiment with virtio 1.0 support using the > latest kernel and MST's qemu tree: > > https://git.kernel.org/cgit/virt/kvm/mst/qemu.git/?h=virtio-1.0 > > The first issue is that the device config endian was wrong (see > attached patch). > > I'm now setting up a BE guest on my x86 laptop, and a BE and LE guest > on a BE powerpc machine, to check that all combinations work correctly. > If others test too, that would be appreciated! > > Cheers, > Rusty.Thanks a lot for finding this! The issue is certainly there, though I think looking at guest features is not the right thing to do: drivers can access config before acking features. At least for PCI, it's very simple: we have a separate memory region for modern devices, we should just use a different accessor, not virtio_config_readw and friends. Untested patch sent (sorry about the untested part, a bit busy right now).> >From 95ac91554ed602f856a2a5fcc25eaffcad1b1c8d Mon Sep 17 00:00:00 2001 > From: Rusty Russell <rusty at rustcorp.com.au> > Date: Tue, 24 Feb 2015 14:47:44 +1030 > Subject: [PATCH] virtio_config_write*/virtio_config_read*: Don't endian swap > for virtio 1.0. > > Signed-off-by: Rusty Russell <rusty at rustcorp.com.au> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 079944c..882a31b 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -662,7 +662,12 @@ uint32_t virtio_config_readw(VirtIODevice *vdev, uint32_t addr) > > k->get_config(vdev, vdev->config); > > - val = lduw_p(vdev->config + addr); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + val = lduw_le_p(vdev->config + addr); > + } else { > + val = lduw_p(vdev->config + addr); > + } > return val; > } > > @@ -677,7 +682,12 @@ uint32_t virtio_config_readl(VirtIODevice *vdev, uint32_t addr) > > k->get_config(vdev, vdev->config); > > - val = ldl_p(vdev->config + addr); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + val = ldl_le_p(vdev->config + addr); > + } else { > + val = ldl_p(vdev->config + addr); > + } > return val; > } > > @@ -706,7 +716,12 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data) > return; > } > > - stw_p(vdev->config + addr, val); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + stw_le_p(vdev->config + addr, val); > + } else { > + stw_p(vdev->config + addr, val); > + } > > if (k->set_config) { > k->set_config(vdev, vdev->config); > @@ -722,7 +737,12 @@ void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data) > return; > } > > - stl_p(vdev->config + addr, val); > + /* Virtio 1.0 is always LE */ > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + stl_le_p(vdev->config + addr, val); > + } else { > + stl_p(vdev->config + addr, val); > + } > > if (k->set_config) { > k->set_config(vdev, vdev->config);
Possibly Parallel Threads
- [PATCH RFC 03/11] virtio: support more feature bits
- [PATCH RFC 03/11] virtio: support more feature bits
- [PATCH RFC v4 03/16] 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