* [PATCH RFC v5 01/19] linux-headers/virtio_config: Update with VIRTIO_F_VERSION_1
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 02/19] virtio: cull virtio_bus_set_vdev_features Cornelia Huck
` (20 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
From: Thomas Huth <thuth@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@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
| 3 +++
1 file changed, 3 insertions(+)
--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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 02/19] virtio: cull virtio_bus_set_vdev_features
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
2014-12-02 13:00 ` [PATCH RFC v5 01/19] linux-headers/virtio_config: Update with VIRTIO_F_VERSION_1 Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 03/19] virtio: feature bit manipulation helpers Cornelia Huck
` (19 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 03/19] virtio: feature bit manipulation helpers
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
2014-12-02 13:00 ` [PATCH RFC v5 01/19] linux-headers/virtio_config: Update with VIRTIO_F_VERSION_1 Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 02/19] virtio: cull virtio_bus_set_vdev_features Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 04/19] virtio: add feature checking helpers Cornelia Huck
` (18 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 04/19] virtio: add feature checking helpers
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (2 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 03/19] virtio: feature bit manipulation helpers Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 05/19] virtio: support more feature bits Cornelia Huck
` (17 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 05/19] virtio: support more feature bits
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (3 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 04/19] virtio: add feature checking helpers Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-10 17:35 ` [Qemu-devel] " Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 06/19] virtio: endianness checks for virtio 1.0 devices Cornelia Huck
` (16 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@de.ibm.com>
---
hw/9pfs/virtio-9p-device.c | 2 +-
hw/block/virtio-blk.c | 2 +-
hw/char/virtio-serial-bus.c | 2 +-
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 | 40 ++++++++++++++++++++--------------
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 | 11 ++++++++++
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 ++++++++++++++++++--------------
22 files changed, 126 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/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..e434718 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,13 @@ 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. */
+ dev->host_features = 0;
+ 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 +786,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 +1072,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 +1389,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 +1494,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 +1622,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 308b393..68e0489 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..23d713b 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -51,6 +51,17 @@ extern PropertyInfo qdev_prop_arraylen;
.defval = (bool)_defval, \
}
+#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) { \
+ .name = (_name), \
+ .info = &(qdev_prop_bit), \
+ .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
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [Qemu-devel] [PATCH RFC v5 05/19] virtio: support more feature bits
2014-12-02 13:00 ` [PATCH RFC v5 05/19] virtio: support more feature bits Cornelia Huck
@ 2014-12-10 17:35 ` Cornelia Huck
0 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-10 17:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, kvm, mst, qemu-devel, virtualization
On Tue, 2 Dec 2014 14:00:13 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 070006c..23d713b 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -51,6 +51,17 @@ extern PropertyInfo qdev_prop_arraylen;
> .defval = (bool)_defval, \
> }
>
> +#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) { \
> + .name = (_name), \
> + .info = &(qdev_prop_bit), \
> + .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), \
This one is of course broken. I'll send an updated patch tomorrow.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFC v5 06/19] virtio: endianness checks for virtio 1.0 devices
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (4 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 05/19] virtio: support more feature bits Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout Cornelia Huck
` (15 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (5 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 06/19] virtio: endianness checks for virtio 1.0 devices Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 14:46 ` Michael S. Tsirkin
2014-12-02 13:00 ` [PATCH RFC v5 08/19] dataplane: allow virtio-1 devices Cornelia Huck
` (14 subsequent siblings)
21 siblings, 1 reply; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@de.ibm.com>
---
hw/virtio/virtio.c | 16 ++++++++++++++++
include/hw/virtio/virtio.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8f69ffa..508dccf 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -96,6 +96,13 @@ static void virtqueue_init(VirtQueue *vq)
{
hwaddr pa = vq->pa;
+ if (pa == -1ULL) {
+ /*
+ * This is a virtio-1 style vq that has already been setup
+ * in virtio_queue_set.
+ */
+ return;
+ }
vq->vring.desc = pa;
vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc);
vq->vring.used = vring_align(vq->vring.avail +
@@ -717,6 +724,15 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
return vdev->vq[n].pa;
}
+void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
+ hwaddr avail, hwaddr used)
+{
+ vdev->vq[n].pa = -1ULL;
+ vdev->vq[n].vring.desc = desc;
+ vdev->vq[n].vring.avail = avail;
+ vdev->vq[n].vring.used = used;
+}
+
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
{
/* Don't allow guest to flip queue between existent and
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 68c40db..80ee313 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -224,6 +224,8 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
int virtio_queue_get_num(VirtIODevice *vdev, int n);
+void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
+ hwaddr avail, hwaddr used);
void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
void virtio_queue_notify(VirtIODevice *vdev, int n);
uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-02 13:00 ` [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout Cornelia Huck
@ 2014-12-02 14:46 ` Michael S. Tsirkin
2014-12-02 14:54 ` Cornelia Huck
0 siblings, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2014-12-02 14:46 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, Dec 02, 2014 at 02:00:15PM +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@de.ibm.com>
> ---
> hw/virtio/virtio.c | 16 ++++++++++++++++
> include/hw/virtio/virtio.h | 2 ++
> 2 files changed, 18 insertions(+)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 8f69ffa..508dccf 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -96,6 +96,13 @@ static void virtqueue_init(VirtQueue *vq)
> {
> hwaddr pa = vq->pa;
>
> + if (pa == -1ULL) {
> + /*
> + * This is a virtio-1 style vq that has already been setup
> + * in virtio_queue_set.
> + */
> + return;
> + }
> vq->vring.desc = pa;
> vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc);
> vq->vring.used = vring_align(vq->vring.avail +
> @@ -717,6 +724,15 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
> return vdev->vq[n].pa;
> }
>
> +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
> + hwaddr avail, hwaddr used)
> +{
> + vdev->vq[n].pa = -1ULL;
> + vdev->vq[n].vring.desc = desc;
> + vdev->vq[n].vring.avail = avail;
> + vdev->vq[n].vring.used = used;
> +}
> +
> void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> {
> /* Don't allow guest to flip queue between existent and
pa == -1ULL tricks look quite ugly.
Can't we set desc/avail/used unconditionally, and drop
the pa value?
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 68c40db..80ee313 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -224,6 +224,8 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
> hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
> void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
> int virtio_queue_get_num(VirtIODevice *vdev, int n);
> +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
> + hwaddr avail, hwaddr used);
> void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
> void virtio_queue_notify(VirtIODevice *vdev, int n);
> uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-02 14:46 ` Michael S. Tsirkin
@ 2014-12-02 14:54 ` Cornelia Huck
2014-12-02 15:41 ` Cornelia Huck
[not found] ` <20141202164136.1a57f358.cornelia.huck@de.ibm.com>
0 siblings, 2 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 14:54 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, 2 Dec 2014 16:46:28 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 02, 2014 at 02:00:15PM +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@de.ibm.com>
> > ---
> > hw/virtio/virtio.c | 16 ++++++++++++++++
> > include/hw/virtio/virtio.h | 2 ++
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 8f69ffa..508dccf 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -96,6 +96,13 @@ static void virtqueue_init(VirtQueue *vq)
> > {
> > hwaddr pa = vq->pa;
> >
> > + if (pa == -1ULL) {
> > + /*
> > + * This is a virtio-1 style vq that has already been setup
> > + * in virtio_queue_set.
> > + */
> > + return;
> > + }
> > vq->vring.desc = pa;
> > vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc);
> > vq->vring.used = vring_align(vq->vring.avail +
> > @@ -717,6 +724,15 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
> > return vdev->vq[n].pa;
> > }
> >
> > +void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
> > + hwaddr avail, hwaddr used)
> > +{
> > + vdev->vq[n].pa = -1ULL;
> > + vdev->vq[n].vring.desc = desc;
> > + vdev->vq[n].vring.avail = avail;
> > + vdev->vq[n].vring.used = used;
> > +}
> > +
> > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> > {
> > /* Don't allow guest to flip queue between existent and
>
> pa == -1ULL tricks look quite ugly.
> Can't we set desc/avail/used unconditionally, and drop
> the pa value?
And have virtio_queue_get_addr() return desc? Let me see if I can come
up with a patch.
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-02 14:54 ` Cornelia Huck
@ 2014-12-02 15:41 ` Cornelia Huck
[not found] ` <20141202164136.1a57f358.cornelia.huck@de.ibm.com>
1 sibling, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 15:41 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, 2 Dec 2014 15:54:44 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> On Tue, 2 Dec 2014 16:46:28 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > pa == -1ULL tricks look quite ugly.
> > Can't we set desc/avail/used unconditionally, and drop
> > the pa value?
>
> And have virtio_queue_get_addr() return desc? Let me see if I can come
> up with a patch.
I came up with the following (untested) patch, which should hopefully
suit mmio as well. I haven't cared about migration yet.
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8f69ffa..ac3c615 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,12 +91,13 @@ struct VirtQueue
};
/* virt queue functions */
-static void virtqueue_init(VirtQueue *vq)
+static void virtqueue_update_rings(VirtQueue *vq)
{
- hwaddr pa = vq->pa;
-
- vq->vring.desc = pa;
- vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc);
+ if (!vq->vring.desc) {
+ /* not yet setup -> nothing to do */
+ return;
+ }
+ vq->vring.avail = vq->vring.desc + vq->vring.num * sizeof(VRingDesc);
vq->vring.used = vring_align(vq->vring.avail +
offsetof(VRingAvail, ring[vq->vring.num]),
vq->vring.align);
@@ -605,7 +605,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,17 +707,34 @@ 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;
+ virtqueue_update_rings(&vdev->vq[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)
{
+ /*
+ * For virtio-1 devices, the number of buffers may only be
+ * updated if the ring addresses have not yet been set up.
+ */
+ if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1) &&
+ vdev->vq[n].vring.desc) {
+ error_report("tried to modify buffer num for virtio-1 device");
+ return;
+ }
/* Don't allow guest to flip queue between existent and
* nonexistent states, or to set it to an invalid size.
*/
@@ -728,7 +744,7 @@ void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
return;
}
vdev->vq[n].vring.num = num;
- virtqueue_init(&vdev->vq[n]);
+ virtqueue_update_rings(&vdev->vq[n]);
}
int virtio_queue_get_num(VirtIODevice *vdev, int n)
@@ -748,6 +764,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 +776,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]);
+ virtqueue_update_rings(&vdev->vq[n]);
}
void virtio_queue_notify_vq(VirtQueue *vq)
@@ -949,7 +970,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 +1066,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 */
+ virtqueue_update_rings(&vdev->vq[i]);
} else if (vdev->vq[i].last_avail_idx) {
error_report("VQ %d address 0x0 "
"inconsistent with Host index 0x%x",
@@ -1084,7 +1107,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..80ee313 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -224,6 +224,8 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
int virtio_queue_get_num(VirtIODevice *vdev, int n);
+void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
+ hwaddr avail, hwaddr used);
void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
void virtio_queue_notify(VirtIODevice *vdev, int n);
uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
^ permalink raw reply related [flat|nested] 35+ messages in thread[parent not found: <20141202164136.1a57f358.cornelia.huck@de.ibm.com>]
* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
[not found] ` <20141202164136.1a57f358.cornelia.huck@de.ibm.com>
@ 2014-12-02 19:03 ` Michael S. Tsirkin
2014-12-03 9:27 ` Cornelia Huck
0 siblings, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2014-12-02 19:03 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
> void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> {
> + /*
> + * For virtio-1 devices, the number of buffers may only be
> + * updated if the ring addresses have not yet been set up.
Where does it say that?
> + */
> + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> + vdev->vq[n].vring.desc) {
> + error_report("tried to modify buffer num for virtio-1 device");
> + return;
> + }
> /* Don't allow guest to flip queue between existent and
> * nonexistent states, or to set it to an invalid size.
> */
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-02 19:03 ` Michael S. Tsirkin
@ 2014-12-03 9:27 ` Cornelia Huck
2014-12-03 9:50 ` Cornelia Huck
[not found] ` <20141203105004.2349cad4.cornelia.huck@de.ibm.com>
0 siblings, 2 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-03 9:27 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, 2 Dec 2014 21:03:45 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
> > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> > {
> > + /*
> > + * For virtio-1 devices, the number of buffers may only be
> > + * updated if the ring addresses have not yet been set up.
>
> Where does it say that?
Hmpf, may have imagined that.
This means we either need to track whether used/avail have been
specified or calculated or move responsibility for re-calculation of
used/avail for the old layout into the callers.
>
> > + */
> > + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1) &&
> > + vdev->vq[n].vring.desc) {
> > + error_report("tried to modify buffer num for virtio-1 device");
> > + return;
> > + }
> > /* Don't allow guest to flip queue between existent and
> > * nonexistent states, or to set it to an invalid size.
> > */
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-03 9:27 ` Cornelia Huck
@ 2014-12-03 9:50 ` Cornelia Huck
[not found] ` <20141203105004.2349cad4.cornelia.huck@de.ibm.com>
1 sibling, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-03 9:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Wed, 3 Dec 2014 10:27:36 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> On Tue, 2 Dec 2014 21:03:45 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
> > > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> > > {
> > > + /*
> > > + * For virtio-1 devices, the number of buffers may only be
> > > + * updated if the ring addresses have not yet been set up.
> >
> > Where does it say that?
>
> Hmpf, may have imagined that.
>
> This means we either need to track whether used/avail have been
> specified or calculated or move responsibility for re-calculation of
> used/avail for the old layout into the callers.
What about this one instead?
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 43b7e02..1e2a720 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -244,9 +244,13 @@ 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);
+ virtio_queue_update_rings(vdev, vdev->queue_sel);
break;
case VIRTIO_MMIO_QUEUEPFN:
if (value == 0) {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8f69ffa..b2d553e 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,6 @@ 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]);
}
void virtio_queue_notify_vq(VirtQueue *vq)
@@ -949,7 +961,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 +1057,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 +1098,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);
^ permalink raw reply related [flat|nested] 35+ messages in thread[parent not found: <20141203105004.2349cad4.cornelia.huck@de.ibm.com>]
* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
[not found] ` <20141203105004.2349cad4.cornelia.huck@de.ibm.com>
@ 2014-12-03 10:52 ` Michael S. Tsirkin
2014-12-03 11:14 ` Cornelia Huck
0 siblings, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2014-12-03 10:52 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, qemu-devel, kvm, virtualization
On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:
> On Wed, 3 Dec 2014 10:27:36 +0100
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
>
> > On Tue, 2 Dec 2014 21:03:45 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Tue, Dec 02, 2014 at 04:41:36PM +0100, Cornelia Huck wrote:
> > > > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> > > > {
> > > > + /*
> > > > + * For virtio-1 devices, the number of buffers may only be
> > > > + * updated if the ring addresses have not yet been set up.
> > >
> > > Where does it say that?
> >
> > Hmpf, may have imagined that.
> >
> > This means we either need to track whether used/avail have been
> > specified or calculated or move responsibility for re-calculation of
> > used/avail for the old layout into the callers.
>
> What about this one instead?
Looks ok overall - some questions below.
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 43b7e02..1e2a720 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -244,9 +244,13 @@ 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);
> + virtio_queue_update_rings(vdev, vdev->queue_sel);
> break;
> case VIRTIO_MMIO_QUEUEPFN:
> if (value == 0) {
Let's just call virtio_queue_update_rings from virtio_queue_set_align?
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 8f69ffa..b2d553e 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)
Do we have to touch this now?
It's only used by MMIO, right?
> @@ -755,7 +768,6 @@ 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]);
Don't we need to update rings?
> }
>
> void virtio_queue_notify_vq(VirtQueue *vq)
> @@ -949,7 +961,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 +1057,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 */
What does XXX mean here?
> + 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 +1098,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);
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-03 10:52 ` Michael S. Tsirkin
@ 2014-12-03 11:14 ` Cornelia Huck
2014-12-03 11:19 ` Michael S. Tsirkin
0 siblings, 1 reply; 35+ messages in thread
From: Cornelia Huck @ 2014-12-03 11:14 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Wed, 3 Dec 2014 12:52:51 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:
> > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> > index 43b7e02..1e2a720 100644
> > --- a/hw/virtio/virtio-mmio.c
> > +++ b/hw/virtio/virtio-mmio.c
> > @@ -244,9 +244,13 @@ 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);
> > + virtio_queue_update_rings(vdev, vdev->queue_sel);
> > break;
> > case VIRTIO_MMIO_QUEUEPFN:
> > if (value == 0) {
>
> Let's just call virtio_queue_update_rings from virtio_queue_set_align?
You're right, set_align is legacy only so we can always call
update_rings.
> > @@ -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)
>
> Do we have to touch this now?
> It's only used by MMIO, right?
I don't think it hurts to put a guard in here.
>
>
> > @@ -755,7 +768,6 @@ 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]);
>
> Don't we need to update rings?
See above, I'll call update_rings in there.
>
> > }
> >
> > void virtio_queue_notify_vq(VirtQueue *vq)
> > @@ -949,7 +961,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 +1057,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 */
>
> What does XXX mean here?
That I have not cared about migration of virtio-1 devices yet :)
>
> > + 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",
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-03 11:14 ` Cornelia Huck
@ 2014-12-03 11:19 ` Michael S. Tsirkin
2014-12-03 11:44 ` Cornelia Huck
0 siblings, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2014-12-03 11:19 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, qemu-devel, kvm, virtualization
On Wed, Dec 03, 2014 at 12:14:10PM +0100, Cornelia Huck wrote:
> On Wed, 3 Dec 2014 12:52:51 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
> > On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:
>
> > > diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> > > index 43b7e02..1e2a720 100644
> > > --- a/hw/virtio/virtio-mmio.c
> > > +++ b/hw/virtio/virtio-mmio.c
> > > @@ -244,9 +244,13 @@ 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);
> > > + virtio_queue_update_rings(vdev, vdev->queue_sel);
> > > break;
> > > case VIRTIO_MMIO_QUEUEPFN:
> > > if (value == 0) {
> >
> > Let's just call virtio_queue_update_rings from virtio_queue_set_align?
>
> You're right, set_align is legacy only so we can always call
> update_rings.
>
>
> > > @@ -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)
> >
> > Do we have to touch this now?
> > It's only used by MMIO, right?
>
> I don't think it hurts to put a guard in here.
I'd say let's not touch mmio ATM.
> >
> >
> > > @@ -755,7 +768,6 @@ 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]);
> >
> > Don't we need to update rings?
>
> See above, I'll call update_rings in there.
>
> >
> > > }
> > >
> > > void virtio_queue_notify_vq(VirtQueue *vq)
> > > @@ -949,7 +961,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 +1057,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 */
> >
> > What does XXX mean here?
>
> That I have not cared about migration of virtio-1 devices yet :)
OK sure, but why put comment here not at start of
function?
> >
> > > + 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",
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout
2014-12-03 11:19 ` Michael S. Tsirkin
@ 2014-12-03 11:44 ` Cornelia Huck
0 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-03 11:44 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Wed, 3 Dec 2014 13:19:17 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Dec 03, 2014 at 12:14:10PM +0100, Cornelia Huck wrote:
> > On Wed, 3 Dec 2014 12:52:51 +0200
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> >
> > > On Wed, Dec 03, 2014 at 10:50:04AM +0100, Cornelia Huck wrote:
> > > > @@ -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)
> > >
> > > Do we have to touch this now?
> > > It's only used by MMIO, right?
> >
> > I don't think it hurts to put a guard in here.
>
> I'd say let's not touch mmio ATM.
This is not mmio but common code :) I don't really see how this can
possibly hurt us; when mmio is converted to virtio-1, their queue setup
code needs to be changed anyway.
> > > > @@ -949,7 +961,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 +1057,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 */
> > >
> > > What does XXX mean here?
> >
> > That I have not cared about migration of virtio-1 devices yet :)
>
> OK sure, but why put comment here not at start of
> function?
I find it easier to annotate the places I notice. YMMV.
>
> > >
> > > > + 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",
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH RFC v5 08/19] dataplane: allow virtio-1 devices
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (6 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 07/19] virtio: allow virtio-1 queue layout Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 09/19] s390x/css: Add a callback for when subchannel gets disabled Cornelia Huck
` (13 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 09/19] s390x/css: Add a callback for when subchannel gets disabled
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (7 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 08/19] dataplane: allow virtio-1 devices Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call Cornelia Huck
` (12 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
From: Thomas Huth <thuth@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@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (8 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 09/19] s390x/css: Add a callback for when subchannel gets disabled Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 11/19] s390x/virtio-ccw: support virtio-1 set_vq format Cornelia Huck
` (11 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
From: Thomas Huth <thuth@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@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
---
hw/s390x/virtio-ccw.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
hw/s390x/virtio-ccw.h | 5 +++++
2 files changed, 57 insertions(+)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index e434718..5311d9f 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) {
+ 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. */
dev->host_features = 0;
virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index 9087f7a..fe5c782 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) \
@@ -68,6 +69,9 @@ typedef struct VirtIOCCWDeviceClass {
int (*exit)(VirtioCcwDevice *dev);
} VirtIOCCWDeviceClass;
+/* The maximum virtio revision we support. */
+#define VIRTIO_CCW_REV_MAX 0
+
/* Performance improves when virtqueue kick processing is decoupled from the
* vcpu thread using ioeventfd for some devices. */
#define VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT 1
@@ -86,6 +90,7 @@ struct VirtioCcwDevice {
SubchDev *sch;
char *bus_id;
uint64_t host_features;
+ int revision;
VirtioBusState bus;
bool ioeventfd_started;
bool ioeventfd_disabled;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 11/19] s390x/virtio-ccw: support virtio-1 set_vq format
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (9 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 12/19] virtio: disallow late feature changes for virtio-1 Cornelia Huck
` (10 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@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 5311d9f..75c9ff9 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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 12/19] virtio: disallow late feature changes for virtio-1
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (10 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 11/19] s390x/virtio-ccw: support virtio-1 set_vq format Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 13/19] virtio: allow to fail setting status Cornelia Huck
` (9 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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 508dccf..4f2dc48 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -980,7 +980,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);
@@ -996,6 +996,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;
@@ -1028,7 +1040,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 80ee313..9a984c2 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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 13/19] virtio: allow to fail setting status
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (11 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 12/19] virtio: disallow late feature changes for virtio-1 Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0 Cornelia Huck
` (8 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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 75c9ff9..ec492b8 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 4f2dc48..be128f7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -548,15 +548,37 @@ void virtio_update_irq(VirtIODevice *vdev)
virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
}
-void virtio_set_status(VirtIODevice *vdev, uint8_t val)
+static int virtio_validate_features(VirtIODevice *vdev)
+{
+ VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+
+ if (k->validate_features) {
+ return k->validate_features(vdev);
+ } else {
+ return 0;
+ }
+}
+
+int virtio_set_status(VirtIODevice *vdev, uint8_t val)
{
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
trace_virtio_set_status(vdev, val);
+ if (virtio_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 9a984c2..e7bedd1 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);
@@ -232,7 +233,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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (12 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 13/19] virtio: allow to fail setting status Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 15/19] virtio-net: no writeable mac for virtio-1 Cornelia Huck
` (7 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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 fe5c782..d40e3be 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -70,7 +70,7 @@ typedef struct VirtIOCCWDeviceClass {
} VirtIOCCWDeviceClass;
/* The maximum virtio revision we support. */
-#define VIRTIO_CCW_REV_MAX 0
+#define VIRTIO_CCW_REV_MAX 1
/* Performance improves when virtqueue kick processing is decoupled from the
* vcpu thread using ioeventfd for some devices. */
--
1.7.9.5
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 15/19] virtio-net: no writeable mac for virtio-1
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (13 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0 Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 16/19] virtio-net: support longer header Cornelia Huck
` (6 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
Devices operating as virtio 1.0 may not allow writes to the mac
address in config space.
Signed-off-by: Cornelia Huck <cornelia.huck@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 16/19] virtio-net: support longer header
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (14 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 15/19] virtio-net: no writeable mac for virtio-1 Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 17/19] virtio-net: enable virtio 1.0 Cornelia Huck
` (5 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 17/19] virtio-net: enable virtio 1.0
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (15 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 16/19] virtio-net: support longer header Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 18/19] virtio: support revision-specific features Cornelia Huck
` (4 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 18/19] virtio: support revision-specific features
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (16 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 17/19] virtio-net: enable virtio 1.0 Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
2014-12-02 13:00 ` [PATCH RFC v5 19/19] virtio-blk: revision specific feature bits Cornelia Huck
` (3 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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 ec492b8..3826074 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)
@@ -854,8 +861,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 e7bedd1..f31e3df 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
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH RFC v5 19/19] virtio-blk: revision specific feature bits
[not found] <1417525227-14051-1-git-send-email-cornelia.huck@de.ibm.com>
` (17 preceding siblings ...)
2014-12-02 13:00 ` [PATCH RFC v5 18/19] virtio: support revision-specific features Cornelia Huck
@ 2014-12-02 13:00 ` Cornelia Huck
[not found] ` <1417525227-14051-11-git-send-email-cornelia.huck@de.ibm.com>
` (2 subsequent siblings)
21 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-02 13:00 UTC (permalink / raw)
To: virtualization, qemu-devel, kvm; +Cc: thuth, mst
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@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
^ permalink raw reply related [flat|nested] 35+ messages in thread[parent not found: <1417525227-14051-11-git-send-email-cornelia.huck@de.ibm.com>]
* Re: [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call
[not found] ` <1417525227-14051-11-git-send-email-cornelia.huck@de.ibm.com>
@ 2014-12-04 16:20 ` Michael S. Tsirkin
[not found] ` <20141204162005.GA23214@redhat.com>
1 sibling, 0 replies; 35+ messages in thread
From: Michael S. Tsirkin @ 2014-12-04 16:20 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, Dec 02, 2014 at 02:00:18PM +0100, Cornelia Huck wrote:
> From: Thomas Huth <thuth@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@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> ---
> hw/s390x/virtio-ccw.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> hw/s390x/virtio-ccw.h | 5 +++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index e434718..5311d9f 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) {
> + 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. */
> dev->host_features = 0;
> virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
You should also clear it on device reset.
> diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
> index 9087f7a..fe5c782 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) \
> @@ -68,6 +69,9 @@ typedef struct VirtIOCCWDeviceClass {
> int (*exit)(VirtioCcwDevice *dev);
> } VirtIOCCWDeviceClass;
>
> +/* The maximum virtio revision we support. */
> +#define VIRTIO_CCW_REV_MAX 0
> +
> /* Performance improves when virtqueue kick processing is decoupled from the
> * vcpu thread using ioeventfd for some devices. */
> #define VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT 1
> @@ -86,6 +90,7 @@ struct VirtioCcwDevice {
> SubchDev *sch;
> char *bus_id;
> uint64_t host_features;
> + int revision;
> VirtioBusState bus;
> bool ioeventfd_started;
> bool ioeventfd_disabled;
> --
> 1.7.9.5
^ permalink raw reply [flat|nested] 35+ messages in thread[parent not found: <20141204162005.GA23214@redhat.com>]
* Re: [PATCH RFC v5 10/19] s390x/virtio-ccw: add virtio set-revision call
[not found] ` <20141204162005.GA23214@redhat.com>
@ 2014-12-04 16:43 ` Cornelia Huck
0 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-04 16:43 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Thu, 4 Dec 2014 18:20:05 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 02, 2014 at 02:00:18PM +0100, Cornelia Huck wrote:
> > From: Thomas Huth <thuth@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@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> > ---
> > hw/s390x/virtio-ccw.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> > hw/s390x/virtio-ccw.h | 5 +++++
> > 2 files changed, 57 insertions(+)
> > @@ -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. */
> > dev->host_features = 0;
> > virtio_add_feature(&dev->host_features, VIRTIO_F_NOTIFY_ON_EMPTY);
>
> You should also clear it on device reset.
Nope, revision survives reset and can only be cleared by disabling and
reenabling the device.
^ permalink raw reply [flat|nested] 35+ messages in thread
[parent not found: <1417525227-14051-15-git-send-email-cornelia.huck@de.ibm.com>]
* Re: [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0
[not found] ` <1417525227-14051-15-git-send-email-cornelia.huck@de.ibm.com>
@ 2014-12-09 13:46 ` Michael S. Tsirkin
2014-12-09 17:24 ` Cornelia Huck
0 siblings, 1 reply; 35+ messages in thread
From: Michael S. Tsirkin @ 2014-12-09 13:46 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, Dec 02, 2014 at 02:00:22PM +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@de.ibm.com>
Looks like this will allow revision 1 for all devices,
we only want this for virtio 1 devices.
The following should fix it I think:
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
diff --git a/hw/s390x/virtio-ccw.h b/hw/s390x/virtio-ccw.h
index d40e3be..f5a1d3e 100644
--- a/hw/s390x/virtio-ccw.h
+++ b/hw/s390x/virtio-ccw.h
@@ -69,9 +69,6 @@ typedef struct VirtIOCCWDeviceClass {
int (*exit)(VirtioCcwDevice *dev);
} VirtIOCCWDeviceClass;
-/* The maximum virtio revision we support. */
-#define VIRTIO_CCW_REV_MAX 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
@@ -104,6 +101,12 @@ struct VirtioCcwDevice {
uint64_t ind_bit;
};
+/* The maximum virtio revision we support. */
+static int virtio_ccw_rev_max(VirtioCcwDevice *dev)
+{
+ return dev->host_features & (1ULL << VIRTIO_F_VERSION_1) ? 1 : 0;
+}
+
/* virtual css bus type */
typedef struct VirtualCssBus {
BusState parent_obj;
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 3826074..922b021 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -693,7 +693,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
}
cpu_physical_memory_read(ccw.cda, &revinfo, len);
if (dev->revision >= 0 ||
- revinfo.revision > VIRTIO_CCW_REV_MAX) {
+ revinfo.revision > virtio_ccw_rev_max(dev)) {
ret = -ENOSYS;
break;
}
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0
2014-12-09 13:46 ` [PATCH RFC v5 14/19] s390x/virtio-ccw: enable virtio 1.0 Michael S. Tsirkin
@ 2014-12-09 17:24 ` Cornelia Huck
0 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-09 17:24 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: thuth, qemu-devel, kvm, virtualization
On Tue, 9 Dec 2014 15:46:46 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Dec 02, 2014 at 02:00:22PM +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@de.ibm.com>
>
> Looks like this will allow revision 1 for all devices,
> we only want this for virtio 1 devices.
>
> The following should fix it I think:
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
>
> @@ -104,6 +101,12 @@ struct VirtioCcwDevice {
> uint64_t ind_bit;
> };
>
> +/* The maximum virtio revision we support. */
> +static int virtio_ccw_rev_max(VirtioCcwDevice *dev)
Make this "static inline int" and I'm fine with this :)
> +{
> + return dev->host_features & (1ULL << VIRTIO_F_VERSION_1) ? 1 : 0;
> +}
> +
> /* virtual css bus type */
> typedef struct VirtualCssBus {
> BusState parent_obj;
^ permalink raw reply [flat|nested] 35+ messages in thread
[parent not found: <1417525227-14051-19-git-send-email-cornelia.huck@de.ibm.com>]
* Re: [Qemu-devel] [PATCH RFC v5 18/19] virtio: support revision-specific features
[not found] ` <1417525227-14051-19-git-send-email-cornelia.huck@de.ibm.com>
@ 2014-12-10 17:37 ` Cornelia Huck
0 siblings, 0 replies; 35+ messages in thread
From: Cornelia Huck @ 2014-12-10 17:37 UTC (permalink / raw)
To: Cornelia Huck; +Cc: thuth, kvm, mst, qemu-devel, virtualization
On Tue, 2 Dec 2014 14:00:26 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> 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@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(-)
There seems to be something wrong with this patch - I noticed when I
fixed prop_bit64. Needs debugging.
^ permalink raw reply [flat|nested] 35+ messages in thread