* [PATCHv2 05/14] virtio_test: support event index
From: Michael S. Tsirkin @ 2011-05-19 23:11 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Add ability to test the new event idx feature,
enable by default.
---
tools/virtio/virtio_test.c | 19 +++++++++++++++++--
1 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index df0c6d2..74d3331 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -198,6 +198,14 @@ const struct option longopts[] = {
.val = 'h',
},
{
+ .name = "event-idx",
+ .val = 'E',
+ },
+ {
+ .name = "no-event-idx",
+ .val = 'e',
+ },
+ {
.name = "indirect",
.val = 'I',
},
@@ -211,13 +219,17 @@ const struct option longopts[] = {
static void help()
{
- fprintf(stderr, "Usage: virtio_test [--help] [--no-indirect]\n");
+ fprintf(stderr, "Usage: virtio_test [--help]"
+ " [--no-indirect]"
+ " [--no-event-idx]"
+ "\n");
}
int main(int argc, char **argv)
{
struct vdev_info dev;
- unsigned long long features = 1ULL << VIRTIO_RING_F_INDIRECT_DESC;
+ unsigned long long features = (1ULL << VIRTIO_RING_F_INDIRECT_DESC) |
+ (1ULL << VIRTIO_RING_F_EVENT_IDX);
int o;
for (;;) {
@@ -228,6 +240,9 @@ int main(int argc, char **argv)
case '?':
help();
exit(2);
+ case 'e':
+ features &= ~(1ULL << VIRTIO_RING_F_EVENT_IDX);
+ break;
case 'h':
help();
goto done;
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 06/14] virtio: add api for delayed callbacks
From: Michael S. Tsirkin @ 2011-05-19 23:11 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Add an API that tells the other side that callbacks
should be delayed until a lot of work has been done.
Implement using the new event_idx feature.
Note: it might seem advantageous to let the drivers
ask for a callback after a specific capacity has
been reached. However, as a single head can
free many entries in the descriptor table,
we don't really have a clue about capacity
until get_buf is called. The API is the simplest
to implement at the moment, we'll see what kind of
hints drivers can pass when there's more than one
user of the feature.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 27 +++++++++++++++++++++++++++
include/linux/virtio.h | 9 +++++++++
2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 1d0f9be..6578e1a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -376,6 +376,33 @@ bool virtqueue_enable_cb(struct virtqueue *_vq)
}
EXPORT_SYMBOL_GPL(virtqueue_enable_cb);
+bool virtqueue_enable_cb_delayed(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ u16 bufs;
+
+ START_USE(vq);
+
+ /* We optimistically turn back on interrupts, then check if there was
+ * more to do. */
+ /* Depending on the VIRTIO_RING_F_USED_EVENT_IDX feature, we need to
+ * either clear the flags bit or point the event index at the next
+ * entry. Always do both to keep code simple. */
+ vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
+ /* TODO: tune this threshold */
+ bufs = (u16)(vq->vring.avail->idx - vq->last_used_idx) * 3 / 4;
+ vring_used_event(&vq->vring) = vq->last_used_idx + bufs;
+ virtio_mb();
+ if (unlikely((u16)(vq->vring.used->idx - vq->last_used_idx) > bufs)) {
+ END_USE(vq);
+ return false;
+ }
+
+ END_USE(vq);
+ return true;
+}
+EXPORT_SYMBOL_GPL(virtqueue_enable_cb_delayed);
+
void *virtqueue_detach_unused_buf(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index aff5b4f..7108857 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -51,6 +51,13 @@ struct virtqueue {
* This re-enables callbacks; it returns "false" if there are pending
* buffers in the queue, to detect a possible race between the driver
* checking for more work, and enabling callbacks.
+ * virtqueue_enable_cb_delayed: restart callbacks after disable_cb.
+ * vq: the struct virtqueue we're talking about.
+ * This re-enables callbacks but hints to the other side to delay
+ * interrupts until most of the available buffers have been processed;
+ * it returns "false" if there are many pending buffers in the queue,
+ * to detect a possible race between the driver checking for more work,
+ * and enabling callbacks.
* virtqueue_detach_unused_buf: detach first unused buffer
* vq: the struct virtqueue we're talking about.
* Returns NULL or the "data" token handed to add_buf
@@ -86,6 +93,8 @@ void virtqueue_disable_cb(struct virtqueue *vq);
bool virtqueue_enable_cb(struct virtqueue *vq);
+bool virtqueue_enable_cb_delayed(struct virtqueue *vq);
+
void *virtqueue_detach_unused_buf(struct virtqueue *vq);
/**
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 07/14] virtio_net: delay TX callbacks
From: Michael S. Tsirkin @ 2011-05-19 23:11 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Ask for delayed callbacks on TX ring full, to give the
other side more of a chance to make progress.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0cb0b06..f685324 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -609,7 +609,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
* before it gets out of hand. Naturally, this wastes entries. */
if (capacity < 2+MAX_SKB_FRAGS) {
netif_stop_queue(dev);
- if (unlikely(!virtqueue_enable_cb(vi->svq))) {
+ if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
/* More just got used, free them then recheck. */
capacity += free_old_xmit_skbs(vi);
if (capacity >= 2+MAX_SKB_FRAGS) {
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 08/14] virtio_ring: Add capacity check API
From: Michael S. Tsirkin @ 2011-05-19 23:11 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
>From: Shirley Ma <mashirle@us.ibm.com>
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 8 ++++++++
include/linux/virtio.h | 5 +++++
2 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 6578e1a..eed5f29 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -344,6 +344,14 @@ void *virtqueue_get_buf(struct virtqueue *_vq, unsigned int *len)
}
EXPORT_SYMBOL_GPL(virtqueue_get_buf);
+int virtqueue_get_capacity(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+
+ return vq->num_free;
+}
+EXPORT_SYMBOL_GPL(virtqueue_get_capacity);
+
void virtqueue_disable_cb(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7108857..58c0953 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -42,6 +42,9 @@ struct virtqueue {
* vq: the struct virtqueue we're talking about.
* len: the length written into the buffer
* Returns NULL or the "data" token handed to add_buf.
+ * virtqueue_get_capacity: get the current capacity of the queue
+ * vq: the struct virtqueue we're talking about.
+ * Returns remaining capacity of the queue.
* virtqueue_disable_cb: disable callbacks
* vq: the struct virtqueue we're talking about.
* Note that this is not necessarily synchronous, hence unreliable and only
@@ -89,6 +92,8 @@ void virtqueue_kick(struct virtqueue *vq);
void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len);
+int virtqueue_get_capacity(struct virtqueue *vq);
+
void virtqueue_disable_cb(struct virtqueue *vq);
bool virtqueue_enable_cb(struct virtqueue *vq);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 09/14] virtio_net: fix TX capacity checks using new API
From: Michael S. Tsirkin @ 2011-05-19 23:11 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
virtio net uses the number of sg entries to
check for TX ring capacity freed. But this
gives incorrect results when indirect buffers
are used. Use the new capacity API instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 9 ++++-----
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f685324..f33c92b 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,19 +509,17 @@ again:
return received;
}
-static unsigned int free_old_xmit_skbs(struct virtnet_info *vi)
+static void free_old_xmit_skbs(struct virtnet_info *vi)
{
struct sk_buff *skb;
- unsigned int len, tot_sgs = 0;
+ unsigned int len;
while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
pr_debug("Sent skb %p\n", skb);
vi->dev->stats.tx_bytes += skb->len;
vi->dev->stats.tx_packets++;
- tot_sgs += skb_vnet_hdr(skb)->num_sg;
dev_kfree_skb_any(skb);
}
- return tot_sgs;
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -611,7 +609,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
/* More just got used, free them then recheck. */
- capacity += free_old_xmit_skbs(vi);
+ free_old_xmit_skbs(vi);
+ capacity = virtqueue_get_capacity(vi->svq);
if (capacity >= 2+MAX_SKB_FRAGS) {
netif_start_queue(dev);
virtqueue_disable_cb(vi->svq);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 10/14] virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-05-19 23:11 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Current code might introduce a lot of latency variation
if there are many pending bufs at the time we
attempt to transmit a new one. This is bad for
real-time applications and can't be good for TCP either.
Free up just enough to both clean up all buffers
eventually and to be able to xmit the next packet.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/net/virtio_net.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f33c92b..42935cb 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -509,17 +509,25 @@ again:
return received;
}
-static void free_old_xmit_skbs(struct virtnet_info *vi)
+static bool free_old_xmit_skbs(struct virtnet_info *vi, int capacity)
{
struct sk_buff *skb;
unsigned int len;
-
- while ((skb = virtqueue_get_buf(vi->svq, &len)) != NULL) {
+ bool c;
+ int n;
+
+ /* We try to free up at least 2 skbs per one sent, so that we'll get
+ * all of the memory back if they are used fast enough. */
+ for (n = 0;
+ ((c = virtqueue_get_capacity(vi->svq) < capacity) || n < 2) &&
+ ((skb = virtqueue_get_buf(vi->svq, &len)));
+ ++n) {
pr_debug("Sent skb %p\n", skb);
vi->dev->stats.tx_bytes += skb->len;
vi->dev->stats.tx_packets++;
dev_kfree_skb_any(skb);
}
+ return !c;
}
static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
@@ -574,8 +582,8 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
struct virtnet_info *vi = netdev_priv(dev);
int capacity;
- /* Free up any pending old buffers before queueing new ones. */
- free_old_xmit_skbs(vi);
+ /* Free enough pending old buffers to enable queueing new ones. */
+ free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS);
/* Try to transmit */
capacity = xmit_skb(vi, skb);
@@ -609,9 +617,7 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
netif_stop_queue(dev);
if (unlikely(!virtqueue_enable_cb_delayed(vi->svq))) {
/* More just got used, free them then recheck. */
- free_old_xmit_skbs(vi);
- capacity = virtqueue_get_capacity(vi->svq);
- if (capacity >= 2+MAX_SKB_FRAGS) {
+ if (!likely(free_old_xmit_skbs(vi, 2+MAX_SKB_FRAGS))) {
netif_start_queue(dev);
virtqueue_disable_cb(vi->svq);
}
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 11/14] virtio: don't delay avail index update
From: Michael S. Tsirkin @ 2011-05-19 23:12 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Update avail index immediately instead of upon kick:
for virtio-net RX this helps parallelism with the host.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/virtio/virtio_ring.c | 28 +++++++++++++++++++---------
1 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index eed5f29..8218fe6 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -89,7 +89,7 @@ struct vring_virtqueue
unsigned int num_free;
/* Head of free buffer list. */
unsigned int free_head;
- /* Number we've added since last sync. */
+ /* Number we've added since last kick. */
unsigned int num_added;
/* Last used index we've seen. */
@@ -174,6 +174,13 @@ int virtqueue_add_buf_gfp(struct virtqueue *_vq,
BUG_ON(data == NULL);
+ /* Prevent drivers from adding more than num bufs without a kick. */
+ if (vq->num_added == vq->vring.num) {
+ printk(KERN_ERR "gaaa!!!\n");
+ END_USE(vq);
+ return -ENOSPC;
+ }
+
/* If the host supports indirect descriptor tables, and we have multiple
* buffers, then go indirect. FIXME: tune this threshold */
if (vq->indirect && (out + in) > 1 && vq->num_free) {
@@ -227,8 +234,14 @@ add_head:
/* Put entry in available array (but don't update avail->idx until they
* do sync). FIXME: avoid modulus here? */
- avail = (vq->vring.avail->idx + vq->num_added++) % vq->vring.num;
+ avail = vq->vring.avail->idx % vq->vring.num;
vq->vring.avail->ring[avail] = head;
+ vq->num_added++;
+
+ /* Descriptors and available array need to be set before we expose the
+ * new available array entries. */
+ virtio_wmb();
+ vq->vring.avail->idx++;
pr_debug("Added buffer head %i to %p\n", head, vq);
END_USE(vq);
@@ -242,17 +255,14 @@ void virtqueue_kick(struct virtqueue *_vq)
struct vring_virtqueue *vq = to_vvq(_vq);
u16 new, old;
START_USE(vq);
- /* Descriptors and available array need to be set before we expose the
- * new available array entries. */
- virtio_wmb();
-
- old = vq->vring.avail->idx;
- new = vq->vring.avail->idx = old + vq->num_added;
- vq->num_added = 0;
/* Need to update avail index before checking if we should notify */
virtio_mb();
+ new = vq->vring.avail->idx;
+ old = new - vq->num_added;
+ vq->num_added = 0;
+
if (vq->event ?
vring_need_event(vring_avail_event(&vq->vring), new, old) :
!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 12/14] virtio: 64 bit features
From: Michael S. Tsirkin @ 2011-05-19 23:12 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Extend features to 64 bit so we can use more
transport bits.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/lguest/lguest_device.c | 8 ++++----
drivers/s390/kvm/kvm_virtio.c | 8 ++++----
drivers/virtio/virtio.c | 8 ++++----
drivers/virtio/virtio_pci.c | 34 ++++++++++++++++++++++++++++------
drivers/virtio/virtio_ring.c | 2 ++
include/linux/virtio.h | 2 +-
include/linux/virtio_config.h | 15 +++++++++------
include/linux/virtio_pci.h | 9 ++++++++-
8 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/drivers/lguest/lguest_device.c b/drivers/lguest/lguest_device.c
index 69c84a1..d2d6953 100644
--- a/drivers/lguest/lguest_device.c
+++ b/drivers/lguest/lguest_device.c
@@ -93,17 +93,17 @@ static unsigned desc_size(const struct lguest_device_desc *desc)
}
/* This gets the device's feature bits. */
-static u32 lg_get_features(struct virtio_device *vdev)
+static u64 lg_get_features(struct virtio_device *vdev)
{
unsigned int i;
- u32 features = 0;
+ u64 features = 0;
struct lguest_device_desc *desc = to_lgdev(vdev)->desc;
u8 *in_features = lg_features(desc);
/* We do this the slow but generic way. */
- for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+ for (i = 0; i < min(desc->feature_len * 8, 64); i++)
if (in_features[i / 8] & (1 << (i % 8)))
- features |= (1 << i);
+ features |= (1ull << i);
return features;
}
diff --git a/drivers/s390/kvm/kvm_virtio.c b/drivers/s390/kvm/kvm_virtio.c
index 414427d..c56293c 100644
--- a/drivers/s390/kvm/kvm_virtio.c
+++ b/drivers/s390/kvm/kvm_virtio.c
@@ -79,16 +79,16 @@ static unsigned desc_size(const struct kvm_device_desc *desc)
}
/* This gets the device's feature bits. */
-static u32 kvm_get_features(struct virtio_device *vdev)
+static u64 kvm_get_features(struct virtio_device *vdev)
{
unsigned int i;
- u32 features = 0;
+ u64 features = 0;
struct kvm_device_desc *desc = to_kvmdev(vdev)->desc;
u8 *in_features = kvm_vq_features(desc);
- for (i = 0; i < min(desc->feature_len * 8, 32); i++)
+ for (i = 0; i < min(desc->feature_len * 8, 64); i++)
if (in_features[i / 8] & (1 << (i % 8)))
- features |= (1 << i);
+ features |= (1ull << i);
return features;
}
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index efb35aa..52b24d7 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -112,7 +112,7 @@ static int virtio_dev_probe(struct device *_d)
struct virtio_device *dev = container_of(_d,struct virtio_device,dev);
struct virtio_driver *drv = container_of(dev->dev.driver,
struct virtio_driver, driver);
- u32 device_features;
+ u64 device_features;
/* We have a driver! */
add_status(dev, VIRTIO_CONFIG_S_DRIVER);
@@ -124,14 +124,14 @@ static int virtio_dev_probe(struct device *_d)
memset(dev->features, 0, sizeof(dev->features));
for (i = 0; i < drv->feature_table_size; i++) {
unsigned int f = drv->feature_table[i];
- BUG_ON(f >= 32);
- if (device_features & (1 << f))
+ BUG_ON(f >= 64);
+ if (device_features & (1ull << f))
set_bit(f, dev->features);
}
/* Transport features always preserved to pass to finalize_features. */
for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++)
- if (device_features & (1 << i))
+ if (device_features & (1ull << i))
set_bit(i, dev->features);
dev->config->finalize_features(dev);
diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
index 4fb5b2b..04b216f 100644
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -44,6 +44,8 @@ struct virtio_pci_device
spinlock_t lock;
struct list_head virtqueues;
+ /* 64 bit features */
+ int features_hi;
/* MSI-X support */
int msix_enabled;
int intx_enabled;
@@ -103,26 +105,46 @@ static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
}
/* virtio config->get_features() implementation */
-static u32 vp_get_features(struct virtio_device *vdev)
+static u64 vp_get_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ u32 flo, fhi;
- /* When someone needs more than 32 feature bits, we'll need to
+ /* When someone needs more than 32 feature bits, we need to
* steal a bit to indicate that the rest are somewhere else. */
- return ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES);
+ flo = ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES);
+ if (flo & (0x1 << VIRTIO_F_FEATURES_HI)) {
+ vp_dev->features_hi = 1;
+ iowrite32(0x1 << VIRTIO_F_FEATURES_HI,
+ vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES);
+ fhi = ioread32(vp_dev->ioaddr + VIRTIO_PCI_HOST_FEATURES_HI);
+ } else {
+ vp_dev->features_hi = 0;
+ fhi = 0;
+ }
+ return (((u64)fhi) << 32) | flo;
}
/* virtio config->finalize_features() implementation */
static void vp_finalize_features(struct virtio_device *vdev)
{
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+ u32 flo, fhi;
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
- /* We only support 32 feature bits. */
- BUILD_BUG_ON(ARRAY_SIZE(vdev->features) != 1);
- iowrite32(vdev->features[0], vp_dev->ioaddr+VIRTIO_PCI_GUEST_FEATURES);
+ /* We only support 64 feature bits. */
+ BUILD_BUG_ON(ARRAY_SIZE(vdev->features) != 64 / BITS_PER_LONG);
+ flo = vdev->features[0];
+ fhi = vdev->features[64 / BITS_PER_LONG - 1] >> (BITS_PER_LONG - 32);
+ iowrite32(flo, vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES);
+ if (flo & (0x1 << VIRTIO_F_FEATURES_HI)) {
+ vp_dev->features_hi = 1;
+ iowrite32(fhi, vp_dev->ioaddr + VIRTIO_PCI_GUEST_FEATURES_HI);
+ } else {
+ vp_dev->features_hi = 0;
+ }
}
/* virtio config->get() implementation */
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 8218fe6..4a7a651 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -534,6 +534,8 @@ void vring_transport_features(struct virtio_device *vdev)
for (i = VIRTIO_TRANSPORT_F_START; i < VIRTIO_TRANSPORT_F_END; i++) {
switch (i) {
+ case VIRTIO_F_FEATURES_HI:
+ break;
case VIRTIO_RING_F_INDIRECT_DESC:
break;
case VIRTIO_RING_F_EVENT_IDX:
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 58c0953..944ebcd 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -119,7 +119,7 @@ struct virtio_device {
struct virtio_config_ops *config;
struct list_head vqs;
/* Note that this is a Linux set_bit-style bitmap. */
- unsigned long features[1];
+ unsigned long features[64 / BITS_PER_LONG];
void *priv;
};
diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
index 800617b..b1a1981 100644
--- a/include/linux/virtio_config.h
+++ b/include/linux/virtio_config.h
@@ -18,16 +18,19 @@
/* We've given up on this device. */
#define VIRTIO_CONFIG_S_FAILED 0x80
-/* Some virtio feature bits (currently bits 28 through 31) are reserved for the
+/* Some virtio feature bits (currently bits 28 through 39) are reserved for the
* transport being used (eg. virtio_ring), the rest are per-device feature
* bits. */
#define VIRTIO_TRANSPORT_F_START 28
-#define VIRTIO_TRANSPORT_F_END 32
+#define VIRTIO_TRANSPORT_F_END 40
/* Do we get callbacks when the ring is completely used, even if we've
* suppressed them? */
#define VIRTIO_F_NOTIFY_ON_EMPTY 24
+/* Enables feature bits 32 to 63 (only really required for virtio_pci). */
+#define VIRTIO_F_FEATURES_HI 31
+
#ifdef __KERNEL__
#include <linux/err.h>
#include <linux/virtio.h>
@@ -72,7 +75,7 @@
* @del_vqs: free virtqueues found by find_vqs().
* @get_features: get the array of feature bits for this device.
* vdev: the virtio_device
- * Returns the first 32 feature bits (all we currently need).
+ * Returns the first 64 feature bits (all we currently need).
* @finalize_features: confirm what device features we'll be using.
* vdev: the virtio_device
* This gives the final feature bits for the device: it can change
@@ -92,7 +95,7 @@ struct virtio_config_ops {
vq_callback_t *callbacks[],
const char *names[]);
void (*del_vqs)(struct virtio_device *);
- u32 (*get_features)(struct virtio_device *vdev);
+ u64 (*get_features)(struct virtio_device *vdev);
void (*finalize_features)(struct virtio_device *vdev);
};
@@ -110,9 +113,9 @@ static inline bool virtio_has_feature(const struct virtio_device *vdev,
{
/* Did you forget to fix assumptions on max features? */
if (__builtin_constant_p(fbit))
- BUILD_BUG_ON(fbit >= 32);
+ BUILD_BUG_ON(fbit >= 64);
else
- BUG_ON(fbit >= 32);
+ BUG_ON(fbit >= 64);
if (fbit < VIRTIO_TRANSPORT_F_START)
virtio_check_driver_offered_feature(vdev, fbit);
diff --git a/include/linux/virtio_pci.h b/include/linux/virtio_pci.h
index 9a3d7c4..90f9725 100644
--- a/include/linux/virtio_pci.h
+++ b/include/linux/virtio_pci.h
@@ -55,9 +55,16 @@
/* Vector value used to disable MSI for queue */
#define VIRTIO_MSI_NO_VECTOR 0xffff
+/* An extended 32-bit r/o bitmask of the features supported by the host */
+#define VIRTIO_PCI_HOST_FEATURES_HI 24
+
+/* An extended 32-bit r/w bitmask of features activated by the guest */
+#define VIRTIO_PCI_GUEST_FEATURES_HI 28
+
/* The remaining space is defined by each driver as the per-driver
* configuration space */
-#define VIRTIO_PCI_CONFIG(dev) ((dev)->msix_enabled ? 24 : 20)
+#define VIRTIO_PCI_CONFIG(dev) ((dev)->features_hi ? 32 : \
+ (dev)->msix_enabled ? 24 : 20)
/* Virtio ABI version, this must match exactly */
#define VIRTIO_PCI_ABI_VERSION 0
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 13/14] virtio_test: update for 64 bit features
From: Michael S. Tsirkin @ 2011-05-19 23:12 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Extend the virtio_test tool so it can work with
64 bit features.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tools/virtio/virtio_test.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 74d3331..96cf9bf 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -55,7 +55,6 @@ void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
{
struct vhost_vring_state state = { .index = info->idx };
struct vhost_vring_file file = { .index = info->idx };
- unsigned long long features = dev->vdev.features[0];
struct vhost_vring_addr addr = {
.index = info->idx,
.desc_user_addr = (uint64_t)(unsigned long)info->vring.desc,
@@ -63,6 +62,10 @@ void vhost_vq_setup(struct vdev_info *dev, struct vq_info *info)
.used_user_addr = (uint64_t)(unsigned long)info->vring.used,
};
int r;
+ unsigned long long features = dev->vdev.features[0];
+ if (sizeof features > sizeof dev->vdev.features[0])
+ features |= ((unsigned long long)dev->vdev.features[1]) << 32;
+
r = ioctl(dev->control, VHOST_SET_FEATURES, &features);
assert(r >= 0);
state.num = info->vring.num;
@@ -107,7 +110,8 @@ static void vdev_info_init(struct vdev_info* dev, unsigned long long features)
int r;
memset(dev, 0, sizeof *dev);
dev->vdev.features[0] = features;
- dev->vdev.features[1] = features >> 32;
+ if (sizeof features > sizeof dev->vdev.features[0])
+ dev->vdev.features[1] = features >> 32;
dev->buf_size = 1024;
dev->buf = malloc(dev->buf_size);
assert(dev->buf);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 14/14] vhost: fix 64 bit features
From: Michael S. Tsirkin @ 2011-05-19 23:12 UTC (permalink / raw)
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
Update vhost_has_feature to make it work correctly for bit > 32.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/vhost.h | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 8e03379..64889d2 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -123,7 +123,7 @@ struct vhost_dev {
struct vhost_memory __rcu *memory;
struct mm_struct *mm;
struct mutex mutex;
- unsigned acked_features;
+ u64 acked_features;
struct vhost_virtqueue *vqs;
int nvqs;
struct file *log_file;
@@ -176,14 +176,14 @@ enum {
(1ULL << VIRTIO_NET_F_MRG_RXBUF),
};
-static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
+static inline bool vhost_has_feature(struct vhost_dev *dev, int bit)
{
- unsigned acked_features;
+ u64 acked_features;
/* TODO: check that we are running from vhost_worker or dev mutex is
* held? */
acked_features = rcu_dereference_index_check(dev->acked_features, 1);
- return acked_features & (1 << bit);
+ return acked_features & (1ull << bit);
}
#endif
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCHv2 00/14] virtio and vhost-net performance enhancements
From: David Miller @ 2011-05-19 23:20 UTC (permalink / raw)
To: mst
Cc: krkumar2, cotte, lguest, xma, kvm, linux-s390, netdev, habanero,
heiko.carstens, linux-kernel, virtualization, steved, borntraeger,
tahm, schwidefsky, linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 20 May 2011 02:10:07 +0300
> Rusty, I think it will be easier to merge vhost and virtio bits in one
> go. Can it all go in through your tree (Dave in the past acked
> sending a very similar patch through you so should not be a problem)?
And in case you want an explicit ack for the net bits:
Acked-by: David S. Miller <davem@davemloft.net>
:-)
^ permalink raw reply
* [PATCHv2 0/2] virtio-net: 64 bit features, event index
From: Michael S. Tsirkin @ 2011-05-19 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
habanero, Heiko Carstens, virtualization, steved,
Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
OK, here's a patch that implements the virtio spec update that I
sent earlier. It supercedes the PUBLISH_USED_IDX patches
I sent out earlier.
Support is added in both userspace and vhost-net.
If you see issues or are just curious, you can
turn the new feature off. For example:
-global virtio-net-pci.event_idx=on
-global virtio-blk-pci.event_idx=off
Also, it's possible to try both vhost-net and virtio-net.
Another part is adding support for 64 bit features in
place. The high bits are actually unused, to test
hack qemu to set some high bit.
linux code is here:
git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost-net-next-event-idx-v3
git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git virtio-net-event-idx-v3
Changes from v1:
- unify used and avail ring handling in a single feature bit
- copy avail event idx fix from vhost-net
Michael S. Tsirkin (2):
virtio/vhost: support 64 bit features
virtio+vhost: event idx feature
hw/qdev-properties.c | 39 +++++++++++++---
hw/qdev.h | 10 ++++
hw/s390-virtio-bus.c | 5 +-
hw/s390-virtio-bus.h | 2 +-
hw/syborg_virtio.c | 7 ++-
hw/vhost_net.c | 14 ++++--
hw/vhost_net.h | 4 +-
hw/virtio-9p.c | 2 +-
hw/virtio-balloon.c | 2 +-
hw/virtio-blk.c | 2 +-
hw/virtio-blk.h | 2 +-
hw/virtio-net.c | 11 +++--
hw/virtio-net.h | 34 +++++++-------
hw/virtio-pci.c | 91 +++++++++++++++++++++++++++----------
hw/virtio-serial-bus.c | 2 +-
hw/virtio.c | 116 ++++++++++++++++++++++++++++++++++++++++++------
hw/virtio.h | 24 +++++++---
17 files changed, 275 insertions(+), 92 deletions(-)
--
1.7.5.53.gc233e
^ permalink raw reply
* [PATCHv2 1/2] virtio/vhost: support 64 bit features
From: Michael S. Tsirkin @ 2011-05-19 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
habanero, Heiko Carstens, virtualization, steved,
Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <cover.1305846929.git.mst@redhat.com>
Add support for extended feature bits: up to 64 bit.
Only virtio-pci is actually implemented,
s390 and syborg are stubbed out (and untested).
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/qdev-properties.c | 39 ++++++++++++++++----
hw/qdev.h | 10 +++++
hw/s390-virtio-bus.c | 5 ++-
hw/s390-virtio-bus.h | 2 +-
hw/syborg_virtio.c | 7 ++--
hw/vhost_net.c | 8 ++--
hw/vhost_net.h | 4 +-
hw/virtio-9p.c | 2 +-
hw/virtio-balloon.c | 2 +-
hw/virtio-blk.c | 2 +-
hw/virtio-blk.h | 2 +-
hw/virtio-net.c | 11 +++---
hw/virtio-net.h | 34 +++++++++---------
hw/virtio-pci.c | 91 +++++++++++++++++++++++++++++++++++-------------
hw/virtio-serial-bus.c | 2 +-
hw/virtio.c | 24 ++++++++++---
hw/virtio.h | 17 +++++----
17 files changed, 179 insertions(+), 83 deletions(-)
diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 1088a26..a933c9e 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -10,20 +10,35 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
return ptr;
}
-static uint32_t qdev_get_prop_mask(Property *prop)
+static uint64_t qdev_get_prop_mask(Property *prop)
{
assert(prop->info->type == PROP_TYPE_BIT);
- return 0x1 << prop->bitnr;
+ return 0x1ULL << prop->bitnr;
+}
+
+static uint64_t qdev_get_prop_val(DeviceState *dev, Property *prop)
+{
+ assert(prop->info->type == PROP_TYPE_BIT);
+ if (prop->info->size == sizeof(uint32_t)) {
+ return *(uint32_t *)qdev_get_prop_ptr(dev, prop);
+ } else {
+ return *(uint64_t *)qdev_get_prop_ptr(dev, prop);
+ }
}
static void bit_prop_set(DeviceState *dev, Property *props, bool val)
{
- uint32_t *p = qdev_get_prop_ptr(dev, props);
- uint32_t mask = qdev_get_prop_mask(props);
+ uint64_t p = qdev_get_prop_val(dev, props);
+ uint64_t mask = qdev_get_prop_mask(props);
if (val)
- *p |= mask;
+ p |= mask;
else
- *p &= ~mask;
+ p &= ~mask;
+ if (props->info->size == sizeof(uint32_t)) {
+ *(uint32_t *)qdev_get_prop_ptr(dev, props) = p;
+ } else {
+ *(uint64_t *)qdev_get_prop_ptr(dev, props) = p;
+ }
}
static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
@@ -51,8 +66,8 @@ static int parse_bit(DeviceState *dev, Property *prop, const char *str)
static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
{
- uint32_t *p = qdev_get_prop_ptr(dev, prop);
- return snprintf(dest, len, (*p & qdev_get_prop_mask(prop)) ? "on" : "off");
+ uint64_t val = qdev_get_prop_val(dev, prop);
+ return snprintf(dest, len, (val & qdev_get_prop_mask(prop)) ? "on" : "off");
}
PropertyInfo qdev_prop_bit = {
@@ -63,6 +78,14 @@ PropertyInfo qdev_prop_bit = {
.print = print_bit,
};
+PropertyInfo qdev_prop_bit64 = {
+ .name = "on/off",
+ .type = PROP_TYPE_BIT,
+ .size = sizeof(uint64_t),
+ .parse = parse_bit,
+ .print = print_bit,
+};
+
/* --- 8bit integer --- */
static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
diff --git a/hw/qdev.h b/hw/qdev.h
index 8a13ec9..e65cab0 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -219,6 +219,7 @@ int do_device_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
/*** qdev-properties.c ***/
extern PropertyInfo qdev_prop_bit;
+extern PropertyInfo qdev_prop_bit64;
extern PropertyInfo qdev_prop_uint8;
extern PropertyInfo qdev_prop_uint16;
extern PropertyInfo qdev_prop_uint32;
@@ -257,6 +258,15 @@ extern PropertyInfo qdev_prop_pci_devfn;
.defval = (bool[]) { (_defval) }, \
}
+#define DEFINE_PROP_BIT64(_name, _state, _field, _bit, _defval) { \
+ .name = (_name), \
+ .info = &(qdev_prop_bit64), \
+ .bitnr = (_bit), \
+ .offset = offsetof(_state, _field) \
+ + type_check(uint64_t,typeof_field(_state, _field)), \
+ .defval = (bool[]) { (_defval) }, \
+ }
+
#define DEFINE_PROP_UINT8(_n, _s, _f, _d) \
DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
#define DEFINE_PROP_UINT16(_n, _s, _f, _d) \
diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c
index 175e5cb..89e8c8e 100644
--- a/hw/s390-virtio-bus.c
+++ b/hw/s390-virtio-bus.c
@@ -310,10 +310,11 @@ static void virtio_s390_notify(void *opaque, uint16_t vector)
kvm_s390_virtio_irq(s390_cpu_addr2state(0), 0, token);
}
-static unsigned virtio_s390_get_features(void *opaque)
+static uint64_t virtio_s390_get_features(void *opaque)
{
VirtIOS390Device *dev = (VirtIOS390Device*)opaque;
- return dev->host_features;
+ /* TODO: support high 32 bit features */
+ return dev->host_features & 0xFFFFFFFFULL;
}
/**************** S390 Virtio Bus Device Descriptions *******************/
diff --git a/hw/s390-virtio-bus.h b/hw/s390-virtio-bus.h
index edf6d04..5c851e3 100644
--- a/hw/s390-virtio-bus.h
+++ b/hw/s390-virtio-bus.h
@@ -43,7 +43,7 @@ typedef struct VirtIOS390Device {
VirtIODevice *vdev;
BlockConf block;
NICConf nic;
- uint32_t host_features;
+ uint64_t host_features;
virtio_serial_conf serial;
virtio_net_conf net;
} VirtIOS390Device;
diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index ee08c49..b64c357 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -67,7 +67,7 @@ typedef struct {
uint32_t int_enable;
uint32_t id;
NICConf nic;
- uint32_t host_features;
+ uint64_t host_features;
virtio_net_conf net;
} SyborgVirtIOProxy;
@@ -244,10 +244,11 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t vector)
qemu_set_irq(proxy->irq, level != 0);
}
-static unsigned syborg_virtio_get_features(void *opaque)
+static uint32_t syborg_virtio_get_features(void *opaque)
{
SyborgVirtIOProxy *proxy = opaque;
- return proxy->host_features;
+ /* TODO: support high 32 bit features */
+ return proxy->host_features & 0xFFFFFFFFULL;
}
static VirtIOBindings syborg_virtio_bindings = {
diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 420e05f..7e94f61 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -41,7 +41,7 @@ struct vhost_net {
VLANClientState *vc;
};
-unsigned vhost_net_get_features(struct vhost_net *net, unsigned features)
+uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
{
/* Clear features not supported by host kernel. */
if (!(net->dev.features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY))) {
@@ -56,7 +56,7 @@ unsigned vhost_net_get_features(struct vhost_net *net, unsigned features)
return features;
}
-void vhost_net_ack_features(struct vhost_net *net, unsigned features)
+void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
{
net->dev.acked_features = net->dev.backend_features;
if (features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) {
@@ -219,11 +219,11 @@ void vhost_net_cleanup(struct vhost_net *net)
{
}
-unsigned vhost_net_get_features(struct vhost_net *net, unsigned features)
+uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
{
return features;
}
-void vhost_net_ack_features(struct vhost_net *net, unsigned features)
+void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
{
}
#endif
diff --git a/hw/vhost_net.h b/hw/vhost_net.h
index 91e40b1..4069258 100644
--- a/hw/vhost_net.h
+++ b/hw/vhost_net.h
@@ -14,7 +14,7 @@ void vhost_net_stop(VHostNetState *net, VirtIODevice *dev);
void vhost_net_cleanup(VHostNetState *net);
-unsigned vhost_net_get_features(VHostNetState *net, unsigned features);
-void vhost_net_ack_features(VHostNetState *net, unsigned features);
+uint64_t vhost_net_get_features(VHostNetState *net, uint64_t features);
+void vhost_net_ack_features(VHostNetState *net, uint64_t features);
#endif
diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..184691a 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -3624,7 +3624,7 @@ static void handle_9p_output(VirtIODevice *vdev, VirtQueue *vq)
free_pdu(s, pdu);
}
-static uint32_t virtio_9p_get_features(VirtIODevice *vdev, uint32_t features)
+static uint64_t virtio_9p_get_features(VirtIODevice *vdev, uint64_t features)
{
features |= 1 << VIRTIO_9P_MOUNT_TAG;
return features;
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index f9add1c..b34adc1 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -195,7 +195,7 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
dev->actual = le32_to_cpu(config.actual);
}
-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-blk.c b/hw/virtio-blk.c
index 91e0394..442ce11 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -466,7 +466,7 @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
}
-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 = to_virtio_blk(vdev);
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index fff46da..61104ea 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -98,7 +98,7 @@ struct virtio_scsi_inhdr
#ifdef __linux__
#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
- DEFINE_PROP_BIT("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true)
+ DEFINE_PROP_BIT64("scsi", _state, _field, VIRTIO_BLK_F_SCSI, true)
#else
#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index 6997e02..c242fd1 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -223,7 +223,7 @@ static int peer_has_ufo(VirtIONet *n)
return n->has_ufo;
}
-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 = to_virtio_net(vdev);
@@ -258,9 +258,9 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
return vhost_net_get_features(tap_get_vhost_net(n->nic->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: */
@@ -273,7 +273,7 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev)
return 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 = to_virtio_net(vdev);
@@ -628,7 +628,8 @@ static ssize_t virtio_net_receive(VLANClientState *nc, const uint8_t *buf, size_
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%" PRIx64,
i, n->mergeable_rx_bufs, offset, size,
guest_hdr_len, host_hdr_len, n->vdev.guest_features);
exit(1);
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 8af9a1c..8f8ac7f 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -169,21 +169,21 @@ struct virtio_net_ctrl_mac {
#define DEFINE_VIRTIO_NET_FEATURES(_state, _field) \
DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
- 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("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_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("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)
#endif
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 5236470..eb86de2 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -64,15 +64,28 @@
/* Config space size */
#define VIRTIO_PCI_CONFIG_NOMSI 20
#define VIRTIO_PCI_CONFIG_MSI 24
-#define VIRTIO_PCI_REGION_SIZE(dev) (msix_present(dev) ? \
- VIRTIO_PCI_CONFIG_MSI : \
- VIRTIO_PCI_CONFIG_NOMSI)
+#define VIRTIO_PCI_CONFIG_HI 32
+/* An extended 32-bit r/o bitmask of the features supported by the host */
+#define VIRTIO_PCI_HOST_FEATURES_HI 24
+
+/* An extended 32-bit r/w bitmask of features activated by the guest */
+#define VIRTIO_PCI_GUEST_FEATURES_HI 28
+
+#define VIRTIO_PCI_REGION_SIZE(proxy) (((proxy)->host_features & \
+ (1ULL << VIRTIO_F_FEATURES_HI)) ? \
+ VIRTIO_PCI_CONFIG_HI : \
+ (msix_present(&(proxy)->pci_dev) ? \
+ VIRTIO_PCI_CONFIG_MSI : \
+ VIRTIO_PCI_CONFIG_NOMSI))
/* The remaining space is defined by each driver as the per-driver
* configuration space */
-#define VIRTIO_PCI_CONFIG(dev) (msix_enabled(dev) ? \
- VIRTIO_PCI_CONFIG_MSI : \
- VIRTIO_PCI_CONFIG_NOMSI)
+#define VIRTIO_PCI_CONFIG(proxy) (((proxy)->vdev->guest_features & \
+ (1ULL << VIRTIO_F_FEATURES_HI)) ? \
+ VIRTIO_PCI_CONFIG_HI : \
+ (msix_enabled(&(proxy)->pci_dev) ? \
+ VIRTIO_PCI_CONFIG_MSI : \
+ VIRTIO_PCI_CONFIG_NOMSI))
/* Virtio ABI version, if we increment this, we break the guest driver. */
#define VIRTIO_PCI_ABI_VERSION 0
@@ -106,7 +119,7 @@ typedef struct {
uint32_t nvectors;
BlockConf block;
NICConf nic;
- uint32_t host_features;
+ uint64_t host_features;
#ifdef CONFIG_LINUX
V9fsConf fsconf;
#endif
@@ -314,11 +327,22 @@ static void virtio_pci_reset(DeviceState *d)
proxy->flags &= ~VIRTIO_PCI_FLAG_BUS_MASTER_BUG;
}
+static inline uint64_t virtio_replace_hi(uint64_t features, uint32_t hi)
+{
+ return (features & 0xffffffffull) | ((uint64_t)hi) << 32;
+}
+
+static inline uint64_t virtio_replace_lo(uint64_t features, uint32_t lo)
+{
+ return (features & 0xffffffff00000000ull) | lo;
+}
+
static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
{
VirtIOPCIProxy *proxy = opaque;
VirtIODevice *vdev = proxy->vdev;
target_phys_addr_t pa;
+ uint64_t f;
switch (addr) {
case VIRTIO_PCI_GUEST_FEATURES:
@@ -329,9 +353,15 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
else
val = 0;
}
+ /* Clearing VIRTIO_F_FEATURES_HI clears high 32 bit. */
+ if (val & (1ULL << VIRTIO_F_FEATURES_HI)) {
+ f = virtio_replace_lo(vdev->guest_features, val);
+ } else {
+ f = val;
+ }
if (vdev->set_features)
- vdev->set_features(vdev, val);
- vdev->guest_features = val;
+ vdev->set_features(vdev, f);
+ vdev->guest_features = f;
break;
case VIRTIO_PCI_QUEUE_PFN:
pa = (target_phys_addr_t)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
@@ -389,6 +419,12 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
val = VIRTIO_NO_VECTOR;
virtio_queue_set_vector(vdev, vdev->queue_sel, val);
break;
+ case VIRTIO_PCI_GUEST_FEATURES_HI:
+ f = virtio_replace_hi(vdev->guest_features, val);
+ if (vdev->set_features)
+ vdev->set_features(vdev, f);
+ vdev->guest_features = f;
+ break;
default:
error_report("%s: unexpected address 0x%x value 0x%x",
__func__, addr, val);
@@ -433,6 +469,11 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
case VIRTIO_MSI_QUEUE_VECTOR:
ret = virtio_queue_vector(vdev, vdev->queue_sel);
break;
+ case VIRTIO_PCI_HOST_FEATURES_HI:
+ ret = proxy->host_features >> 32;
+ break;
+ case VIRTIO_PCI_GUEST_FEATURES_HI:
+ ret = vdev->guest_features >> 32;
default:
break;
}
@@ -443,7 +484,7 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr)
{
VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+ uint32_t config = VIRTIO_PCI_CONFIG(proxy);
addr -= proxy->addr;
if (addr < config)
return virtio_ioport_read(proxy, addr);
@@ -454,7 +495,7 @@ static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr)
static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
{
VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+ uint32_t config = VIRTIO_PCI_CONFIG(proxy);
addr -= proxy->addr;
if (addr < config)
return virtio_ioport_read(proxy, addr);
@@ -465,7 +506,7 @@ static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr)
{
VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+ uint32_t config = VIRTIO_PCI_CONFIG(proxy);
addr -= proxy->addr;
if (addr < config)
return virtio_ioport_read(proxy, addr);
@@ -476,7 +517,7 @@ static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr)
static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val)
{
VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+ uint32_t config = VIRTIO_PCI_CONFIG(proxy);
addr -= proxy->addr;
if (addr < config) {
virtio_ioport_write(proxy, addr, val);
@@ -489,7 +530,7 @@ static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val)
static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val)
{
VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+ uint32_t config = VIRTIO_PCI_CONFIG(proxy);
addr -= proxy->addr;
if (addr < config) {
virtio_ioport_write(proxy, addr, val);
@@ -502,7 +543,7 @@ static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val)
static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
{
VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
+ uint32_t config = VIRTIO_PCI_CONFIG(proxy);
addr -= proxy->addr;
if (addr < config) {
virtio_ioport_write(proxy, addr, val);
@@ -517,7 +558,7 @@ static void virtio_map(PCIDevice *pci_dev, int region_num,
{
VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
VirtIODevice *vdev = proxy->vdev;
- unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
+ unsigned config_len = VIRTIO_PCI_REGION_SIZE(proxy) + vdev->config_len;
proxy->addr = addr;
@@ -551,7 +592,7 @@ static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
msix_write_config(pci_dev, address, val, len);
}
-static unsigned virtio_pci_get_features(void *opaque)
+static uint64_t virtio_pci_get_features(void *opaque)
{
VirtIOPCIProxy *proxy = opaque;
return proxy->host_features;
@@ -788,13 +829,6 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
proxy->pci_dev.config_write = virtio_write_config;
- size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev) + vdev->config_len;
- if (size & (size-1))
- size = 1 << qemu_fls(size);
-
- pci_register_bar(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
- virtio_map);
-
if (!kvm_has_many_ioeventfds()) {
proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
}
@@ -803,6 +837,15 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
proxy->host_features |= 0x1 << VIRTIO_F_BAD_FEATURE;
proxy->host_features = vdev->get_features(vdev, proxy->host_features);
+ if (proxy->host_features & 0xffffffff00000000ull) {
+ proxy->host_features |= 0x1ULL << VIRTIO_F_FEATURES_HI;
+ }
+ size = VIRTIO_PCI_REGION_SIZE(proxy) + vdev->config_len;
+ if (size & (size-1))
+ size = 1 << qemu_fls(size);
+
+ pci_register_bar(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
+ virtio_map);
}
static int virtio_blk_init_pci(PCIDevice *pci_dev)
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 6227379..76acd02 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -461,7 +461,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/virtio.c b/hw/virtio.c
index 31bd9e3..b9acbd4 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -653,6 +653,8 @@ void virtio_notify_config(VirtIODevice *vdev)
void virtio_save(VirtIODevice *vdev, QEMUFile *f)
{
+ uint32_t guest_features_hi = vdev->guest_features >> 32;
+ uint32_t guest_features_lo = vdev->guest_features;
int i;
if (vdev->binding->save_config)
@@ -661,7 +663,10 @@ 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);
+ qemu_put_be32s(f, &guest_features_lo);
+ if (guest_features_lo & (1ULL << VIRTIO_F_FEATURES_HI)) {
+ qemu_put_be32s(f, &guest_features_hi);
+ }
qemu_put_be32(f, vdev->config_len);
qemu_put_buffer(f, vdev->config, vdev->config_len);
@@ -687,8 +692,9 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
int virtio_load(VirtIODevice *vdev, QEMUFile *f)
{
int num, i, ret;
- uint32_t features;
- uint32_t supported_features =
+ uint32_t features_hi, features_lo;
+ uint64_t features;
+ uint64_t supported_features =
vdev->binding->get_features(vdev->binding_opaque);
if (vdev->binding->load_config) {
@@ -700,9 +706,17 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
qemu_get_8s(f, &vdev->status);
qemu_get_8s(f, &vdev->isr);
qemu_get_be16s(f, &vdev->queue_sel);
- qemu_get_be32s(f, &features);
+ qemu_get_be32s(f, &features_lo);
+ if (features_lo & (1ULL << VIRTIO_F_FEATURES_HI)) {
+ qemu_get_be32s(f, &features_hi);
+ } else {
+ features_hi = 0;
+ }
+ features = ((uint64_t)features_hi) << 32 | features_lo;
+
if (features & ~supported_features) {
- error_report("Features 0x%x unsupported. Allowed features: 0x%x",
+ error_report("Features 0x%" PRIx64 " unsupported. "
+ "Allowed features: 0x%" PRIx64,
features, supported_features);
return -1;
}
diff --git a/hw/virtio.h b/hw/virtio.h
index bc72289..9517b97 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -49,6 +49,9 @@
/* A guest should never accept this. It implies negotiation is broken. */
#define VIRTIO_F_BAD_FEATURE 30
+/* Enables feature bits 32 to 63 (only really required for virtio_pci). */
+#define VIRTIO_F_FEATURES_HI 31
+
/* from Linux's linux/virtio_ring.h */
/* This marks a buffer as continuing via the next field. */
@@ -93,7 +96,7 @@ typedef struct {
int (*load_config)(void * opaque, QEMUFile *f);
int (*load_queue)(void * opaque, int n, QEMUFile *f);
int (*load_done)(void * opaque, QEMUFile *f);
- unsigned (*get_features)(void * opaque);
+ uint64_t (*get_features)(void * opaque);
bool (*query_guest_notifiers)(void * opaque);
int (*set_guest_notifiers)(void * opaque, bool assigned);
int (*set_host_notifier)(void * opaque, int n, bool assigned);
@@ -110,14 +113,14 @@ 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;
int nvectors;
- 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);
@@ -209,8 +212,8 @@ void virtio_blk_exit(VirtIODevice *vdev);
void virtio_serial_exit(VirtIODevice *vdev);
#define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
- DEFINE_PROP_BIT("indirect_desc", _state, _field, \
- VIRTIO_RING_F_INDIRECT_DESC, true)
+ DEFINE_PROP_BIT64("indirect_desc", _state, _field, \
+ VIRTIO_RING_F_INDIRECT_DESC, true)
target_phys_addr_t virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
target_phys_addr_t virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
--
1.7.5.53.gc233e
^ permalink raw reply related
* [PATCHv2 2/2] virtio+vhost: event idx feature
From: Michael S. Tsirkin @ 2011-05-19 23:24 UTC (permalink / raw)
To: qemu-devel
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
habanero, Heiko Carstens, virtualization, steved,
Christian Borntraeger, Tom Lendacky, Martin Schwidefsky, linux390
In-Reply-To: <cover.1305846929.git.mst@redhat.com>
Add support for used_event feature, and utilize it to
reduce the number of interrupts and exits for the guest.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/vhost_net.c | 6 ++++
hw/virtio.c | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
hw/virtio.h | 9 +++++-
3 files changed, 97 insertions(+), 10 deletions(-)
diff --git a/hw/vhost_net.c b/hw/vhost_net.c
index 7e94f61..41841d9 100644
--- a/hw/vhost_net.c
+++ b/hw/vhost_net.c
@@ -50,6 +50,9 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
if (!(net->dev.features & (1 << VIRTIO_RING_F_INDIRECT_DESC))) {
features &= ~(1 << VIRTIO_RING_F_INDIRECT_DESC);
}
+ if (!(net->dev.features & (1 << VIRTIO_RING_F_EVENT_IDX))) {
+ features &= ~(1 << VIRTIO_RING_F_EVENT_IDX);
+ }
if (!(net->dev.features & (1 << VIRTIO_NET_F_MRG_RXBUF))) {
features &= ~(1 << VIRTIO_NET_F_MRG_RXBUF);
}
@@ -65,6 +68,9 @@ void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
if (features & (1 << VIRTIO_RING_F_INDIRECT_DESC)) {
net->dev.acked_features |= (1 << VIRTIO_RING_F_INDIRECT_DESC);
}
+ if (features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
+ net->dev.acked_features |= (1 << VIRTIO_RING_F_EVENT_IDX);
+ }
if (features & (1 << VIRTIO_NET_F_MRG_RXBUF)) {
net->dev.acked_features |= (1 << VIRTIO_NET_F_MRG_RXBUF);
}
diff --git a/hw/virtio.c b/hw/virtio.c
index b9acbd4..d51ac93 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -72,7 +72,17 @@ struct VirtQueue
VRing vring;
target_phys_addr_t pa;
uint16_t last_avail_idx;
+ /* Last used index value we have signalled on */
+ uint16_t signalled_used;
+
+ /* Last used index value we have signalled on */
+ bool signalled_used_valid;
+
+ /* Notification enabled? */
+ bool notification;
+
int inuse;
+
uint16_t vector;
void (*handle_output)(VirtIODevice *vdev, VirtQueue *vq);
VirtIODevice *vdev;
@@ -141,6 +151,11 @@ static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
return lduw_phys(pa);
}
+static inline uint16_t vring_used_event(VirtQueue *vq)
+{
+ return vring_avail_ring(vq, vq->vring.num);
+}
+
static inline void vring_used_ring_id(VirtQueue *vq, int i, uint32_t val)
{
target_phys_addr_t pa;
@@ -162,11 +177,11 @@ static uint16_t vring_used_idx(VirtQueue *vq)
return lduw_phys(pa);
}
-static inline void vring_used_idx_increment(VirtQueue *vq, uint16_t val)
+static inline void vring_used_idx_set(VirtQueue *vq, uint16_t val)
{
target_phys_addr_t pa;
pa = vq->vring.used + offsetof(VRingUsed, idx);
- stw_phys(pa, vring_used_idx(vq) + val);
+ stw_phys(pa, val);
}
static inline void vring_used_flags_set_bit(VirtQueue *vq, int mask)
@@ -183,12 +198,26 @@ static inline void vring_used_flags_unset_bit(VirtQueue *vq, int mask)
stw_phys(pa, lduw_phys(pa) & ~mask);
}
+static inline void vring_avail_event(VirtQueue *vq, uint16_t val)
+{
+ target_phys_addr_t pa;
+ if (!vq->notification) {
+ return;
+ }
+ pa = vq->vring.used + offsetof(VRingUsed, ring[vq->vring.num]);
+ stw_phys(pa, val);
+}
+
void virtio_queue_set_notification(VirtQueue *vq, int enable)
{
- if (enable)
+ vq->notification = enable;
+ if (vq->vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
+ vring_avail_event(vq, vring_avail_idx(vq));
+ } else if (enable) {
vring_used_flags_unset_bit(vq, VRING_USED_F_NO_NOTIFY);
- else
+ } else {
vring_used_flags_set_bit(vq, VRING_USED_F_NO_NOTIFY);
+ }
}
int virtio_queue_ready(VirtQueue *vq)
@@ -234,11 +263,16 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
void virtqueue_flush(VirtQueue *vq, unsigned int count)
{
+ uint16_t old, new;
/* Make sure buffer is written before we update index. */
wmb();
trace_virtqueue_flush(vq, count);
- vring_used_idx_increment(vq, count);
+ old = vring_used_idx(vq);
+ new = old + count;
+ vring_used_idx_set(vq, new);
vq->inuse -= count;
+ if (unlikely((int16_t)(new - vq->signalled_used) < (uint16_t)(new - old)))
+ vq->signalled_used_valid = false;
}
void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
@@ -395,6 +429,9 @@ int virtqueue_pop(VirtQueue *vq, VirtQueueElement *elem)
max = vq->vring.num;
i = head = virtqueue_get_head(vq, vq->last_avail_idx++);
+ if (vq->vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) {
+ vring_avail_event(vq, vring_avail_idx(vq));
+ }
if (vring_desc_flags(desc_pa, i) & VRING_DESC_F_INDIRECT) {
if (vring_desc_len(desc_pa, i) % sizeof(VRingDesc)) {
@@ -478,6 +515,9 @@ void virtio_reset(void *opaque)
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;
+ vdev->vq[i].notification = true;
}
}
@@ -629,13 +669,45 @@ void virtio_irq(VirtQueue *vq)
virtio_notify_vector(vq->vdev, vq->vector);
}
-void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
+/* Assuming a given event_idx value from the other size, if
+ * we have just incremented index from old to new_idx,
+ * should we trigger an event? */
+static inline int vring_need_event(uint16_t event, uint16_t new, uint16_t old)
+{
+ /* Note: Xen has similar logic for notification hold-off
+ * in include/xen/interface/io/ring.h with req_event and req_prod
+ * corresponding to event_idx + 1 and new respectively.
+ * Note also that req_event and req_prod in Xen start at 1,
+ * event indexes in virtio start at 0. */
+ return (uint16_t)(new - event - 1) < (uint16_t)(new - old);
+}
+
+static bool vring_notify(VirtIODevice *vdev, VirtQueue *vq)
{
+ uint16_t old, new;
+ bool v;
/* Always notify when queue is empty (when feature acknowledge) */
- if ((vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT) &&
- (!(vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) ||
- (vq->inuse || vring_avail_idx(vq) != vq->last_avail_idx)))
+ if (((vdev->guest_features & (1 << VIRTIO_F_NOTIFY_ON_EMPTY)) &&
+ !vq->inuse && vring_avail_idx(vq) == vq->last_avail_idx)) {
+ return true;
+ }
+
+ if (!(vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX))) {
+ return !(vring_avail_flags(vq) & VRING_AVAIL_F_NO_INTERRUPT);
+ }
+
+ v = vq->signalled_used_valid;
+ vq->signalled_used_valid = true;
+ old = vq->signalled_used;
+ new = vq->signalled_used = vring_used_idx(vq);
+ return !v || vring_need_event(vring_used_event(vq), new, old);
+}
+
+void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
+{
+ if (!vring_notify(vdev, vq)) {
return;
+ }
trace_virtio_notify(vdev, vq);
vdev->isr |= 0x01;
@@ -732,6 +804,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
vdev->vq[i].vring.num = qemu_get_be32(f);
vdev->vq[i].pa = 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) {
uint16_t nheads;
diff --git a/hw/virtio.h b/hw/virtio.h
index 9517b97..3438a91 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -46,6 +46,11 @@
#define VIRTIO_F_NOTIFY_ON_EMPTY 24
/* We support indirect buffer descriptors */
#define VIRTIO_RING_F_INDIRECT_DESC 28
+/* The Guest publishes the used index for which it expects an interrupt
+ * at the end of the avail ring. Host should ignore the avail->flags field. */
+/* The Host publishes the avail index for which it expects a kick
+ * at the end of the used ring. Guest should ignore the used->flags field. */
+#define VIRTIO_RING_F_EVENT_IDX 29
/* A guest should never accept this. It implies negotiation is broken. */
#define VIRTIO_F_BAD_FEATURE 30
@@ -213,7 +218,9 @@ void virtio_serial_exit(VirtIODevice *vdev);
#define DEFINE_VIRTIO_COMMON_FEATURES(_state, _field) \
DEFINE_PROP_BIT64("indirect_desc", _state, _field, \
- VIRTIO_RING_F_INDIRECT_DESC, true)
+ VIRTIO_RING_F_INDIRECT_DESC, true), \
+ DEFINE_PROP_BIT64("event_idx", _state, _field, \
+ VIRTIO_RING_F_EVENT_IDX, true)
target_phys_addr_t virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
target_phys_addr_t virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
--
1.7.5.53.gc233e
^ permalink raw reply related
* Re: [PATCH 14/18] virtio: add api for delayed callbacks
From: Rusty Russell @ 2011-05-20 7:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <20110519072412.GA31253@redhat.com>
On Thu, 19 May 2011 10:24:12 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Mon, May 16, 2011 at 04:43:21PM +0930, Rusty Russell wrote:
> > On Sun, 15 May 2011 15:48:18 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > On Mon, May 09, 2011 at 03:27:33PM +0930, Rusty Russell wrote:
> > > > On Wed, 4 May 2011 23:52:33 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > > Add an API that tells the other side that callbacks
> > > > > should be delayed until a lot of work has been done.
> > > > > Implement using the new used_event feature.
> > > >
> > > > Since you're going to add a capacity query anyway, why not add the
> > > > threshold argument here?
> > >
> > > I thought that if we keep the API kind of generic
> > > there might be more of a chance that future transports
> > > will be able to implement it. For example, with an
> > > old host we can't commit to a specific index.
> >
> > No, it's always a hint anyway: you can be notified before the threshold
> > is reached. But best make it explicit I think.
> >
> > Cheers,
> > Rusty.
>
>
> I tried doing that and remembered the real reason I went for this API:
>
> capacity is limited by descriptor table space, not
> used ring space: each entry in the used ring frees up multiple entries
> in the descriptor ring. Thus the ring can't provide
> callback after capacity is N: capacity is only available
> after we get bufs.
I think this indicates a problem, but I haven't reviewed your patches
except very cursorily. I am slack...
> We could try and make the API pass in the number of freed bufs, however:
> - this is not really what virtio-net cares about (it cares about
> capacity)
Yes, max sg elements and max requests are separate, though in the
current virtio implementation remaining sg <= remaining request slots.
That's why we currently feed back remaining descriptors to the driver,
not the number of request slots.
This implies that the thresholds should refer to descriptor numbers
(ie. wake me when there are this many descriptors freed), not the used
ring at all. Which means we're barking up the wrong tree...
I think this needs more thought.
> - if the driver passes a number > number of outstanding bufs, it will
> never get a callback. So to stay correct the driver will need to
> track number of outstanding requests. The simpler API avoids that.
I think the driver should simply say "wake me when you have this many
descriptors free". And set it during initialization, rather than every
time. The virtio_ring code should handle it from there.
Perhaps that can be done with the current technique, where the
virtio_ring makes an educated guess on when sufficient capacity will be
available...
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCHv2 00/14] virtio and vhost-net performance enhancements
From: Rusty Russell @ 2011-05-20 7:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Krishna Kumar, Carsten Otte, lguest, Shirley Ma, kvm, linux-s390,
netdev, habanero, Heiko Carstens, linux-kernel, virtualization,
steved, Christian Borntraeger, Tom Lendacky, Martin Schwidefsky,
linux390
In-Reply-To: <cover.1305846412.git.mst@redhat.com>
On Fri, 20 May 2011 02:10:07 +0300, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> OK, here is the large patchset that implements the virtio spec update
> that I sent earlier (the spec itself needs a minor update, will send
> that out too next week, but I think we are on the same page here
> already). It supercedes the PUBLISH_USED_IDX patches I sent
> out earlier.
>
> What will follow will be a patchset that actually includes 4 sets of
> patches. I note below their status. Please consider for 2.6.40, at
> least partially. Rusty, do you think it's feasible?
Erk. I'm still unsure that we should be using ring capacity as the
thresholding mechanism, given that *descriptor* exhaustion is what we
actually face.
That said, I will review these thoroughly in 14 hours (Sat morning my
time). Perhaps I can convince myself that it's not a problem, because
it *is* simpler...
> List of patches and what they do:
>
> I) With the first patchset, we change virtio ring notification
> hand-off to work like the one in Xen -
> each side publishes an event index, the other one
> notifies when it reaches that value -
> With the one difference that event index starts at 0,
> same as request index (in xen event index starts at 1).
>
> These are the patches in this set:
> virtio: event index interface
> virtio ring: inline function to check for events
> virtio_ring: support event idx feature
> vhost: support event index
> virtio_test: support event index
>
> Changes in this part of the patchset from v1 - address comments by Rusty et al.
>
> I tested this a lot with virtio net block and with the simulator and esp
> with the simulator it's easy to see drastic performance improvement
> here:
>
> [virtio]# time ./virtio_test
> spurious wakeus: 0x7
>
> real 0m0.169s
> user 0m0.140s
> sys 0m0.019s
> [virtio]# time ./virtio_test --no-event-idx
> spurious wakeus: 0x11
>
> real 0m0.649s
> user 0m0.295s
> sys 0m0.335s
>
> And these patches are mostly unchanged from the very first version,
> changes being almost exclusively code cleanups. So I consider this part
> the most stable, I strongly think these patches should go into 2.6.40.
> One extra reason besides performance is that maintaining
> them out of tree is very painful as guest/host ABI is affected.
>
> II) Second set of patches: new apis and use in virtio_net
> With the indexes in place it becomes possibile to request an event after
> many requests (and not just on the next one as done now). This shall fix
> the TX queue overrun which currently triggers a storm of interrupts.
>
> Another issue I tried to fix is capacity checks in virtio-net,
> there's a new API for that, and on top of that,
> I implemented a patch improving real-time characteristics
> of virtio_net
>
> Thus we get the second patchset:
> virtio: add api for delayed callbacks
> virtio_net: delay TX callbacks
> virtio_ring: Add capacity check API
> virtio_net: fix TX capacity checks using new API
> virtio_net: limit xmit polling
>
> This has some fixes that I posted previously applied,
> but otherwise ideantical to v1. I tried to change API
> for enable_cb_delayed as Rusty suggested but failed to do this.
> I think it's not possible to define cleanly.
>
> These work fine for me, I think they can be merged for 2.6.40
> too but would be nice to hear back from Shirley, Tom, Krishna.
See other mail.
> III) There's also a patch that adds a tweak to virtio ring
> virtio: don't delay avail index update
>
> This seems to help small message sizes where we are constantly draining
> the RX VQ.
This is independent. If someone shows some benchmark improvement I'm
definitely happy to put this in .40, if nothing else.
> I'll need to benchmark this to be able to give any numbers
> with confidence, but I don't see how it can hurt anything.
> Thoughts?
>
> IV) Last part is a set of patches to extend feature bits
> to 64 bit. I tested this by using feature bit 32.
> vhost: fix 64 bit features
> virtio_test: update for 64 bit features
> virtio: 64 bit features
Sweetness, but .41 material at this stage.
Thanks,
Rusty.
^ permalink raw reply
* Re: vmbus driver
From: Christoph Hellwig @ 2011-05-20 12:26 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, virtualization
In-Reply-To: <1305842785-30206-1-git-send-email-kys@microsoft.com>
On Thu, May 19, 2011 at 03:06:25PM -0700, K. Y. Srinivasan wrote:
> A few days ago you applied all the outstanding patches for the Hyper-V
> drivers. With these patches, I have addressed all of the known review
> comments for the vmbus driver (and a lot of comments/issues in other
> drivers as well). I am still hoping I can address
> whatever other issues/comments there might be with the intention to
> get the vmbus driver out of staging in the current window. What is your
> sense in terms of how feasible this is. From my side, I can assure you
> that I will address all legitimate issues in a very timely manner and this
> will not be dependent upon the location of the drivers (staging or
> outside staging). Looking forward to hearing from you.
There's no point in merging it without a user. Make sure either
the network or storage driver is in a good enough shape to move with it,
to make sure the APIs it exports are actually sanely usable.
On the other hand the HV clocksource looks mostly mergeable and doesn't
depend on vmbus. Send a patch to add it to drivers/clocksource to the
maintainer and it should be mergeable with very little remaining
cleanup.
^ permalink raw reply
* Re: vmbus driver
From: Greg KH @ 2011-05-20 13:05 UTC (permalink / raw)
To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, virtualization
In-Reply-To: <1305842785-30206-1-git-send-email-kys@microsoft.com>
On Thu, May 19, 2011 at 03:06:25PM -0700, K. Y. Srinivasan wrote:
>
> Greg,
>
> A few days ago you applied all the outstanding patches for the Hyper-V
> drivers. With these patches, I have addressed all of the known review
> comments for the vmbus driver (and a lot of comments/issues in other
> drivers as well). I am still hoping I can address
> whatever other issues/comments there might be with the intention to
> get the vmbus driver out of staging in the current window. What is your
> sense in terms of how feasible this is. From my side, I can assure you
> that I will address all legitimate issues in a very timely manner and this
> will not be dependent upon the location of the drivers (staging or
> outside staging). Looking forward to hearing from you.
The merge window is closed now, and I'm on the road in asia for about 3
weeks, so doing this, at this point in the development cycle, is going
to be hard.
I'll go review the bus code again after the code is all merged with
Linus, which should take a week or so depending on my schedule, and let
you know what's left to do (I think there still is something wierd with
the way the hv_driver is structured, but I could be wrong.)
In the mean time, I'm sure the block and network driver still need a lot
of work, and merging the bus code doesn't make much sense without them
as a user as that is what people really want to use, so you can continue
to work on them.
thanks,
greg k-h
^ permalink raw reply
* RE: vmbus driver
From: KY Srinivasan @ 2011-05-20 13:12 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110520122649.GA550@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, May 20, 2011 8:27 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: vmbus driver
>
> On Thu, May 19, 2011 at 03:06:25PM -0700, K. Y. Srinivasan wrote:
> > A few days ago you applied all the outstanding patches for the Hyper-V
> > drivers. With these patches, I have addressed all of the known review
> > comments for the vmbus driver (and a lot of comments/issues in other
> > drivers as well). I am still hoping I can address
> > whatever other issues/comments there might be with the intention to
> > get the vmbus driver out of staging in the current window. What is your
> > sense in terms of how feasible this is. From my side, I can assure you
> > that I will address all legitimate issues in a very timely manner and this
> > will not be dependent upon the location of the drivers (staging or
> > outside staging). Looking forward to hearing from you.
>
> There's no point in merging it without a user. Make sure either
> the network or storage driver is in a good enough shape to move with it,
> to make sure the APIs it exports are actually sanely usable.
Well, the util driver that implements a range of other services such as KVP,
time synch, heartbeat etc. is also a client of the vmbus driver (perhaps not in the
same way as the storage and network drivers). I was hoping to
move the util driver out of staging along with the vmbus driver.
On a different note, thanks to the feedback I got from you, Greg and others,
both storage and network drivers are in a much better shape than they ever were.
I will continue to cleanup the storage drivers and I would greatly appreciate your
feedback and review.
>
> On the other hand the HV clocksource looks mostly mergeable and doesn't
> depend on vmbus. Send a patch to add it to drivers/clocksource to the
> maintainer and it should be mergeable with very little remaining
> cleanup.
Agreed, now that the merge window is closed, I will have to wait for a few weeks.
Regards,
K. Y
^ permalink raw reply
* RE: vmbus driver
From: KY Srinivasan @ 2011-05-20 13:21 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110520130527.GB10033@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Friday, May 20, 2011 9:05 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: vmbus driver
>
> On Thu, May 19, 2011 at 03:06:25PM -0700, K. Y. Srinivasan wrote:
> >
> > Greg,
> >
> > A few days ago you applied all the outstanding patches for the Hyper-V
> > drivers. With these patches, I have addressed all of the known review
> > comments for the vmbus driver (and a lot of comments/issues in other
> > drivers as well). I am still hoping I can address
> > whatever other issues/comments there might be with the intention to
> > get the vmbus driver out of staging in the current window. What is your
> > sense in terms of how feasible this is. From my side, I can assure you
> > that I will address all legitimate issues in a very timely manner and this
> > will not be dependent upon the location of the drivers (staging or
> > outside staging). Looking forward to hearing from you.
>
> The merge window is closed now, and I'm on the road in asia for about 3
> weeks, so doing this, at this point in the development cycle, is going
> to be hard.
>
> I'll go review the bus code again after the code is all merged with
> Linus, which should take a week or so depending on my schedule, and let
> you know what's left to do (I think there still is something wierd with
> the way the hv_driver is structured, but I could be wrong.)
Thanks Greg. I look forward to your feedback.
>
> In the mean time, I'm sure the block and network driver still need a lot
> of work, and merging the bus code doesn't make much sense without them
> as a user as that is what people really want to use, so you can continue
> to work on them.
I will continue to cleanup the block and network driver code. As you know the
util driver is also a client of the vmbus driver (as far as the communication with
the host goes). So, it may still make sense to plan for getting the vmbus driver out
of staging along with the util and the timesource driver.
Regards,
K. Y
^ permalink raw reply
* Re: vmbus driver
From: Christoph Hellwig @ 2011-05-20 13:22 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <6E21E5352C11B742B20C142EB499E0481FC2AF@TK5EX14MBXC122.redmond.corp.microsoft.com>
On Fri, May 20, 2011 at 01:12:32PM +0000, KY Srinivasan wrote:
> Well, the util driver that implements a range of other services such as KVP,
> time synch, heartbeat etc. is also a client of the vmbus driver (perhaps not in the
The KVP driver is a different module as far as I can see. But it really
needs a lot of work, as no one should use the ugly connector interface
for new code. The closest equivalent is gennetlink, but I'd like to
understand what it's actually supposed to do in practice.
^ permalink raw reply
* RE: vmbus driver
From: KY Srinivasan @ 2011-05-20 13:36 UTC (permalink / raw)
To: Christoph Hellwig
Cc: devel@linuxdriverproject.org, gregkh@suse.de,
linux-kernel@vger.kernel.org, virtualization@lists.osdl.org
In-Reply-To: <20110520122649.GA550@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, May 20, 2011 8:27 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: vmbus driver
>
> On Thu, May 19, 2011 at 03:06:25PM -0700, K. Y. Srinivasan wrote:
> > A few days ago you applied all the outstanding patches for the Hyper-V
> > drivers. With these patches, I have addressed all of the known review
> > comments for the vmbus driver (and a lot of comments/issues in other
> > drivers as well). I am still hoping I can address
> > whatever other issues/comments there might be with the intention to
> > get the vmbus driver out of staging in the current window. What is your
> > sense in terms of how feasible this is. From my side, I can assure you
> > that I will address all legitimate issues in a very timely manner and this
> > will not be dependent upon the location of the drivers (staging or
> > outside staging). Looking forward to hearing from you.
>
> There's no point in merging it without a user. Make sure either
> the network or storage driver is in a good enough shape to move with it,
> to make sure the APIs it exports are actually sanely usable.
>
> On the other hand the HV clocksource looks mostly mergeable and doesn't
> depend on vmbus. Send a patch to add it to drivers/clocksource to the
> maintainer and it should be mergeable with very little remaining
> cleanup.
I see maintainers for each of the clocksource drivers and I see John Stultz and
Thomas Gleixner listed as the maintainers for Timekeeping. Who should sign-off
on the Hyper-V clocksource.
Regards,
K. Y
^ permalink raw reply
* RE: vmbus driver
From: KY Srinivasan @ 2011-05-20 14:08 UTC (permalink / raw)
To: Christoph Hellwig
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org
In-Reply-To: <20110520132220.GA23390@infradead.org>
> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Friday, May 20, 2011 9:22 AM
> To: KY Srinivasan
> Cc: Christoph Hellwig; gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org
> Subject: Re: vmbus driver
>
> On Fri, May 20, 2011 at 01:12:32PM +0000, KY Srinivasan wrote:
> > Well, the util driver that implements a range of other services such as KVP,
> > time synch, heartbeat etc. is also a client of the vmbus driver (perhaps not in
> the
>
> The KVP driver is a different module as far as I can see. But it really
> needs a lot of work, as no one should use the ugly connector interface
> for new code. The closest equivalent is gennetlink, but I'd like to
> understand what it's actually supposed to do in practice.
Chris,
I wrote the KVP component of the util driver less than a year ago and
This code was reviewed on this list before it was accepted. The KVP (Key Value Pair)
functionality supports host based queries on the guest. The data gathering in
the guest is done in user-mode and the kernel component of KVP is used to
communicate with the host. I am using the connector interface to support
communication between the kernel component and the user-mode daemon.
The KVP functionality is needed to integrate with the Microsoft management
stack on the host.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH] arch/tile: add /proc/tile, /proc/sys/tile, and a sysfs cpu attribute
From: Chris Metcalf @ 2011-05-20 14:26 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linux-kernel, virtualization
In-Reply-To: <201105191722.17685.arnd@arndb.de>
On 5/19/2011 11:22 AM, Arnd Bergmann wrote:
> On Thursday 19 May 2011, Chris Metcalf wrote:
>>>> /proc/tile/board
>>>> Information on part numbers, serial numbers, etc., of the
>>>> hardware that the kernel is executing on
>>>>
>>>> /proc/tile/switch
>>>> The type of control path for the onboard network switch, if any.
>> These two report information about the hardware, not the hypervisor. For
>> example:
>>
>> # cat /proc/tile/board
>> board_part: 402-00002-05
>> board_serial: NBS-5002-00012
>> chip_serial: P62338.01.110
>> chip_revision: A0
>> board_revision: 2.2
>> board_description: Tilera TILExpressPro-64, TILEPro64 processor (866 MHz-capable), 1 10GbE, 6 1GbE
>> # cat /proc/tile/switch
>> control: mdio gbe/0
> I think it's ok to have it below /sys/hypervisor, because the information
> is provided through a hypervisor ABI, even though it describes something
> else. This is more like /sys/firmware, but the boundaries between that
> and /sys/hypervisor are not clearly defined when running virtualized anyway.
I'll create a /sys/hypervisor/board/ and report the attributes there.
>>>> /proc/tile/hardwall
>>>> Information on the set of currently active hardwalls (note that
>>>> the implementation is already present in arch/tile/kernel/hardwall.c;
>>>> this change just enables it)
>> This one is not a hypervisor-related file. It just lists information about
>> the set of Linux hardwalls currently active. Again, it's not primarily
>> intended for programmatic use, but as a diagnostic tool.
> same here, I'd still put it into the hypervisor structure.
Since /proc/tile/hardwall has no connection to the hypervisor whatsoever,
I'm reluctant to put it under /sys/hypervisor.
Perhaps in this case it would be reasonable to just have the hardwall
subsystem put the file in /proc/driver/hardwall, or even /proc/hardwall?
Or I could make the /dev/hardwall char device dump out the ASCII text that
we currently get from /proc/hardwall if you read from it, which is a little
weird but not inconceivable. For example it currently shows things like this:
# cat /proc/tile/hardwall
2x2 1,1 pids: 484@2,1 479@1,1
2x2 0,3 pids:
In this example "2x2 1,1" is a 2x2 grid of cpus starting at grid (x,y)
position (1,1), with task 484 bound to the cpu at (x,y) position (2,1).
--
Chris Metcalf, Tilera Corp.
http://www.tilera.com
^ permalink raw reply
* Re: [PATCH] arch/tile: add /proc/tile, /proc/sys/tile, and a sysfs cpu attribute
From: Arnd Bergmann @ 2011-05-20 14:37 UTC (permalink / raw)
To: Chris Metcalf; +Cc: linux-kernel, virtualization
In-Reply-To: <4DD67A31.90802@tilera.com>
On Friday 20 May 2011 16:26:57 Chris Metcalf wrote:
> >>>> /proc/tile/hardwall
> >>>> Information on the set of currently active hardwalls (note that
> >>>> the implementation is already present in arch/tile/kernel/hardwall.c;
> >>>> this change just enables it)
> >> This one is not a hypervisor-related file. It just lists information about
> >> the set of Linux hardwalls currently active. Again, it's not primarily
> >> intended for programmatic use, but as a diagnostic tool.
> > same here, I'd still put it into the hypervisor structure.
>
> Since /proc/tile/hardwall has no connection to the hypervisor whatsoever,
> I'm reluctant to put it under /sys/hypervisor.
Ah, I see. I didn't notice that it was in the other file. You are
absolutely right, this does not belong into /sys/hypervisor and
fits well into procfs, we just need to find the right place.
> Perhaps in this case it would be reasonable to just have the hardwall
> subsystem put the file in /proc/driver/hardwall, or even /proc/hardwall?
> Or I could make the /dev/hardwall char device dump out the ASCII text that
> we currently get from /proc/hardwall if you read from it, which is a little
> weird but not inconceivable. For example it currently shows things like this:
>
> # cat /proc/tile/hardwall
> 2x2 1,1 pids: 484@2,1 479@1,1
> 2x2 0,3 pids:
>
> In this example "2x2 1,1" is a 2x2 grid of cpus starting at grid (x,y)
> position (1,1), with task 484 bound to the cpu at (x,y) position (2,1).
Any chance you can still restructure the information? I would recommend
making it a first-class procfs member, since the data is really per-task.
You can add a conditional entry to tgid_base_stuff[] in fs/proc/base.c
to make it show up for each pid, and then just have the per-task information
in there to do the lookup the other way round:
# cat /proc/484/hardwall
2x2 1,1 @2,1
# cat /proc/479/hardwall
2x2 1,1 @1,1
Arnd
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox