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>