* [PATCH 1/3] net: maintain namespace isolation between vlan and real device
From: Arnd Bergmann @ 2010-01-30 22:23 UTC (permalink / raw)
To: David Miller
Cc: Herbert Xu, Michael S. Tsirkin, netdev, bridge, linux-kernel,
virtualization, Stephen Hemminger
In-Reply-To: <201001302322.15237.arnd@arndb.de>
In the vlan and macvlan drivers, the start_xmit function forwards
data to the dev_queue_xmit function for another device, which may
potentially belong to a different namespace.
To make sure that classification stays within a single namespace,
this resets the potentially critical fields.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/macvlan.c | 2 +-
include/linux/netdevice.h | 9 +++++++++
net/8021q/vlan_dev.c | 2 +-
net/core/dev.c | 35 +++++++++++++++++++++++++++++++----
4 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index fa0dc51..d32e0bd 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -269,7 +269,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
}
xmit_world:
- skb->dev = vlan->lowerdev;
+ skb_set_dev(skb, vlan->lowerdev);
return dev_queue_xmit(skb);
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 93a32a5..622ba5a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1004,6 +1004,15 @@ static inline bool netdev_uses_dsa_tags(struct net_device *dev)
return 0;
}
+#ifndef CONFIG_NET_NS
+static inline void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
+{
+ skb->dev = dev;
+}
+#else /* CONFIG_NET_NS */
+void skb_set_dev(struct sk_buff *skb, struct net_device *dev);
+#endif
+
static inline bool netdev_uses_trailer_tags(struct net_device *dev)
{
#ifdef CONFIG_NET_DSA_TAG_TRAILER
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index a9e1f17..9e83272 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -322,7 +322,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
}
- skb->dev = vlan_dev_info(dev)->real_dev;
+ skb_set_dev(skb, vlan_dev_info(dev)->real_dev);
len = skb->len;
ret = dev_queue_xmit(skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 2cba5c5..94c1eee 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1448,13 +1448,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
if (skb->len > (dev->mtu + dev->hard_header_len))
return NET_RX_DROP;
- skb_dst_drop(skb);
+ skb_set_dev(skb, dev);
skb->tstamp.tv64 = 0;
skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, dev);
- skb->mark = 0;
- secpath_reset(skb);
- nf_reset(skb);
return netif_rx(skb);
}
EXPORT_SYMBOL_GPL(dev_forward_skb);
@@ -1614,6 +1611,36 @@ static bool dev_can_checksum(struct net_device *dev, struct sk_buff *skb)
return false;
}
+/**
+ * skb_dev_set -- assign a new device to a buffer
+ * @skb: buffer for the new device
+ * @dev: network device
+ *
+ * If an skb is owned by a device already, we have to reset
+ * all data private to the namespace a device belongs to
+ * before assigning it a new device.
+ */
+#ifdef CONFIG_NET_NS
+void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
+{
+ skb_dst_drop(skb);
+ if (skb->dev && !net_eq(dev_net(skb->dev), dev_net(dev))) {
+ secpath_reset(skb);
+ nf_reset(skb);
+ skb_init_secmark(skb);
+ skb->mark = 0;
+ skb->priority = 0;
+ skb->nf_trace = 0;
+ skb->ipvs_property = 0;
+#ifdef CONFIG_NET_SCHED
+ skb->tc_index = 0;
+#endif
+ }
+ skb->dev = dev;
+}
+EXPORT_SYMBOL(skb_set_dev);
+#endif /* CONFIG_NET_NS */
+
/*
* Invalidate hardware checksum when packet is to be mangled, and
* complete checksum manually on outgoing path.
--
1.6.3.3
^ permalink raw reply related
* [PATCH 0/3 v4] macvtap driver
From: Arnd Bergmann @ 2010-01-30 22:22 UTC (permalink / raw)
To: David Miller
Cc: Herbert Xu, Michael S. Tsirkin, netdev, bridge, linux-kernel,
virtualization, Stephen Hemminger
In-Reply-To: <201001271104.20607.arnd@arndb.de>
This is the fourth version of the macvtap driver,
based on the comments I got for the last version
I got a few days ago. Very few changes:
* release netdev in chardev open function so
we can destroy it properly.
* Implement TUNSETSNDBUF
* fix sleeping call in rcu_read_lock
* Fix comment in namespace isolation patch
* Fix small context difference to make it apply
to net-next
I can't really test here while travelling, so please
give it a go if you're interested in this driver.
Arnd
^ permalink raw reply
* Re: virtio: console: Return -EFAULT on copy_xx_user errors, allow larger writes
From: Rusty Russell @ 2010-01-30 4:55 UTC (permalink / raw)
To: Amit Shah; +Cc: virtualization
In-Reply-To: <1264772563-25517-1-git-send-email-amit.shah@redhat.com>
On Sat, 30 Jan 2010 12:12:37 am Amit Shah wrote:
> These updated patches in the series return -EFAULT on copy_xx_user
> errors and also move the copy_from_user into fops_write() instead of it
> being in send_buf. This enables send_buf to just read from kernel
> buffers, making it simpler.
>
> This also allows write()s to write more to the host in one go,
> removingthe 4k limitation. I do limit the writes to 32k at once to not
> put too much pressure on the GFP_KERNEL area.
>
> Both of these were pointed out by Marcelo.
Applied.
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCHv2] virtio: use smp_XX barriers on SMP
From: Rusty Russell @ 2010-01-30 4:52 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization
In-Reply-To: <20100129112718.GC6548@redhat.com>
On Fri, 29 Jan 2010 09:57:18 pm Michael S. Tsirkin wrote:
> On Fri, Jan 29, 2010 at 07:44:59AM +1030, Rusty Russell wrote:
> > These barriers are actually trying to make sure in_use is set in a timely
> > manner (this is debug code). They're bogus AFAICT:
>
> So .. let's just drop these barriers, and replace in_use with an atomic?
Nope, just deleted them. See below.
Thanks!
Rusty.
virtio: remove bogus barriers from DEBUG version of virtio_ring.c
With DEBUG defined, we add an ->in_use flag to detect if the caller
invokes two virtio methods in parallel. The barriers attempt to ensure
timely update of the ->in_use flag.
But they're voodoo: if we need these barriers it implies that the
calling code doesn't have sufficient synchronization to ensure the
code paths aren't invoked at the same time anyway, and we want to
detect it.
Also, adding barriers changes timing, so turning on debug has more
chance of hiding real problems.
Thanks to MST for drawing my attention to this code...
CC: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/virtio/virtio_ring.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -36,10 +36,9 @@
panic("%s:in_use = %i\n", \
(_vq)->vq.name, (_vq)->in_use); \
(_vq)->in_use = __LINE__; \
- mb(); \
} while (0)
#define END_USE(_vq) \
- do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; mb(); } while(0)
+ do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; } while(0)
#else
#define BAD_RING(_vq, fmt, args...) \
do { \
^ permalink raw reply
* Re: [PATCHv2] virtio: use smp_XX barriers on SMP
From: Michael S. Tsirkin @ 2010-01-29 13:48 UTC (permalink / raw)
To: Rusty Russell; +Cc: virtualization
In-Reply-To: <201001290744.59324.rusty@rustcorp.com.au>
On Fri, Jan 29, 2010 at 07:44:59AM +1030, Rusty Russell wrote:
> On Fri, 29 Jan 2010 12:07:08 am Michael S. Tsirkin wrote:
> > On Thu, Jan 28, 2010 at 10:01:09AM +1030, Rusty Russell wrote:
> > > On Thu, 28 Jan 2010 09:12:23 am Michael S. Tsirkin wrote:
> > > > Where possible, we should use SMP barriers which are more lightweight than
> > > > mandatory barriers, because mandatory barriers also control MMIO effects on
> > > > accesses through relaxed memory I/O windows (which virtio does not use)
> > > > (compare specifically smp_rmb and rmb on x86_64).
> > >
> > > Xen had a similar issue, in that UP guests running on SMP hosts need to issue
> > > SMP barriers. Is this not also a requirement for virtio?
> >
> > Of course it is. That's why I have ifdef CONFIG_SMP and use
> > mandatory barriers on UP.
>
> Sorry, this was an off-the-cuff comment. I was concerned that UP barriers
> might be insufficient on some archs. But I can't find any such archs when
> I actually look (at least, x86, s390 and powerpc).
I see. Yes, smp barriers seem in practice to be a subset of mandatory
barriers. Maybe a documentation patch specifying this is in order.
> I've applied your patch, minus the first two substitutions:
>
> @@ -36,10 +54,10 @@
> panic("%s:in_use = %i\n", \
> (_vq)->vq.name, (_vq)->in_use); \
> (_vq)->in_use = __LINE__; \
> - mb(); \
> + virtio_mb(); \
> } while (0)
> #define END_USE(_vq) \
> - do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; mb(); } while(0)
> + do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; virtio_mb(); } while(0)
> #else
> #define BAD_RING(_vq, fmt, args...) \
> do { \
>
> These barriers are actually trying to make sure in_use is set in a timely
> manner (this is debug code). They're bogus AFAICT: if you don't have any
> other synchronization then you're in danger of nesting and you want to know.
> And barriers might change timing too much when DEBUG is defined.
>
> Thanks!
> Rusty.
^ permalink raw reply
* [PATCH 29/31] virtio: console: Add ability to hot-unplug ports
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1264772563-25517-6-git-send-email-amit.shah@redhat.com>
Remove port data; deregister from the hvc core if it's a console port.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 65 ++++++++++++++++++++++++++++++++++++++-
include/linux/virtio_console.h | 1 +
2 files changed, 64 insertions(+), 2 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6d783b7..8241bb7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -790,6 +790,36 @@ static struct attribute_group port_attribute_group = {
.attrs = port_sysfs_entries,
};
+/* Remove all port-specific data. */
+static int remove_port(struct port *port)
+{
+ spin_lock_irq(&port->portdev->ports_lock);
+ list_del(&port->list);
+ spin_unlock_irq(&port->portdev->ports_lock);
+
+ if (is_console_port(port)) {
+ spin_lock_irq(&pdrvdata_lock);
+ list_del(&port->cons.list);
+ spin_unlock_irq(&pdrvdata_lock);
+ hvc_remove(port->cons.hvc);
+ }
+ if (port->guest_connected)
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
+ while (port->in_vq->vq_ops->detach_unused_buf(port->in_vq))
+ ;
+
+ sysfs_remove_group(&port->dev->kobj, &port_attribute_group);
+ device_destroy(pdrvdata.class, port->dev->devt);
+ cdev_del(&port->cdev);
+
+ discard_port_data(port);
+ kfree(port->name);
+
+ kfree(port);
+ return 0;
+}
+
/* Any private messages that the Host and Guest want to share */
static void handle_control_message(struct ports_device *portdev,
struct port_buffer *buf)
@@ -861,6 +891,32 @@ static void handle_control_message(struct ports_device *portdev,
err);
break;
+ case VIRTIO_CONSOLE_PORT_REMOVE:
+ /*
+ * Hot unplug the port. We don't decrement nr_ports
+ * since we don't want to deal with extra complexities
+ * of using the lowest-available port id: We can just
+ * pick up the nr_ports number as the id and not have
+ * userspace send it to us. This helps us in two
+ * ways:
+ *
+ * - We don't need to have a 'port_id' field in the
+ * config space when a port is hot-added. This is a
+ * good thing as we might queue up multiple hotplug
+ * requests issued in our workqueue.
+ *
+ * - Another way to deal with this would have been to
+ * use a bitmap of the active ports and select the
+ * lowest non-active port from that map. That
+ * bloats the already tight config space and we
+ * would end up artificially limiting the
+ * max. number of ports to sizeof(bitmap). Right
+ * now we can support 2^32 ports (as the port id is
+ * stored in a u32 type).
+ *
+ */
+ remove_port(port);
+ break;
}
}
@@ -1085,12 +1141,17 @@ static void config_work_handler(struct work_struct *work)
/*
* Port 0 got hot-added. Since we already did all the
* other initialisation for it, just tell the Host
- * that the port is ready.
+ * that the port is ready if we find the port. In
+ * case the port was hot-removed earlier, we call
+ * add_port to add the port.
*/
struct port *port;
port = find_port_by_id(portdev, 0);
- send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
+ if (!port)
+ add_port(portdev, 0);
+ else
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
return;
}
if (virtconconf.nr_ports > portdev->config.max_nr_ports) {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index 13e56c8..dd08675 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -41,6 +41,7 @@ struct virtio_console_control {
#define VIRTIO_CONSOLE_RESIZE 2
#define VIRTIO_CONSOLE_PORT_OPEN 3
#define VIRTIO_CONSOLE_PORT_NAME 4
+#define VIRTIO_CONSOLE_PORT_REMOVE 5
#ifdef __KERNEL__
int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
--
1.6.2.5
^ permalink raw reply related
* [PATCH 23/31] virtio: console: Add file operations to ports for open/read/write/poll
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1264772563-25517-5-git-send-email-amit.shah@redhat.com>
Allow guest userspace applications to open, read from, write to, poll
the ports via the char dev interface.
When a port gets opened, a notification is sent to the host via a
control message indicating a connection has been established. Similarly,
on closing of the port, a notification is sent indicating disconnection.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 164 +++++++++++++++++++++++++++++++++++++++-
include/linux/virtio_console.h | 1 +
2 files changed, 164 insertions(+), 1 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6aa400e..8d23c41 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -19,11 +19,15 @@
#include <linux/cdev.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/fs.h>
#include <linux/init.h>
#include <linux/list.h>
+#include <linux/poll.h>
+#include <linux/sched.h>
#include <linux/spinlock.h>
#include <linux/virtio.h>
#include <linux/virtio_console.h>
+#include <linux/wait.h>
#include <linux/workqueue.h>
#include "hvc_console.h"
@@ -166,8 +170,14 @@ struct port {
struct cdev cdev;
struct device *dev;
+ /* A waitqueue for poll() or blocking read operations */
+ wait_queue_head_t waitqueue;
+
/* The 'id' to identify the port with the Host */
u32 id;
+
+ /* Is the host device open */
+ bool host_connected;
};
/* This is the very early arch-specified put chars function. */
@@ -424,6 +434,146 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
return out_count;
}
+/* The condition that must be true for polling to end */
+static bool wait_is_over(struct port *port)
+{
+ return port_has_data(port) || !port->host_connected;
+}
+
+static ssize_t port_fops_read(struct file *filp, char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct port *port;
+ ssize_t ret;
+
+ port = filp->private_data;
+
+ if (!port_has_data(port)) {
+ /*
+ * If nothing's connected on the host just return 0 in
+ * case of list_empty; this tells the userspace app
+ * that there's no connection
+ */
+ if (!port->host_connected)
+ return 0;
+ if (filp->f_flags & O_NONBLOCK)
+ return -EAGAIN;
+
+ ret = wait_event_interruptible(port->waitqueue,
+ wait_is_over(port));
+ if (ret < 0)
+ return ret;
+ }
+ /*
+ * We could've received a disconnection message while we were
+ * waiting for more data.
+ *
+ * This check is not clubbed in the if() statement above as we
+ * might receive some data as well as the host could get
+ * disconnected after we got woken up from our wait. So we
+ * really want to give off whatever data we have and only then
+ * check for host_connected.
+ */
+ if (!port_has_data(port) && !port->host_connected)
+ return 0;
+
+ return fill_readbuf(port, ubuf, count, true);
+}
+
+static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
+ size_t count, loff_t *offp)
+{
+ struct port *port;
+ char *buf;
+ ssize_t ret;
+
+ port = filp->private_data;
+
+ count = min((size_t)(32 * 1024), count);
+
+ buf = kmalloc(count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = copy_from_user(buf, ubuf, count);
+ if (ret) {
+ ret = -EFAULT;
+ goto free_buf;
+ }
+
+ ret = send_buf(port, buf, count);
+free_buf:
+ kfree(buf);
+ return ret;
+}
+
+static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
+{
+ struct port *port;
+ unsigned int ret;
+
+ port = filp->private_data;
+ poll_wait(filp, &port->waitqueue, wait);
+
+ ret = 0;
+ if (port->inbuf)
+ ret |= POLLIN | POLLRDNORM;
+ if (port->host_connected)
+ ret |= POLLOUT;
+ if (!port->host_connected)
+ ret |= POLLHUP;
+
+ return ret;
+}
+
+static int port_fops_release(struct inode *inode, struct file *filp)
+{
+ struct port *port;
+
+ port = filp->private_data;
+
+ /* Notify host of port being closed */
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 0);
+
+ return 0;
+}
+
+static int port_fops_open(struct inode *inode, struct file *filp)
+{
+ struct cdev *cdev = inode->i_cdev;
+ struct port *port;
+
+ port = container_of(cdev, struct port, cdev);
+ filp->private_data = port;
+
+ /*
+ * Don't allow opening of console port devices -- that's done
+ * via /dev/hvc
+ */
+ if (is_console_port(port))
+ return -ENXIO;
+
+ /* Notify host of port being opened */
+ send_control_msg(filp->private_data, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
+ return 0;
+}
+
+/*
+ * The file operations that we support: programs in the guest can open
+ * a console device, read from it, write to it, poll for data and
+ * close it. The devices are at
+ * /dev/vport<device number>p<port number>
+ */
+static const struct file_operations port_fops = {
+ .owner = THIS_MODULE,
+ .open = port_fops_open,
+ .read = port_fops_read,
+ .write = port_fops_write,
+ .poll = port_fops_poll,
+ .release = port_fops_release,
+};
+
/*
* The put_chars() callback is pretty straightforward.
*
@@ -567,6 +717,9 @@ int init_port_console(struct port *port)
list_add_tail(&port->cons.list, &pdrvdata.consoles);
spin_unlock_irq(&pdrvdata_lock);
+ /* Notify host of port being opened */
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_OPEN, 1);
+
return 0;
}
@@ -606,6 +759,10 @@ static void handle_control_message(struct ports_device *portdev,
port->cons.hvc->irq_requested = 1;
resize_console(port);
break;
+ case VIRTIO_CONSOLE_PORT_OPEN:
+ port->host_connected = cpkt->value;
+ wake_up_interruptible(&port->waitqueue);
+ break;
}
}
@@ -652,6 +809,8 @@ static void in_intr(struct virtqueue *vq)
spin_unlock_irqrestore(&port->inbuf_lock, flags);
+ wake_up_interruptible(&port->waitqueue);
+
if (is_console_port(port) && hvc_poll(port->cons.hvc))
hvc_kick();
}
@@ -704,10 +863,12 @@ static int add_port(struct ports_device *portdev, u32 id)
port->inbuf = NULL;
port->cons.hvc = NULL;
+ port->host_connected = false;
+
port->in_vq = portdev->in_vqs[port->id];
port->out_vq = portdev->out_vqs[port->id];
- cdev_init(&port->cdev, NULL);
+ cdev_init(&port->cdev, &port_fops);
devt = MKDEV(portdev->chr_major, id);
err = cdev_add(&port->cdev, devt, 1);
@@ -728,6 +889,7 @@ static int add_port(struct ports_device *portdev, u32 id)
}
spin_lock_init(&port->inbuf_lock);
+ init_waitqueue_head(&port->waitqueue);
inbuf = alloc_buf(PAGE_SIZE);
if (!inbuf) {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index cada769..6a76a44 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -39,6 +39,7 @@ struct virtio_console_control {
#define VIRTIO_CONSOLE_PORT_READY 0
#define VIRTIO_CONSOLE_CONSOLE_PORT 1
#define VIRTIO_CONSOLE_RESIZE 2
+#define VIRTIO_CONSOLE_PORT_OPEN 3
#ifdef __KERNEL__
int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
--
1.6.2.5
^ permalink raw reply related
* [PATCH 22/31] virtio: console: Associate each port with a char device
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1264772563-25517-4-git-send-email-amit.shah@redhat.com>
The char device will be used as an interface by applications on the
guest to communicate with apps on the host.
The devices created are placed in /dev/vportNpn where N is the
virtio-console device number and n is the port number for that device.
One dynamic major device number is allocated for each device and minor
numbers are allocated for the ports contained within that device.
The file operation for the char devs will be added in the following
commits.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/Kconfig | 8 ++++
drivers/char/virtio_console.c | 77 +++++++++++++++++++++++++++++++++++++++--
2 files changed, 82 insertions(+), 3 deletions(-)
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 31be3ac..bcedee1 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -666,6 +666,14 @@ config VIRTIO_CONSOLE
help
Virtio console for use with lguest and other hypervisors.
+ Also serves as a general-purpose serial device for data
+ transfer between the guest and host. Character devices at
+ /dev/vportNpn will be created when corresponding ports are
+ found, where N is the device number and n is the port number
+ within that device. If specified by the host, a sysfs
+ attribute called 'name' will be populated with a name for
+ the port which can be used by udev scripts to create a
+ symlink to the device.
config HVCS
tristate "IBM Hypervisor Virtual Console Server support"
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 5f61021..6aa400e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -16,6 +16,8 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#include <linux/cdev.h>
+#include <linux/device.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/list.h>
@@ -34,6 +36,12 @@
* across multiple devices and multiple ports per device.
*/
struct ports_driver_data {
+ /* Used for registering chardevs */
+ struct class *class;
+
+ /* Number of devices this driver is handling */
+ unsigned int index;
+
/*
* This is used to keep track of the number of hvc consoles
* spawned by this driver. This number is given as the first
@@ -119,6 +127,12 @@ struct ports_device {
/* The control messages to the Host are sent via this buffer */
struct port_buffer *outbuf;
+
+ /* Used for numbering devices for sysfs and debugfs */
+ unsigned int drv_index;
+
+ /* Major number for this device. Ports will be created as minors. */
+ int chr_major;
};
/* This struct holds the per-port data */
@@ -148,6 +162,10 @@ struct port {
*/
struct console cons;
+ /* Each port associates with a separate char device */
+ struct cdev cdev;
+ struct device *dev;
+
/* The 'id' to identify the port with the Host */
u32 id;
};
@@ -398,7 +416,7 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
port->inbuf = NULL;
if (add_inbuf(port->in_vq, buf) < 0)
- dev_warn(&port->portdev->vdev->dev, "failed add_buf\n");
+ dev_warn(port->dev, "failed add_buf\n");
spin_unlock_irqrestore(&port->inbuf_lock, flags);
}
@@ -671,6 +689,7 @@ static int add_port(struct ports_device *portdev, u32 id)
{
struct port *port;
struct port_buffer *inbuf;
+ dev_t devt;
int err;
port = kmalloc(sizeof(*port), GFP_KERNEL);
@@ -688,12 +707,32 @@ static int add_port(struct ports_device *portdev, u32 id)
port->in_vq = portdev->in_vqs[port->id];
port->out_vq = portdev->out_vqs[port->id];
+ cdev_init(&port->cdev, NULL);
+
+ devt = MKDEV(portdev->chr_major, id);
+ err = cdev_add(&port->cdev, devt, 1);
+ if (err < 0) {
+ dev_err(&port->portdev->vdev->dev,
+ "Error %d adding cdev for port %u\n", err, id);
+ goto free_port;
+ }
+ port->dev = device_create(pdrvdata.class, &port->portdev->vdev->dev,
+ devt, port, "vport%up%u",
+ port->portdev->drv_index, id);
+ if (IS_ERR(port->dev)) {
+ err = PTR_ERR(port->dev);
+ dev_err(&port->portdev->vdev->dev,
+ "Error %d creating device for port %u\n",
+ err, id);
+ goto free_cdev;
+ }
+
spin_lock_init(&port->inbuf_lock);
inbuf = alloc_buf(PAGE_SIZE);
if (!inbuf) {
err = -ENOMEM;
- goto free_port;
+ goto free_device;
}
/* Register the input buffer the first time. */
@@ -723,6 +762,10 @@ static int add_port(struct ports_device *portdev, u32 id)
free_inbuf:
free_buf(inbuf);
+free_device:
+ device_destroy(pdrvdata.class, port->dev->devt);
+free_cdev:
+ cdev_del(&port->cdev);
free_port:
kfree(port);
fail:
@@ -835,6 +878,10 @@ fail:
return err;
}
+static const struct file_operations portdev_fops = {
+ .owner = THIS_MODULE,
+};
+
/*
* Once we're further in boot, we get probed like any other virtio
* device.
@@ -860,6 +907,20 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
portdev->vdev = vdev;
vdev->priv = portdev;
+ spin_lock_irq(&pdrvdata_lock);
+ portdev->drv_index = pdrvdata.index++;
+ spin_unlock_irq(&pdrvdata_lock);
+
+ portdev->chr_major = register_chrdev(0, "virtio-portsdev",
+ &portdev_fops);
+ if (portdev->chr_major < 0) {
+ dev_err(&vdev->dev,
+ "Error %d registering chrdev for device %u\n",
+ portdev->chr_major, portdev->drv_index);
+ err = portdev->chr_major;
+ goto free;
+ }
+
multiport = false;
portdev->config.nr_ports = 1;
portdev->config.max_nr_ports = 1;
@@ -892,7 +953,7 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
err = init_vqs(portdev);
if (err < 0) {
dev_err(&vdev->dev, "Error %d initializing vqs\n", err);
- goto free;
+ goto free_chrdev;
}
spin_lock_init(&portdev->ports_lock);
@@ -922,6 +983,8 @@ free_vqs:
vdev->config->del_vqs(vdev);
kfree(portdev->in_vqs);
kfree(portdev->out_vqs);
+free_chrdev:
+ unregister_chrdev(portdev->chr_major, "virtio-portsdev");
free:
kfree(portdev);
fail:
@@ -950,6 +1013,14 @@ static struct virtio_driver virtio_console = {
static int __init init(void)
{
+ int err;
+
+ pdrvdata.class = class_create(THIS_MODULE, "virtio-ports");
+ if (IS_ERR(pdrvdata.class)) {
+ err = PTR_ERR(pdrvdata.class);
+ pr_err("Error %d creating virtio-ports class\n", err);
+ return err;
+ }
INIT_LIST_HEAD(&pdrvdata.consoles);
return register_virtio_driver(&virtio_console);
--
1.6.2.5
^ permalink raw reply related
* [PATCH 21/31] virtio: console: Prepare for writing to userspace buffers
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1264772563-25517-3-git-send-email-amit.shah@redhat.com>
When ports get advertised as char devices, the buffers will come from
userspace. Equip the fill_readbuf function with the ability to write
to userspace buffers.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 20 ++++++++++++++------
1 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 9d33239..5f61021 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -365,7 +365,8 @@ fail:
* Give out the data that's requested from the buffer that we have
* queued up.
*/
-static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
+static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
+ bool to_user)
{
struct port_buffer *buf;
unsigned long flags;
@@ -374,12 +375,18 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
return 0;
buf = port->inbuf;
- if (out_count > buf->len - buf->offset)
- out_count = buf->len - buf->offset;
+ out_count = min(out_count, buf->len - buf->offset);
- memcpy(out_buf, buf->buf + buf->offset, out_count);
+ if (to_user) {
+ ssize_t ret;
+
+ ret = copy_to_user(out_buf, buf->buf + buf->offset, out_count);
+ if (ret)
+ return -EFAULT;
+ } else {
+ memcpy(out_buf, buf->buf + buf->offset, out_count);
+ }
- /* Return the number of bytes actually copied */
buf->offset += out_count;
if (buf->offset == buf->len) {
@@ -395,6 +402,7 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
spin_unlock_irqrestore(&port->inbuf_lock, flags);
}
+ /* Return the number of bytes actually copied */
return out_count;
}
@@ -438,7 +446,7 @@ static int get_chars(u32 vtermno, char *buf, int count)
/* If we don't have an input queue yet, we can't get input. */
BUG_ON(!port->in_vq);
- return fill_readbuf(port, buf, count);
+ return fill_readbuf(port, buf, count, false);
}
static void resize_console(struct port *port)
--
1.6.2.5
^ permalink raw reply related
* [PATCH 20/31] virtio: console: Add a new MULTIPORT feature, support for generic ports
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1264772563-25517-2-git-send-email-amit.shah@redhat.com>
This commit adds a new feature, MULTIPORT. If the host supports this
feature as well, the config space has the number of ports defined for
that device. New ports are spawned according to this information.
The config space also has the maximum number of ports that can be
spawned for a particular device. This is useful in initializing the
appropriate number of virtqueues in advance, as ports might be
hot-plugged in later.
Using this feature, generic ports can be created which are not tied to
hvc consoles.
We also open up a private channel between the host and the guest via
which some "control" messages are exchanged for the ports, like whether
the port being spawned is a console port, resizing the console window,
etc.
Next commits will add support for hotplugging and presenting char
devices in /dev/ for bi-directional guest-host communication.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 405 ++++++++++++++++++++++++++++++++++------
include/linux/virtio_console.h | 21 ++
2 files changed, 370 insertions(+), 56 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d010510..9d33239 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1,5 +1,6 @@
/*
* Copyright (C) 2006, 2007, 2009 Rusty Russell, IBM Corporation
+ * Copyright (C) 2009, Red Hat, Inc.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -21,6 +22,7 @@
#include <linux/spinlock.h>
#include <linux/virtio.h>
#include <linux/virtio_console.h>
+#include <linux/workqueue.h>
#include "hvc_console.h"
/*
@@ -69,17 +71,6 @@ struct console {
u32 vtermno;
};
-/*
- * This is a per-device struct that stores data common to all the
- * ports for that device (vdev->priv).
- */
-struct ports_device {
- /* Array of per-port IO virtqueues */
- struct virtqueue **in_vqs, **out_vqs;
-
- struct virtio_device *vdev;
-};
-
struct port_buffer {
char *buf;
@@ -92,8 +83,49 @@ struct port_buffer {
size_t offset;
};
+/*
+ * This is a per-device struct that stores data common to all the
+ * ports for that device (vdev->priv).
+ */
+struct ports_device {
+ /*
+ * Workqueue handlers where we process deferred work after
+ * notification
+ */
+ struct work_struct control_work;
+
+ struct list_head ports;
+
+ /* To protect the list of ports */
+ spinlock_t ports_lock;
+
+ /* To protect the vq operations for the control channel */
+ spinlock_t cvq_lock;
+
+ /* The current config space is stored here */
+ struct virtio_console_config config;
+
+ /* The virtio device we're associated with */
+ struct virtio_device *vdev;
+
+ /*
+ * A couple of virtqueues for the control channel: one for
+ * guest->host transfers, one for host->guest transfers
+ */
+ struct virtqueue *c_ivq, *c_ovq;
+
+ /* Array of per-port IO virtqueues */
+ struct virtqueue **in_vqs, **out_vqs;
+
+ /* The control messages to the Host are sent via this buffer */
+ struct port_buffer *outbuf;
+};
+
/* This struct holds the per-port data */
struct port {
+ /* Next port in the list, head is in the ports_device */
+ struct list_head list;
+
/* Pointer to the parent virtio_console device */
struct ports_device *portdev;
@@ -115,6 +147,9 @@ struct port {
* hooked up to an hvc console
*/
struct console cons;
+
+ /* The 'id' to identify the port with the Host */
+ u32 id;
};
/* This is the very early arch-specified put chars function. */
@@ -139,25 +174,56 @@ out:
return port;
}
+static struct port *find_port_by_id(struct ports_device *portdev, u32 id)
+{
+ struct port *port;
+ unsigned long flags;
+
+ spin_lock_irqsave(&portdev->ports_lock, flags);
+ list_for_each_entry(port, &portdev->ports, list)
+ if (port->id == id)
+ goto out;
+ port = NULL;
+out:
+ spin_unlock_irqrestore(&portdev->ports_lock, flags);
+
+ return port;
+}
+
static struct port *find_port_by_vq(struct ports_device *portdev,
struct virtqueue *vq)
{
struct port *port;
- struct console *cons;
unsigned long flags;
- spin_lock_irqsave(&pdrvdata_lock, flags);
- list_for_each_entry(cons, &pdrvdata.consoles, list) {
- port = container_of(cons, struct port, cons);
+ spin_lock_irqsave(&portdev->ports_lock, flags);
+ list_for_each_entry(port, &portdev->ports, list)
if (port->in_vq == vq || port->out_vq == vq)
goto out;
- }
port = NULL;
out:
- spin_unlock_irqrestore(&pdrvdata_lock, flags);
+ spin_unlock_irqrestore(&portdev->ports_lock, flags);
return port;
}
+static bool is_console_port(struct port *port)
+{
+ if (port->cons.hvc)
+ return true;
+ return false;
+}
+
+static inline bool use_multiport(struct ports_device *portdev)
+{
+ /*
+ * This condition can be true when put_chars is called from
+ * early_init
+ */
+ if (!portdev->vdev)
+ return 0;
+ return portdev->vdev->features[0] & (1 << VIRTIO_CONSOLE_F_MULTIPORT);
+}
+
static void free_buf(struct port_buffer *buf)
{
kfree(buf->buf);
@@ -233,6 +299,36 @@ static bool port_has_data(struct port *port)
return ret;
}
+static ssize_t send_control_msg(struct port *port, unsigned int event,
+ unsigned int value)
+{
+ struct scatterlist sg[1];
+ struct virtio_console_control cpkt;
+ struct virtqueue *vq;
+ struct port_buffer *outbuf;
+ int tmplen;
+
+ if (!use_multiport(port->portdev))
+ return 0;
+
+ cpkt.id = port->id;
+ cpkt.event = event;
+ cpkt.value = value;
+
+ vq = port->portdev->c_ovq;
+ outbuf = port->portdev->outbuf;
+
+ memcpy(outbuf->buf, (void *)&cpkt, sizeof(cpkt));
+
+ sg_init_one(sg, outbuf->buf, sizeof(cpkt));
+ if (vq->vq_ops->add_buf(vq, sg, 1, 0, outbuf) >= 0) {
+ vq->vq_ops->kick(vq);
+ while (!vq->vq_ops->get_buf(vq, &tmplen))
+ cpu_relax();
+ }
+ return 0;
+}
+
static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
{
struct scatterlist sg[1];
@@ -387,24 +483,7 @@ static void notifier_del_vio(struct hvc_struct *hp, int data)
hp->irq_requested = 0;
}
-static void hvc_handle_input(struct virtqueue *vq)
-{
- struct port *port;
- unsigned long flags;
-
- port = find_port_by_vq(vq->vdev->priv, vq);
- if (!port)
- return;
-
- spin_lock_irqsave(&port->inbuf_lock, flags);
- port->inbuf = get_inbuf(port);
- spin_unlock_irqrestore(&port->inbuf_lock, flags);
-
- if (hvc_poll(port->cons.hvc))
- hvc_kick();
-}
-
-/* The operations for the console. */
+/* The operations for console ports. */
static const struct hv_ops hv_ops = {
.get_chars = get_chars,
.put_chars = put_chars,
@@ -428,7 +507,7 @@ int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int))
return hvc_instantiate(0, 0, &hv_ops);
}
-int __devinit init_port_console(struct port *port)
+int init_port_console(struct port *port)
{
int ret;
@@ -465,7 +544,122 @@ int __devinit init_port_console(struct port *port)
return 0;
}
-static int __devinit add_port(struct ports_device *portdev)
+/* Any private messages that the Host and Guest want to share */
+static void handle_control_message(struct ports_device *portdev,
+ struct port_buffer *buf)
+{
+ struct virtio_console_control *cpkt;
+ struct port *port;
+
+ cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
+
+ port = find_port_by_id(portdev, cpkt->id);
+ if (!port) {
+ /* No valid header at start of buffer. Drop it. */
+ dev_dbg(&portdev->vdev->dev,
+ "Invalid index %u in control packet\n", cpkt->id);
+ return;
+ }
+
+ switch (cpkt->event) {
+ case VIRTIO_CONSOLE_CONSOLE_PORT:
+ if (!cpkt->value)
+ break;
+ if (is_console_port(port))
+ break;
+
+ init_port_console(port);
+ /*
+ * Could remove the port here in case init fails - but
+ * have to notify the host first.
+ */
+ break;
+ case VIRTIO_CONSOLE_RESIZE:
+ if (!is_console_port(port))
+ break;
+ port->cons.hvc->irq_requested = 1;
+ resize_console(port);
+ break;
+ }
+}
+
+static void control_work_handler(struct work_struct *work)
+{
+ struct ports_device *portdev;
+ struct virtqueue *vq;
+ struct port_buffer *buf;
+ unsigned int len;
+
+ portdev = container_of(work, struct ports_device, control_work);
+ vq = portdev->c_ivq;
+
+ spin_lock(&portdev->cvq_lock);
+ while ((buf = vq->vq_ops->get_buf(vq, &len))) {
+ spin_unlock(&portdev->cvq_lock);
+
+ buf->len = len;
+ buf->offset = 0;
+
+ handle_control_message(portdev, buf);
+
+ spin_lock(&portdev->cvq_lock);
+ if (add_inbuf(portdev->c_ivq, buf) < 0) {
+ dev_warn(&portdev->vdev->dev,
+ "Error adding buffer to queue\n");
+ free_buf(buf);
+ }
+ }
+ spin_unlock(&portdev->cvq_lock);
+}
+
+static void in_intr(struct virtqueue *vq)
+{
+ struct port *port;
+ unsigned long flags;
+
+ port = find_port_by_vq(vq->vdev->priv, vq);
+ if (!port)
+ return;
+
+ spin_lock_irqsave(&port->inbuf_lock, flags);
+ port->inbuf = get_inbuf(port);
+
+ spin_unlock_irqrestore(&port->inbuf_lock, flags);
+
+ if (is_console_port(port) && hvc_poll(port->cons.hvc))
+ hvc_kick();
+}
+
+static void control_intr(struct virtqueue *vq)
+{
+ struct ports_device *portdev;
+
+ portdev = vq->vdev->priv;
+ schedule_work(&portdev->control_work);
+}
+
+static void fill_queue(struct virtqueue *vq, spinlock_t *lock)
+{
+ struct port_buffer *buf;
+ int ret;
+
+ do {
+ buf = alloc_buf(PAGE_SIZE);
+ if (!buf)
+ break;
+
+ spin_lock_irq(lock);
+ ret = add_inbuf(vq, buf);
+ if (ret < 0) {
+ spin_unlock_irq(lock);
+ free_buf(buf);
+ break;
+ }
+ spin_unlock_irq(lock);
+ } while (ret > 0);
+}
+
+static int add_port(struct ports_device *portdev, u32 id)
{
struct port *port;
struct port_buffer *inbuf;
@@ -478,11 +672,13 @@ static int __devinit add_port(struct ports_device *portdev)
}
port->portdev = portdev;
+ port->id = id;
port->inbuf = NULL;
+ port->cons.hvc = NULL;
- port->in_vq = portdev->in_vqs[0];
- port->out_vq = portdev->out_vqs[0];
+ port->in_vq = portdev->in_vqs[port->id];
+ port->out_vq = portdev->out_vqs[port->id];
spin_lock_init(&port->inbuf_lock);
@@ -495,9 +691,25 @@ static int __devinit add_port(struct ports_device *portdev)
/* Register the input buffer the first time. */
add_inbuf(port->in_vq, inbuf);
- err = init_port_console(port);
- if (err)
- goto free_inbuf;
+ /*
+ * If we're not using multiport support, this has to be a console port
+ */
+ if (!use_multiport(port->portdev)) {
+ err = init_port_console(port);
+ if (err)
+ goto free_inbuf;
+ }
+
+ spin_lock_irq(&portdev->ports_lock);
+ list_add_tail(&port->list, &port->portdev->ports);
+ spin_unlock_irq(&portdev->ports_lock);
+
+ /*
+ * Tell the Host we're set so that it can send us various
+ * configuration parameters for this port (eg, port name,
+ * caching, whether this is a console port, etc.)
+ */
+ send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
return 0;
@@ -514,12 +726,11 @@ static int init_vqs(struct ports_device *portdev)
vq_callback_t **io_callbacks;
char **io_names;
struct virtqueue **vqs;
- u32 nr_ports, nr_queues;
+ u32 i, j, nr_ports, nr_queues;
int err;
- /* We currently only have one port and two queues for that port */
- nr_ports = 1;
- nr_queues = 2;
+ nr_ports = portdev->config.max_nr_ports;
+ nr_queues = use_multiport(portdev) ? (nr_ports + 1) * 2 : 2;
vqs = kmalloc(nr_queues * sizeof(struct virtqueue *), GFP_KERNEL);
if (!vqs) {
@@ -549,11 +760,32 @@ static int init_vqs(struct ports_device *portdev)
goto free_invqs;
}
- io_callbacks[0] = hvc_handle_input;
- io_callbacks[1] = NULL;
- io_names[0] = "input";
- io_names[1] = "output";
-
+ /*
+ * For backward compat (newer host but older guest), the host
+ * spawns a console port first and also inits the vqs for port
+ * 0 before others.
+ */
+ j = 0;
+ io_callbacks[j] = in_intr;
+ io_callbacks[j + 1] = NULL;
+ io_names[j] = "input";
+ io_names[j + 1] = "output";
+ j += 2;
+
+ if (use_multiport(portdev)) {
+ io_callbacks[j] = control_intr;
+ io_callbacks[j + 1] = NULL;
+ io_names[j] = "control-i";
+ io_names[j + 1] = "control-o";
+
+ for (i = 1; i < nr_ports; i++) {
+ j += 2;
+ io_callbacks[j] = in_intr;
+ io_callbacks[j + 1] = NULL;
+ io_names[j] = "input";
+ io_names[j + 1] = "output";
+ }
+ }
/* Find the queues. */
err = portdev->vdev->config->find_vqs(portdev->vdev, nr_queues, vqs,
io_callbacks,
@@ -561,9 +793,20 @@ static int init_vqs(struct ports_device *portdev)
if (err)
goto free_outvqs;
+ j = 0;
portdev->in_vqs[0] = vqs[0];
portdev->out_vqs[0] = vqs[1];
-
+ j += 2;
+ if (use_multiport(portdev)) {
+ portdev->c_ivq = vqs[j];
+ portdev->c_ovq = vqs[j + 1];
+
+ for (i = 1; i < nr_ports; i++) {
+ j += 2;
+ portdev->in_vqs[i] = vqs[j];
+ portdev->out_vqs[i] = vqs[j + 1];
+ }
+ }
kfree(io_callbacks);
kfree(io_names);
kfree(vqs);
@@ -587,11 +830,17 @@ fail:
/*
* Once we're further in boot, we get probed like any other virtio
* device.
+ *
+ * If the host also supports multiple console ports, we check the
+ * config space to see how many ports the host has spawned. We
+ * initialize each port found.
*/
static int __devinit virtcons_probe(struct virtio_device *vdev)
{
struct ports_device *portdev;
+ u32 i;
int err;
+ bool multiport;
portdev = kmalloc(sizeof(*portdev), GFP_KERNEL);
if (!portdev) {
@@ -603,16 +852,59 @@ static int __devinit virtcons_probe(struct virtio_device *vdev)
portdev->vdev = vdev;
vdev->priv = portdev;
+ multiport = false;
+ portdev->config.nr_ports = 1;
+ portdev->config.max_nr_ports = 1;
+ if (virtio_has_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT)) {
+ multiport = true;
+ vdev->features[0] |= 1 << VIRTIO_CONSOLE_F_MULTIPORT;
+
+ vdev->config->get(vdev, offsetof(struct virtio_console_config,
+ nr_ports),
+ &portdev->config.nr_ports,
+ sizeof(portdev->config.nr_ports));
+ vdev->config->get(vdev, offsetof(struct virtio_console_config,
+ max_nr_ports),
+ &portdev->config.max_nr_ports,
+ sizeof(portdev->config.max_nr_ports));
+ if (portdev->config.nr_ports > portdev->config.max_nr_ports) {
+ dev_warn(&vdev->dev,
+ "More ports (%u) specified than allowed (%u). Will init %u ports.",
+ portdev->config.nr_ports,
+ portdev->config.max_nr_ports,
+ portdev->config.max_nr_ports);
+
+ portdev->config.nr_ports = portdev->config.max_nr_ports;
+ }
+ }
+
+ /* Let the Host know we support multiple ports.*/
+ vdev->config->finalize_features(vdev);
+
err = init_vqs(portdev);
if (err < 0) {
dev_err(&vdev->dev, "Error %d initializing vqs\n", err);
goto free;
}
- /* We only have one port. */
- err = add_port(portdev);
- if (err)
- goto free_vqs;
+ spin_lock_init(&portdev->ports_lock);
+ INIT_LIST_HEAD(&portdev->ports);
+
+ if (multiport) {
+ spin_lock_init(&portdev->cvq_lock);
+ INIT_WORK(&portdev->control_work, &control_work_handler);
+
+ portdev->outbuf = alloc_buf(PAGE_SIZE);
+ if (!portdev->outbuf) {
+ err = -ENOMEM;
+ dev_err(&vdev->dev, "OOM for control outbuf\n");
+ goto free_vqs;
+ }
+ fill_queue(portdev->c_ivq, &portdev->cvq_lock);
+ }
+
+ for (i = 0; i < portdev->config.nr_ports; i++)
+ add_port(portdev, i);
/* Start using the new console output. */
early_put_chars = NULL;
@@ -635,6 +927,7 @@ static struct virtio_device_id id_table[] = {
static unsigned int features[] = {
VIRTIO_CONSOLE_F_SIZE,
+ VIRTIO_CONSOLE_F_MULTIPORT,
};
static struct virtio_driver virtio_console = {
diff --git a/include/linux/virtio_console.h b/include/linux/virtio_console.h
index 9e0da40..cada769 100644
--- a/include/linux/virtio_console.h
+++ b/include/linux/virtio_console.h
@@ -6,18 +6,39 @@
/*
* This header, excluding the #ifdef __KERNEL__ part, is BSD licensed so
* anyone can use the definitions to implement compatible drivers/servers.
+ *
+ * Copyright (C) Red Hat, Inc., 2009
*/
/* Feature bits */
#define VIRTIO_CONSOLE_F_SIZE 0 /* Does host provide console size? */
+#define VIRTIO_CONSOLE_F_MULTIPORT 1 /* Does host provide multiple ports? */
struct virtio_console_config {
/* colums of the screens */
__u16 cols;
/* rows of the screens */
__u16 rows;
+ /* max. number of ports this device can hold */
+ __u32 max_nr_ports;
+ /* number of ports added so far */
+ __u32 nr_ports;
} __attribute__((packed));
+/*
+ * A message that's passed between the Host and the Guest for a
+ * particular port.
+ */
+struct virtio_console_control {
+ __u32 id; /* Port number */
+ __u16 event; /* The kind of control event (see below) */
+ __u16 value; /* Extra information for the key */
+};
+
+/* Some events for control messages */
+#define VIRTIO_CONSOLE_PORT_READY 0
+#define VIRTIO_CONSOLE_CONSOLE_PORT 1
+#define VIRTIO_CONSOLE_RESIZE 2
#ifdef __KERNEL__
int __init virtio_cons_early_init(int (*put_chars)(u32, const char *, int));
--
1.6.2.5
^ permalink raw reply related
* [PATCH 19/31] virtio: console: Introduce a send_buf function for a common path for sending data to host
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: Amit Shah, virtualization
In-Reply-To: <1264772563-25517-1-git-send-email-amit.shah@redhat.com>
Adding support for generic ports that will write to userspace will need
some code changes.
Consolidate the write routine into send_buf() and put_chars() now just
calls into the new function.
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
drivers/char/virtio_console.c | 50 +++++++++++++++++++++++++++--------------
1 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 5096d92..d010510 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -233,6 +233,38 @@ static bool port_has_data(struct port *port)
return ret;
}
+static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
+{
+ struct scatterlist sg[1];
+ struct virtqueue *out_vq;
+ ssize_t ret;
+ unsigned int len;
+
+ out_vq = port->out_vq;
+
+ sg_init_one(sg, in_buf, in_count);
+ ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, in_buf);
+
+ /* Tell Host to go! */
+ out_vq->vq_ops->kick(out_vq);
+
+ if (ret < 0) {
+ len = 0;
+ goto fail;
+ }
+
+ /*
+ * Wait till the host acknowledges it pushed out the data we
+ * sent. Also ensure we return to userspace the number of
+ * bytes that were successfully consumed by the host.
+ */
+ while (!out_vq->vq_ops->get_buf(out_vq, &len))
+ cpu_relax();
+fail:
+ /* We're expected to return the amount of data we wrote */
+ return len;
+}
+
/*
* Give out the data that's requested from the buffer that we have
* queued up.
@@ -280,10 +312,7 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count)
*/
static int put_chars(u32 vtermno, const char *buf, int count)
{
- struct scatterlist sg[1];
struct port *port;
- struct virtqueue *out_vq;
- unsigned int len;
port = find_port_by_vtermno(vtermno);
if (!port)
@@ -292,20 +321,7 @@ static int put_chars(u32 vtermno, const char *buf, int count)
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
- out_vq = port->out_vq;
- /* This is a convenient routine to initialize a single-elem sg list */
- sg_init_one(sg, buf, count);
-
- /* This shouldn't fail: if it does, we lose chars. */
- if (out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, port) >= 0) {
- /* Tell Host to go! */
- out_vq->vq_ops->kick(out_vq);
- while (!out_vq->vq_ops->get_buf(out_vq, &len))
- cpu_relax();
- }
-
- /* We're expected to return the amount of data we wrote: all of it. */
- return count;
+ return send_buf(port, (void *)buf, count);
}
/*
--
1.6.2.5
^ permalink raw reply related
* virtio: console: Return -EFAULT on copy_xx_user errors, allow larger writes
From: Amit Shah @ 2010-01-29 13:42 UTC (permalink / raw)
To: rusty; +Cc: virtualization
Hey Rusty,
These updated patches in the series return -EFAULT on copy_xx_user
errors and also move the copy_from_user into fops_write() instead of it
being in send_buf. This enables send_buf to just read from kernel
buffers, making it simpler.
This also allows write()s to write more to the host in one go,
removingthe 4k limitation. I do limit the writes to 32k at once to not
put too much pressure on the GFP_KERNEL area.
Both of these were pointed out by Marcelo.
I'm including the relative diff between the version that's in linux-next
and the new version below.
Please apply,
Thanks.
Amit.
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index b923b5c..2c2de35 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -162,9 +162,6 @@ struct port {
*/
spinlock_t inbuf_lock;
- /* Buffer that's used to pass data from the guest to the host */
- struct port_buffer *outbuf;
-
/* The IO vqs for this port */
struct virtqueue *in_vq, *out_vq;
@@ -399,43 +396,23 @@ static ssize_t send_control_msg(struct port *port, unsigned int event,
return 0;
}
-static ssize_t send_buf(struct port *port, const char *in_buf, size_t in_count,
- bool from_user)
+static ssize_t send_buf(struct port *port, void *in_buf, size_t in_count)
{
struct scatterlist sg[1];
struct virtqueue *out_vq;
- struct port_buffer *buf;
ssize_t ret;
- unsigned int tmplen;
+ unsigned int len;
out_vq = port->out_vq;
- buf = port->outbuf;
- if (in_count > buf->size)
- in_count = buf->size;
-
- if (from_user) {
- ret = copy_from_user(buf->buf, in_buf, in_count);
- } else {
- /*
- * Since we're not sure when the host will actually
- * consume the data and tell us about it, we have to
- * copy the data here in case the caller frees the
- * in_buf.
- */
- memcpy(buf->buf, in_buf, in_count);
- ret = 0; /* Emulate copy_from_user behaviour */
- }
- buf->len = in_count - ret;
-
- sg_init_one(sg, buf->buf, buf->len);
- ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, buf);
+ sg_init_one(sg, in_buf, in_count);
+ ret = out_vq->vq_ops->add_buf(out_vq, sg, 1, 0, in_buf);
/* Tell Host to go! */
out_vq->vq_ops->kick(out_vq);
if (ret < 0) {
- buf->len = 0;
+ len = 0;
goto fail;
}
@@ -444,13 +421,11 @@ static ssize_t send_buf(struct port *port, const char *in_buf, size_t in_count,
* sent. Also ensure we return to userspace the number of
* bytes that were successfully consumed by the host.
*/
- while (!out_vq->vq_ops->get_buf(out_vq, &tmplen))
+ while (!out_vq->vq_ops->get_buf(out_vq, &len))
cpu_relax();
-
- buf->len = tmplen;
fail:
/* We're expected to return the amount of data we wrote */
- return buf->len;
+ return len;
}
/*
@@ -461,26 +436,25 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
bool to_user)
{
struct port_buffer *buf;
- ssize_t ret;
unsigned long flags;
if (!out_count || !port_has_data(port))
return 0;
buf = port->inbuf;
- if (out_count > buf->len - buf->offset)
- out_count = buf->len - buf->offset;
+ out_count = min(out_count, buf->len - buf->offset);
if (to_user) {
+ ssize_t ret;
+
ret = copy_to_user(out_buf, buf->buf + buf->offset, out_count);
+ if (ret)
+ return -EFAULT;
} else {
memcpy(out_buf, buf->buf + buf->offset, out_count);
- ret = 0; /* Emulate copy_to_user behaviour */
}
- /* Return the number of bytes actually copied */
- ret = out_count - ret;
- buf->offset += ret;
+ buf->offset += out_count;
if (buf->offset == buf->len) {
/*
@@ -495,7 +469,8 @@ static ssize_t fill_readbuf(struct port *port, char *out_buf, size_t out_count,
spin_unlock_irqrestore(&port->inbuf_lock, flags);
}
- return ret;
+ /* Return the number of bytes actually copied */
+ return out_count;
}
/* The condition that must be true for polling to end */
@@ -548,10 +523,27 @@ static ssize_t port_fops_write(struct file *filp, const char __user *ubuf,
size_t count, loff_t *offp)
{
struct port *port;
+ char *buf;
+ ssize_t ret;
port = filp->private_data;
- return send_buf(port, ubuf, count, true);
+ count = min((size_t)(32 * 1024), count);
+
+ buf = kmalloc(count, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ ret = copy_from_user(buf, ubuf, count);
+ if (ret) {
+ ret = -EFAULT;
+ goto free_buf;
+ }
+
+ ret = send_buf(port, buf, count);
+free_buf:
+ kfree(buf);
+ return ret;
}
static unsigned int port_fops_poll(struct file *filp, poll_table *wait)
@@ -657,7 +649,7 @@ static int put_chars(u32 vtermno, const char *buf, int count)
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
- return send_buf(port, buf, count, false);
+ return send_buf(port, (void *)buf, count);
}
/*
@@ -874,7 +866,6 @@ static int remove_port(struct port *port)
cdev_del(&port->cdev);
discard_port_data(port);
- free_buf(port->outbuf);
kfree(port->name);
debugfs_remove(port->debugfs_file);
@@ -1143,11 +1134,6 @@ static int add_port(struct ports_device *portdev, u32 id)
err = -ENOMEM;
goto free_device;
}
- port->outbuf = alloc_buf(PAGE_SIZE);
- if (!port->outbuf) {
- err = -ENOMEM;
- goto free_inbuf;
- }
/* Register the input buffer the first time. */
add_inbuf(port->in_vq, inbuf);
@@ -1158,7 +1144,7 @@ static int add_port(struct ports_device *portdev, u32 id)
if (!use_multiport(port->portdev)) {
err = init_port_console(port);
if (err)
- goto free_outbuf;
+ goto free_inbuf;
}
spin_lock_irq(&portdev->ports_lock);
@@ -1186,8 +1172,6 @@ static int add_port(struct ports_device *portdev, u32 id)
}
return 0;
-free_outbuf:
- free_buf(port->outbuf);
free_inbuf:
free_buf(inbuf);
free_device:
^ permalink raw reply related
* [PATCH 2/2] virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800
From: Rusty Russell @ 2010-01-29 13:20 UTC (permalink / raw)
To: netdev; +Cc: Shirley Ma, Michael S. Tsirkin, David Miller, virtualization
In-Reply-To: <201001292349.05360.rusty@rustcorp.com.au>
From: Shirley Ma <mashirle@us.ibm.com>
virtio_net receives packets from its pre-allocated vring buffers, then it
delivers these packets to upper layer protocols as skb buffs. So it's not
necessary to pre-allocate skb for each mergable buffer, then frees extra
skbs when buffers are merged into a large packet. This patch has deferred
skb allocation in receiving packets for both big packets and mergeable buffers
to reduce skb pre-allocations and skb frees. It frees unused buffers by calling
detach_unused_buf in vring, so recv skb queue is not needed.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/net/virtio_net.c | 427 +++++++++++++++++++++++++++--------------------
1 file changed, 248 insertions(+), 179 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index c708ecc..72b3f21 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -56,8 +56,7 @@ struct virtnet_info
/* Host will merge rx buffers for big packets (shake it! shake it!) */
bool mergeable_rx_bufs;
- /* Receive & send queues. */
- struct sk_buff_head recv;
+ /* Send queue. */
struct sk_buff_head send;
/* Work struct for refilling if we run low on memory. */
@@ -75,34 +74,44 @@ struct skb_vnet_hdr {
unsigned int num_sg;
};
+struct padded_vnet_hdr {
+ struct virtio_net_hdr hdr;
+ /*
+ * virtio_net_hdr should be in a separated sg buffer because of a
+ * QEMU bug, and data sg buffer shares same page with this header sg.
+ * This padding makes next sg 16 byte aligned after virtio_net_hdr.
+ */
+ char padding[6];
+};
+
static inline struct skb_vnet_hdr *skb_vnet_hdr(struct sk_buff *skb)
{
return (struct skb_vnet_hdr *)skb->cb;
}
-static void give_a_page(struct virtnet_info *vi, struct page *page)
-{
- page->private = (unsigned long)vi->pages;
- vi->pages = page;
-}
-
-static void trim_pages(struct virtnet_info *vi, struct sk_buff *skb)
+/*
+ * private is used to chain pages for big packets, put the whole
+ * most recent used list in the beginning for reuse
+ */
+static void give_pages(struct virtnet_info *vi, struct page *page)
{
- unsigned int i;
+ struct page *end;
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++)
- give_a_page(vi, skb_shinfo(skb)->frags[i].page);
- skb_shinfo(skb)->nr_frags = 0;
- skb->data_len = 0;
+ /* Find end of list, sew whole thing into vi->pages. */
+ for (end = page; end->private; end = (struct page *)end->private);
+ end->private = (unsigned long)vi->pages;
+ vi->pages = page;
}
static struct page *get_a_page(struct virtnet_info *vi, gfp_t gfp_mask)
{
struct page *p = vi->pages;
- if (p)
+ if (p) {
vi->pages = (struct page *)p->private;
- else
+ /* clear private here, it is used to chain pages */
+ p->private = 0;
+ } else
p = alloc_page(gfp_mask);
return p;
}
@@ -118,99 +127,142 @@ static void skb_xmit_done(struct virtqueue *svq)
netif_wake_queue(vi->dev);
}
-static void receive_skb(struct net_device *dev, struct sk_buff *skb,
- unsigned len)
+static void set_skb_frag(struct sk_buff *skb, struct page *page,
+ unsigned int offset, unsigned int *len)
{
- struct virtnet_info *vi = netdev_priv(dev);
- struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
- int err;
- int i;
-
- if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
- pr_debug("%s: short packet %i\n", dev->name, len);
- dev->stats.rx_length_errors++;
- goto drop;
- }
+ int i = skb_shinfo(skb)->nr_frags;
+ skb_frag_t *f;
+
+ f = &skb_shinfo(skb)->frags[i];
+ f->size = min((unsigned)PAGE_SIZE - offset, *len);
+ f->page_offset = offset;
+ f->page = page;
+
+ skb->data_len += f->size;
+ skb->len += f->size;
+ skb_shinfo(skb)->nr_frags++;
+ *len -= f->size;
+}
- if (vi->mergeable_rx_bufs) {
- unsigned int copy;
- char *p = page_address(skb_shinfo(skb)->frags[0].page);
+static struct sk_buff *page_to_skb(struct virtnet_info *vi,
+ struct page *page, unsigned int len)
+{
+ struct sk_buff *skb;
+ struct skb_vnet_hdr *hdr;
+ unsigned int copy, hdr_len, offset;
+ char *p;
- if (len > PAGE_SIZE)
- len = PAGE_SIZE;
- len -= sizeof(struct virtio_net_hdr_mrg_rxbuf);
+ p = page_address(page);
- memcpy(&hdr->mhdr, p, sizeof(hdr->mhdr));
- p += sizeof(hdr->mhdr);
+ /* copy small packet so we can reuse these pages for small data */
+ skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
+ if (unlikely(!skb))
+ return NULL;
- copy = len;
- if (copy > skb_tailroom(skb))
- copy = skb_tailroom(skb);
+ hdr = skb_vnet_hdr(skb);
- memcpy(skb_put(skb, copy), p, copy);
+ if (vi->mergeable_rx_bufs) {
+ hdr_len = sizeof hdr->mhdr;
+ offset = hdr_len;
+ } else {
+ hdr_len = sizeof hdr->hdr;
+ offset = sizeof(struct padded_vnet_hdr);
+ }
- len -= copy;
+ memcpy(hdr, p, hdr_len);
- if (!len) {
- give_a_page(vi, skb_shinfo(skb)->frags[0].page);
- skb_shinfo(skb)->nr_frags--;
- } else {
- skb_shinfo(skb)->frags[0].page_offset +=
- sizeof(hdr->mhdr) + copy;
- skb_shinfo(skb)->frags[0].size = len;
- skb->data_len += len;
- skb->len += len;
- }
+ len -= hdr_len;
+ p += offset;
- while (--hdr->mhdr.num_buffers) {
- struct sk_buff *nskb;
+ copy = len;
+ if (copy > skb_tailroom(skb))
+ copy = skb_tailroom(skb);
+ memcpy(skb_put(skb, copy), p, copy);
- i = skb_shinfo(skb)->nr_frags;
- if (i >= MAX_SKB_FRAGS) {
- pr_debug("%s: packet too long %d\n", dev->name,
- len);
- dev->stats.rx_length_errors++;
- goto drop;
- }
+ len -= copy;
+ offset += copy;
- nskb = vi->rvq->vq_ops->get_buf(vi->rvq, &len);
- if (!nskb) {
- pr_debug("%s: rx error: %d buffers missing\n",
- dev->name, hdr->mhdr.num_buffers);
- dev->stats.rx_length_errors++;
- goto drop;
- }
+ while (len) {
+ set_skb_frag(skb, page, offset, &len);
+ page = (struct page *)page->private;
+ offset = 0;
+ }
- __skb_unlink(nskb, &vi->recv);
- vi->num--;
+ if (page)
+ give_pages(vi, page);
- skb_shinfo(skb)->frags[i] = skb_shinfo(nskb)->frags[0];
- skb_shinfo(nskb)->nr_frags = 0;
- kfree_skb(nskb);
+ return skb;
+}
- if (len > PAGE_SIZE)
- len = PAGE_SIZE;
+static int receive_mergeable(struct virtnet_info *vi, struct sk_buff *skb)
+{
+ struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb);
+ struct page *page;
+ int num_buf, i, len;
+
+ num_buf = hdr->mhdr.num_buffers;
+ while (--num_buf) {
+ i = skb_shinfo(skb)->nr_frags;
+ if (i >= MAX_SKB_FRAGS) {
+ pr_debug("%s: packet too long\n", skb->dev->name);
+ skb->dev->stats.rx_length_errors++;
+ return -EINVAL;
+ }
- skb_shinfo(skb)->frags[i].size = len;
- skb_shinfo(skb)->nr_frags++;
- skb->data_len += len;
- skb->len += len;
+ page = vi->rvq->vq_ops->get_buf(vi->rvq, &len);
+ if (!page) {
+ pr_debug("%s: rx error: %d buffers missing\n",
+ skb->dev->name, hdr->mhdr.num_buffers);
+ skb->dev->stats.rx_length_errors++;
+ return -EINVAL;
}
- } else {
- len -= sizeof(hdr->hdr);
+ if (len > PAGE_SIZE)
+ len = PAGE_SIZE;
+
+ set_skb_frag(skb, page, 0, &len);
+
+ --vi->num;
+ }
+ return 0;
+}
+
+static void receive_buf(struct net_device *dev, void *buf, unsigned int len)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ struct sk_buff *skb;
+ struct page *page;
+ struct skb_vnet_hdr *hdr;
- if (len <= MAX_PACKET_LEN)
- trim_pages(vi, skb);
+ if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
+ pr_debug("%s: short packet %i\n", dev->name, len);
+ dev->stats.rx_length_errors++;
+ if (vi->mergeable_rx_bufs || vi->big_packets)
+ give_pages(vi, buf);
+ else
+ dev_kfree_skb(buf);
+ return;
+ }
- err = pskb_trim(skb, len);
- if (err) {
- pr_debug("%s: pskb_trim failed %i %d\n", dev->name,
- len, err);
+ if (!vi->mergeable_rx_bufs && !vi->big_packets) {
+ skb = buf;
+ len -= sizeof(struct virtio_net_hdr);
+ skb_trim(skb, len);
+ } else {
+ page = buf;
+ skb = page_to_skb(vi, page, len);
+ if (unlikely(!skb)) {
dev->stats.rx_dropped++;
- goto drop;
+ give_pages(vi, page);
+ return;
}
+ if (vi->mergeable_rx_bufs)
+ if (receive_mergeable(vi, skb)) {
+ dev_kfree_skb(skb);
+ return;
+ }
}
+ hdr = skb_vnet_hdr(skb);
skb->truesize += skb->data_len;
dev->stats.rx_bytes += skb->len;
dev->stats.rx_packets++;
@@ -267,110 +319,119 @@ static void receive_skb(struct net_device *dev, struct sk_buff *skb,
frame_err:
dev->stats.rx_frame_errors++;
-drop:
dev_kfree_skb(skb);
}
-static bool try_fill_recv_maxbufs(struct virtnet_info *vi, gfp_t gfp)
+static int add_recvbuf_small(struct virtnet_info *vi, gfp_t gfp)
{
struct sk_buff *skb;
- struct scatterlist sg[2+MAX_SKB_FRAGS];
- int num, err, i;
- bool oom = false;
-
- sg_init_table(sg, 2+MAX_SKB_FRAGS);
- do {
- struct skb_vnet_hdr *hdr;
+ struct skb_vnet_hdr *hdr;
+ struct scatterlist sg[2];
+ int err;
- skb = netdev_alloc_skb_ip_align(vi->dev, MAX_PACKET_LEN);
- if (unlikely(!skb)) {
- oom = true;
- break;
- }
+ skb = netdev_alloc_skb_ip_align(vi->dev, MAX_PACKET_LEN);
+ if (unlikely(!skb))
+ return -ENOMEM;
- skb_put(skb, MAX_PACKET_LEN);
+ skb_put(skb, MAX_PACKET_LEN);
- hdr = skb_vnet_hdr(skb);
- sg_set_buf(sg, &hdr->hdr, sizeof(hdr->hdr));
+ hdr = skb_vnet_hdr(skb);
+ sg_set_buf(sg, &hdr->hdr, sizeof hdr->hdr);
- if (vi->big_packets) {
- for (i = 0; i < MAX_SKB_FRAGS; i++) {
- skb_frag_t *f = &skb_shinfo(skb)->frags[i];
- f->page = get_a_page(vi, gfp);
- if (!f->page)
- break;
+ skb_to_sgvec(skb, sg + 1, 0, skb->len);
- f->page_offset = 0;
- f->size = PAGE_SIZE;
+ err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, 2, skb);
+ if (err < 0)
+ dev_kfree_skb(skb);
- skb->data_len += PAGE_SIZE;
- skb->len += PAGE_SIZE;
+ return err;
+}
- skb_shinfo(skb)->nr_frags++;
- }
+static int add_recvbuf_big(struct virtnet_info *vi, gfp_t gfp)
+{
+ struct scatterlist sg[MAX_SKB_FRAGS + 2];
+ struct page *first, *list = NULL;
+ char *p;
+ int i, err, offset;
+
+ /* page in sg[MAX_SKB_FRAGS + 1] is list tail */
+ for (i = MAX_SKB_FRAGS + 1; i > 1; --i) {
+ first = get_a_page(vi, gfp);
+ if (!first) {
+ if (list)
+ give_pages(vi, list);
+ return -ENOMEM;
}
+ sg_set_buf(&sg[i], page_address(first), PAGE_SIZE);
- num = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1;
- skb_queue_head(&vi->recv, skb);
+ /* chain new page in list head to match sg */
+ first->private = (unsigned long)list;
+ list = first;
+ }
- err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, num, skb);
- if (err < 0) {
- skb_unlink(skb, &vi->recv);
- trim_pages(vi, skb);
- kfree_skb(skb);
- break;
- }
- vi->num++;
- } while (err >= num);
- if (unlikely(vi->num > vi->max))
- vi->max = vi->num;
- vi->rvq->vq_ops->kick(vi->rvq);
- return !oom;
+ first = get_a_page(vi, gfp);
+ if (!first) {
+ give_pages(vi, list);
+ return -ENOMEM;
+ }
+ p = page_address(first);
+
+ /* sg[0], sg[1] share the same page */
+ /* a separated sg[0] for virtio_net_hdr only during to QEMU bug*/
+ sg_set_buf(&sg[0], p, sizeof(struct virtio_net_hdr));
+
+ /* sg[1] for data packet, from offset */
+ offset = sizeof(struct padded_vnet_hdr);
+ sg_set_buf(&sg[1], p + offset, PAGE_SIZE - offset);
+
+ /* chain first in list head */
+ first->private = (unsigned long)list;
+ err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, MAX_SKB_FRAGS + 2,
+ first);
+ if (err < 0)
+ give_pages(vi, first);
+
+ return err;
}
-/* Returns false if we couldn't fill entirely (OOM). */
-static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
+static int add_recvbuf_mergeable(struct virtnet_info *vi, gfp_t gfp)
{
- struct sk_buff *skb;
- struct scatterlist sg[1];
+ struct page *page;
+ struct scatterlist sg;
int err;
- bool oom = false;
-
- if (!vi->mergeable_rx_bufs)
- return try_fill_recv_maxbufs(vi, gfp);
- do {
- skb_frag_t *f;
+ page = get_a_page(vi, gfp);
+ if (!page)
+ return -ENOMEM;
- skb = netdev_alloc_skb_ip_align(vi->dev, GOOD_COPY_LEN);
- if (unlikely(!skb)) {
- oom = true;
- break;
- }
+ sg_init_one(&sg, page_address(page), PAGE_SIZE);
- f = &skb_shinfo(skb)->frags[0];
- f->page = get_a_page(vi, gfp);
- if (!f->page) {
- oom = true;
- kfree_skb(skb);
- break;
- }
+ err = vi->rvq->vq_ops->add_buf(vi->rvq, &sg, 0, 1, page);
+ if (err < 0)
+ give_pages(vi, page);
- f->page_offset = 0;
- f->size = PAGE_SIZE;
+ return err;
+}
- skb_shinfo(skb)->nr_frags++;
+/* Returns false if we couldn't fill entirely (OOM). */
+static bool try_fill_recv(struct virtnet_info *vi, gfp_t gfp)
+{
+ int err;
+ bool oom = false;
- sg_init_one(sg, page_address(f->page), PAGE_SIZE);
- skb_queue_head(&vi->recv, skb);
+ do {
+ if (vi->mergeable_rx_bufs)
+ err = add_recvbuf_mergeable(vi, gfp);
+ else if (vi->big_packets)
+ err = add_recvbuf_big(vi, gfp);
+ else
+ err = add_recvbuf_small(vi, gfp);
- err = vi->rvq->vq_ops->add_buf(vi->rvq, sg, 0, 1, skb);
if (err < 0) {
- skb_unlink(skb, &vi->recv);
- kfree_skb(skb);
+ oom = true;
break;
}
- vi->num++;
+ ++vi->num;
} while (err > 0);
if (unlikely(vi->num > vi->max))
vi->max = vi->num;
@@ -408,15 +469,14 @@ static void refill_work(struct work_struct *work)
static int virtnet_poll(struct napi_struct *napi, int budget)
{
struct virtnet_info *vi = container_of(napi, struct virtnet_info, napi);
- struct sk_buff *skb = NULL;
+ void *buf;
unsigned int len, received = 0;
again:
while (received < budget &&
- (skb = vi->rvq->vq_ops->get_buf(vi->rvq, &len)) != NULL) {
- __skb_unlink(skb, &vi->recv);
- receive_skb(vi->dev, skb, len);
- vi->num--;
+ (buf = vi->rvq->vq_ops->get_buf(vi->rvq, &len)) != NULL) {
+ receive_buf(vi->dev, buf, len);
+ --vi->num;
received++;
}
@@ -496,9 +556,9 @@ static int xmit_skb(struct virtnet_info *vi, struct sk_buff *skb)
/* Encode metadata header at front. */
if (vi->mergeable_rx_bufs)
- sg_set_buf(sg, &hdr->mhdr, sizeof(hdr->mhdr));
+ sg_set_buf(sg, &hdr->mhdr, sizeof hdr->mhdr);
else
- sg_set_buf(sg, &hdr->hdr, sizeof(hdr->hdr));
+ sg_set_buf(sg, &hdr->hdr, sizeof hdr->hdr);
hdr->num_sg = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1;
return vi->svq->vq_ops->add_buf(vi->svq, sg, hdr->num_sg, 0, skb);
@@ -916,8 +976,7 @@ static int virtnet_probe(struct virtio_device *vdev)
dev->features |= NETIF_F_HW_VLAN_FILTER;
}
- /* Initialize our empty receive and send queues. */
- skb_queue_head_init(&vi->recv);
+ /* Initialize our empty send queue. */
skb_queue_head_init(&vi->send);
err = register_netdev(dev);
@@ -952,25 +1011,35 @@ free:
return err;
}
+static void free_unused_bufs(struct virtnet_info *vi)
+{
+ void *buf;
+ while (1) {
+ buf = vi->rvq->vq_ops->detach_unused_buf(vi->rvq);
+ if (!buf)
+ break;
+ if (vi->mergeable_rx_bufs || vi->big_packets)
+ give_pages(vi, buf);
+ else
+ dev_kfree_skb(buf);
+ --vi->num;
+ }
+ BUG_ON(vi->num != 0);
+}
+
static void __devexit virtnet_remove(struct virtio_device *vdev)
{
struct virtnet_info *vi = vdev->priv;
- struct sk_buff *skb;
/* Stop all the virtqueues. */
vdev->config->reset(vdev);
- /* Free our skbs in send and recv queues, if any. */
- while ((skb = __skb_dequeue(&vi->recv)) != NULL) {
- kfree_skb(skb);
- vi->num--;
- }
+ /* Free our skbs in send queue, if any. */
__skb_queue_purge(&vi->send);
- BUG_ON(vi->num != 0);
-
unregister_netdev(vi->dev);
cancel_delayed_work_sync(&vi->refill);
+ free_unused_bufs(vi);
vdev->config->del_vqs(vi->vdev);
^ permalink raw reply related
* [PATCH 1/2] virtio: Add ability to detach unused buffers from vrings
From: Rusty Russell @ 2010-01-29 13:19 UTC (permalink / raw)
To: netdev; +Cc: Shirley Ma, Michael S. Tsirkin, David Miller, virtualization
In-Reply-To: <201001292346.43675.rusty@rustcorp.com.au>
From: Shirley Ma <mashirle@us.ibm.com>
There's currently no way for a virtio driver to ask for unused
buffers, so it has to keep a list itself to reclaim them at shutdown.
This is redundant, since virtio_ring stores that information. So
add a new hook to do this.
Signed-off-by: Shirley Ma <xma@us.ibm.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
drivers/virtio/virtio_ring.c | 25 +++++++++++++++++++++++++
include/linux/virtio.h | 4 ++++
2 files changed, 29 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index fbd2ecd..71929ee 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -334,6 +334,30 @@ static bool vring_enable_cb(struct virtqueue *_vq)
return true;
}
+static void *vring_detach_unused_buf(struct virtqueue *_vq)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ unsigned int i;
+ void *buf;
+
+ START_USE(vq);
+
+ for (i = 0; i < vq->vring.num; i++) {
+ if (!vq->data[i])
+ continue;
+ /* detach_buf clears data, so grab it now. */
+ buf = vq->data[i];
+ detach_buf(vq, i);
+ END_USE(vq);
+ return buf;
+ }
+ /* That should have freed everything. */
+ BUG_ON(vq->num_free != vq->vring.num);
+
+ END_USE(vq);
+ return NULL;
+}
+
irqreturn_t vring_interrupt(int irq, void *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -360,6 +384,7 @@ static struct virtqueue_ops vring_vq_ops = {
.kick = vring_kick,
.disable_cb = vring_disable_cb,
.enable_cb = vring_enable_cb,
+ .detach_unused_buf = vring_detach_unused_buf,
};
struct virtqueue *vring_new_virtqueue(unsigned int num,
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 057a2e0..f508c65 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -51,6 +51,9 @@ 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.
+ * @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
*
* Locking rules are straightforward: the driver is responsible for
* locking. No two operations may be invoked simultaneously, with the exception
@@ -71,6 +74,7 @@ struct virtqueue_ops {
void (*disable_cb)(struct virtqueue *vq);
bool (*enable_cb)(struct virtqueue *vq);
+ void *(*detach_unused_buf)(struct virtqueue *vq);
};
/**
^ permalink raw reply related
* [PATCH 0/2] virtio net improvements
From: Rusty Russell @ 2010-01-29 13:16 UTC (permalink / raw)
To: netdev, David Miller; +Cc: Shirley Ma, Michael S. Tsirkin, virtualization
Hi Dave,
Nice driver optimization from Shirley, but requires a new virtio hook.
Do you want to take both? I have nothing else overlapping it.
Cheers,
Rusty.
^ permalink raw reply
* Re: [PATCHv2] virtio: use smp_XX barriers on SMP
From: Michael S. Tsirkin @ 2010-01-29 11:27 UTC (permalink / raw)
To: Rusty Russell; +Cc: virtualization
In-Reply-To: <201001290744.59324.rusty@rustcorp.com.au>
On Fri, Jan 29, 2010 at 07:44:59AM +1030, Rusty Russell wrote:
> On Fri, 29 Jan 2010 12:07:08 am Michael S. Tsirkin wrote:
> > On Thu, Jan 28, 2010 at 10:01:09AM +1030, Rusty Russell wrote:
> > > On Thu, 28 Jan 2010 09:12:23 am Michael S. Tsirkin wrote:
> > > > Where possible, we should use SMP barriers which are more lightweight than
> > > > mandatory barriers, because mandatory barriers also control MMIO effects on
> > > > accesses through relaxed memory I/O windows (which virtio does not use)
> > > > (compare specifically smp_rmb and rmb on x86_64).
> > >
> > > Xen had a similar issue, in that UP guests running on SMP hosts need to issue
> > > SMP barriers. Is this not also a requirement for virtio?
> >
> > Of course it is. That's why I have ifdef CONFIG_SMP and use
> > mandatory barriers on UP.
>
> Sorry, this was an off-the-cuff comment. I was concerned that UP barriers
> might be insufficient on some archs. But I can't find any such archs when
> I actually look (at least, x86, s390 and powerpc).
>
> I've applied your patch, minus the first two substitutions:
>
> @@ -36,10 +54,10 @@
> panic("%s:in_use = %i\n", \
> (_vq)->vq.name, (_vq)->in_use); \
> (_vq)->in_use = __LINE__; \
> - mb(); \
> + virtio_mb(); \
> } while (0)
> #define END_USE(_vq) \
> - do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; mb(); } while(0)
> + do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; virtio_mb(); } while(0)
> #else
> #define BAD_RING(_vq, fmt, args...) \
> do { \
>
> These barriers are actually trying to make sure in_use is set in a timely
> manner (this is debug code). They're bogus AFAICT: if you don't have any
> other synchronization then you're in danger of nesting and you want to know.
> And barriers might change timing too much when DEBUG is defined.
>
> Thanks!
> Rusty.
So .. let's just drop these barriers, and replace in_use with an atomic?
--
MST
^ permalink raw reply
* Re: [PATCHv2] virtio: use smp_XX barriers on SMP
From: Rusty Russell @ 2010-01-28 21:14 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization
In-Reply-To: <20100128133708.GA3776@redhat.com>
On Fri, 29 Jan 2010 12:07:08 am Michael S. Tsirkin wrote:
> On Thu, Jan 28, 2010 at 10:01:09AM +1030, Rusty Russell wrote:
> > On Thu, 28 Jan 2010 09:12:23 am Michael S. Tsirkin wrote:
> > > Where possible, we should use SMP barriers which are more lightweight than
> > > mandatory barriers, because mandatory barriers also control MMIO effects on
> > > accesses through relaxed memory I/O windows (which virtio does not use)
> > > (compare specifically smp_rmb and rmb on x86_64).
> >
> > Xen had a similar issue, in that UP guests running on SMP hosts need to issue
> > SMP barriers. Is this not also a requirement for virtio?
>
> Of course it is. That's why I have ifdef CONFIG_SMP and use
> mandatory barriers on UP.
Sorry, this was an off-the-cuff comment. I was concerned that UP barriers
might be insufficient on some archs. But I can't find any such archs when
I actually look (at least, x86, s390 and powerpc).
I've applied your patch, minus the first two substitutions:
@@ -36,10 +54,10 @@
panic("%s:in_use = %i\n", \
(_vq)->vq.name, (_vq)->in_use); \
(_vq)->in_use = __LINE__; \
- mb(); \
+ virtio_mb(); \
} while (0)
#define END_USE(_vq) \
- do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; mb(); } while(0)
+ do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; virtio_mb(); } while(0)
#else
#define BAD_RING(_vq, fmt, args...) \
do { \
These barriers are actually trying to make sure in_use is set in a timely
manner (this is debug code). They're bogus AFAICT: if you don't have any
other synchronization then you're in danger of nesting and you want to know.
And barriers might change timing too much when DEBUG is defined.
Thanks!
Rusty.
^ permalink raw reply
* Re: [PATCHv2] virtio: use smp_XX barriers on SMP
From: Michael S. Tsirkin @ 2010-01-28 13:37 UTC (permalink / raw)
To: Rusty Russell; +Cc: virtualization
In-Reply-To: <201001281001.09999.rusty@rustcorp.com.au>
On Thu, Jan 28, 2010 at 10:01:09AM +1030, Rusty Russell wrote:
> On Thu, 28 Jan 2010 09:12:23 am Michael S. Tsirkin wrote:
> > virtio is communicating with a virtual "device" that actually runs on
> > another host processor. Thus SMP barriers can be used to control
> > memory access ordering.
> >
> > Where possible, we should use SMP barriers which are more lightweight than
> > mandatory barriers, because mandatory barriers also control MMIO effects on
> > accesses through relaxed memory I/O windows (which virtio does not use)
> > (compare specifically smp_rmb and rmb on x86_64).
>
> Xen had a similar issue, in that UP guests running on SMP hosts need to issue
> SMP barriers. Is this not also a requirement for virtio?
Of course it is. That's why I have ifdef CONFIG_SMP and use
mandatory barriers on UP.
> But I'm not sure what came out of the discussion: Jeremy?
>
> Cheers,
> Rusty.
^ permalink raw reply
* Re: [PATCHv2] virtio: use smp_XX barriers on SMP
From: Rusty Russell @ 2010-01-27 23:31 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization
In-Reply-To: <20100127224222.GA31545@redhat.com>
On Thu, 28 Jan 2010 09:12:23 am Michael S. Tsirkin wrote:
> virtio is communicating with a virtual "device" that actually runs on
> another host processor. Thus SMP barriers can be used to control
> memory access ordering.
>
> Where possible, we should use SMP barriers which are more lightweight than
> mandatory barriers, because mandatory barriers also control MMIO effects on
> accesses through relaxed memory I/O windows (which virtio does not use)
> (compare specifically smp_rmb and rmb on x86_64).
Xen had a similar issue, in that UP guests running on SMP hosts need to issue
SMP barriers. Is this not also a requirement for virtio?
But I'm not sure what came out of the discussion: Jeremy?
Cheers,
Rusty.
^ permalink raw reply
* [PATCHv2] virtio: use smp_XX barriers on SMP
From: Michael S. Tsirkin @ 2010-01-27 22:42 UTC (permalink / raw)
To: Rusty Russell, Michael S. Tsirkin, virtualization
virtio is communicating with a virtual "device" that actually runs on
another host processor. Thus SMP barriers can be used to control
memory access ordering.
Where possible, we should use SMP barriers which are more lightweight than
mandatory barriers, because mandatory barriers also control MMIO effects on
accesses through relaxed memory I/O windows (which virtio does not use)
(compare specifically smp_rmb and rmb on x86_64).
We can't just use smp_mb and friends though, because
we must force memory ordering even if guest is UP since host could be
running on another CPU, but SMP barriers are defined to barrier() in
that configuration. So, for UP fall back to mandatory barriers instead.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
Changes from v1: fall back on mandatory barriers for UP
drivers/virtio/virtio_ring.c | 30 ++++++++++++++++++++++++------
1 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index fbd2ecd..6794662 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -21,6 +21,24 @@
#include <linux/virtio_config.h>
#include <linux/device.h>
+/* virtio guest is communicating with a virtual "device" that actually runs on
+ * a host processor. Memory barriers are used to control SMP effects. */
+#ifdef CONFIG_SMP
+/* Where possible, use SMP barriers which are more lightweight than mandatory
+ * barriers, because mandatory barriers control MMIO effects on accesses
+ * through relaxed memory I/O windows (which virtio does not use). */
+#define virtio_mb() smp_mb()
+#define virtio_rmb() smp_rmb()
+#define virtio_wmb() smp_wmb()
+#else
+/* We must force memory ordering even if guest is UP since host could be
+ * running on another CPU, but SMP barriers are defined to barrier() in that
+ * configuration. So fall back to mandatory barriers instead. */
+#define virtio_mb() mb()
+#define virtio_rmb() rmb()
+#define virtio_wmb() wmb()
+#endif
+
#ifdef DEBUG
/* For development, we want to crash whenever the ring is screwed. */
#define BAD_RING(_vq, fmt, args...) \
@@ -36,10 +54,10 @@
panic("%s:in_use = %i\n", \
(_vq)->vq.name, (_vq)->in_use); \
(_vq)->in_use = __LINE__; \
- mb(); \
+ virtio_mb(); \
} while (0)
#define END_USE(_vq) \
- do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; mb(); } while(0)
+ do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; virtio_mb(); } while(0)
#else
#define BAD_RING(_vq, fmt, args...) \
do { \
@@ -221,13 +239,13 @@ static void vring_kick(struct virtqueue *_vq)
START_USE(vq);
/* Descriptors and available array need to be set before we expose the
* new available array entries. */
- wmb();
+ virtio_wmb();
vq->vring.avail->idx += vq->num_added;
vq->num_added = 0;
/* Need to update avail index before checking if we should notify */
- mb();
+ virtio_mb();
if (!(vq->vring.used->flags & VRING_USED_F_NO_NOTIFY))
/* Prod other side to tell it about changes. */
@@ -286,7 +304,7 @@ static void *vring_get_buf(struct virtqueue *_vq, unsigned int *len)
}
/* Only get used array entries after they have been exposed by host. */
- rmb();
+ virtio_rmb();
i = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].id;
*len = vq->vring.used->ring[vq->last_used_idx%vq->vring.num].len;
@@ -324,7 +342,7 @@ static bool vring_enable_cb(struct virtqueue *_vq)
/* We optimistically turn back on interrupts, then check if there was
* more to do. */
vq->vring.avail->flags &= ~VRING_AVAIL_F_NO_INTERRUPT;
- mb();
+ virtio_mb();
if (unlikely(more_used(vq))) {
END_USE(vq);
return false;
--
1.6.6.144.g5c3af
^ permalink raw reply related
* Re: [PATCH] virtio: use smp_XX barriers
From: Michael S. Tsirkin @ 2010-01-27 9:47 UTC (permalink / raw)
To: Avi Kivity; +Cc: virtualization
In-Reply-To: <4B600B78.1020505@redhat.com>
On Wed, Jan 27, 2010 at 11:46:32AM +0200, Avi Kivity wrote:
> On 01/27/2010 11:36 AM, Michael S. Tsirkin wrote:
>>
>>
>>> smp_mb() is used for processor-vs-processor ordering, which can't happen
>>> on UP systems, but for process-vs-device, we must use mb().
>>>
>>> (this shows up if running a UP guest on an SMP host).
>>>
>> Currently, yes. But virtio is not a real device.
>> Here's what I was really trying to improve: rmb() is an lfence on
>> x86_64, but smp_rmb() is a barrier() and this is really sufficient for
>> virtio because x86_64 does not reorder memory reads.
>>
>>
>
> x86_64 can do speculative and reordered reads.
Intel system programming says:
7.2.3.2 Neither Loads Nor Stores Are Reordered with Like Operations
Doesn't this mean that reads are not reordered? If no what does it
mean?
>> Does this mean such an optimization would need a new macro?
>>
>
> Please no.
>
> --
> Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply
* Re: [PATCH] virtio: use smp_XX barriers
From: Avi Kivity @ 2010-01-27 9:46 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization
In-Reply-To: <20100127093610.GD3476@redhat.com>
On 01/27/2010 11:36 AM, Michael S. Tsirkin wrote:
>
>
>> smp_mb() is used for processor-vs-processor ordering, which can't happen
>> on UP systems, but for process-vs-device, we must use mb().
>>
>> (this shows up if running a UP guest on an SMP host).
>>
> Currently, yes. But virtio is not a real device.
> Here's what I was really trying to improve: rmb() is an lfence on
> x86_64, but smp_rmb() is a barrier() and this is really sufficient for
> virtio because x86_64 does not reorder memory reads.
>
>
x86_64 can do speculative and reordered reads.
> Does this mean such an optimization would need a new macro?
>
Please no.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply
* Re: [PATCH] virtio: use smp_XX barriers
From: Michael S. Tsirkin @ 2010-01-27 9:36 UTC (permalink / raw)
To: Avi Kivity; +Cc: virtualization
In-Reply-To: <4B600740.3060806@redhat.com>
On Wed, Jan 27, 2010 at 11:28:32AM +0200, Avi Kivity wrote:
> On 01/21/2010 07:10 PM, Michael S. Tsirkin wrote:
>> Documentation/memory-barriers.txt says:
>> Mandatory barriers should not be used to control SMP effects, since
>> mandatory barriers unnecessarily impose overhead on UP systems.
>>
>> This rule applies to virtio, so let's do it correctly.
>>
>>
>
> This is wrong. A UP guest still runs in parallel with the virtio device.
Yes, you are right here. I forgot that these macros
are compiled out on UP guest. Rusty please ignore this patch.
> smp_mb() is used for processor-vs-processor ordering, which can't happen
> on UP systems, but for process-vs-device, we must use mb().
>
> (this shows up if running a UP guest on an SMP host).
Currently, yes. But virtio is not a real device.
Here's what I was really trying to improve: rmb() is an lfence on
x86_64, but smp_rmb() is a barrier() and this is really sufficient for
virtio because x86_64 does not reorder memory reads.
Does this mean such an optimization would need a new macro?
--
MST
^ permalink raw reply
* Re: [PATCH] virtio: use smp_XX barriers
From: Avi Kivity @ 2010-01-27 9:28 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: virtualization
In-Reply-To: <20100121171055.GA16693@redhat.com>
On 01/21/2010 07:10 PM, Michael S. Tsirkin wrote:
> Documentation/memory-barriers.txt says:
> Mandatory barriers should not be used to control SMP effects, since
> mandatory barriers unnecessarily impose overhead on UP systems.
>
> This rule applies to virtio, so let's do it correctly.
>
>
This is wrong. A UP guest still runs in parallel with the virtio device.
smp_mb() is used for processor-vs-processor ordering, which can't happen
on UP systems, but for process-vs-device, we must use mb().
(this shows up if running a UP guest on an SMP host).
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply
* Re: Remove host throttling
From: Rusty Russell @ 2010-01-25 10:32 UTC (permalink / raw)
To: Amit Shah; +Cc: virtualization
In-Reply-To: <1263977465-13983-1-git-send-email-amit.shah@redhat.com>
On Wed, 20 Jan 2010 07:21:02 pm Amit Shah wrote:
>
> Hey Rusty,
>
> These are the patches to be replaced -- patches 19, 30, 31 from the
> previous series. Patch 26 from the earlier series is to be dropped.
Thanks, applied!
Rusty.
^ 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