From 5d006d8d09e82f086ca0baf79a2907f2c1e25af7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:29 -0500 Subject: lguest: set max_pfn_mapped, growl loudly at Yinghai Lu 6af61a7614a306fe882a0c2b4ddc63b65aa66efc 'x86: clean up max_pfn_mapped usage - 32-bit' makes the following comment: XEN PV and lguest may need to assign max_pfn_mapped too. But no CC. Yinghai, wasting fellow developers' time is a VERY bad habit. If you do it again, I will hunt you down and try to extract the three hours of my life I just lost :) Signed-off-by: Rusty Russell Cc: Yinghai Lu --- arch/x86/lguest/boot.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c index 0313a5eec412..d9249a882aa5 100644 --- a/arch/x86/lguest/boot.c +++ b/arch/x86/lguest/boot.c @@ -1014,6 +1014,9 @@ __init void lguest_init(void) init_pg_tables_start = __pa(pg0); init_pg_tables_end = __pa(pg0); + /* As described in head_32.S, we map the first 128M of memory. */ + max_pfn_mapped = (128*1024*1024) >> PAGE_SHIFT; + /* Load the %fs segment register (the per-cpu segment register) with * the normal data segment to get through booting. */ asm volatile ("mov %0, %%fs" : : "r" (__KERNEL_DS) : "memory"); -- cgit v1.2.2 From 0c12091d82e48dc423fb1f51eb0062c557a084af Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:31 -0500 Subject: lguest: Guest int3 fix Ron Minnich noticed that guest userspace gets a GPF when it tries to int3: we need to copy the privilege level from the guest-supplied IDT to the real IDT. int3 is the only common case where guest userspace expects to invoke an interrupt, so that's the symptom of failing to do this. Signed-off-by: Rusty Russell --- drivers/lguest/interrupts_and_traps.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c index 0414ddf87587..a1039068f95c 100644 --- a/drivers/lguest/interrupts_and_traps.c +++ b/drivers/lguest/interrupts_and_traps.c @@ -406,7 +406,8 @@ void load_guest_idt_entry(struct lg_cpu *cpu, unsigned int num, u32 lo, u32 hi) * deliver_trap() to bounce it back into the Guest. */ static void default_idt_entry(struct desc_struct *idt, int trap, - const unsigned long handler) + const unsigned long handler, + const struct desc_struct *base) { /* A present interrupt gate. */ u32 flags = 0x8e00; @@ -415,6 +416,10 @@ static void default_idt_entry(struct desc_struct *idt, * the Guest to use the "int" instruction to trigger it. */ if (trap == LGUEST_TRAP_ENTRY) flags |= (GUEST_PL << 13); + else if (base) + /* Copy priv. level from what Guest asked for. This allows + * debug (int 3) traps from Guest userspace, for example. */ + flags |= (base->b & 0x6000); /* Now pack it into the IDT entry in its weird format. */ idt->a = (LGUEST_CS<<16) | (handler&0x0000FFFF); @@ -428,7 +433,7 @@ void setup_default_idt_entries(struct lguest_ro_state *state, unsigned int i; for (i = 0; i < ARRAY_SIZE(state->guest_idt); i++) - default_idt_entry(&state->guest_idt[i], i, def[i]); + default_idt_entry(&state->guest_idt[i], i, def[i], NULL); } /*H:240 We don't use the IDT entries in the "struct lguest" directly, instead @@ -442,6 +447,8 @@ void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt, /* We can simply copy the direct traps, otherwise we use the default * ones in the Switcher: they will return to the Host. */ for (i = 0; i < ARRAY_SIZE(cpu->arch.idt); i++) { + const struct desc_struct *gidt = &cpu->arch.idt[i]; + /* If no Guest can ever override this trap, leave it alone. */ if (!direct_trap(i)) continue; @@ -449,12 +456,15 @@ void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt, /* Only trap gates (type 15) can go direct to the Guest. * Interrupt gates (type 14) disable interrupts as they are * entered, which we never let the Guest do. Not present - * entries (type 0x0) also can't go direct, of course. */ - if (idt_type(cpu->arch.idt[i].a, cpu->arch.idt[i].b) == 0xF) - idt[i] = cpu->arch.idt[i]; + * entries (type 0x0) also can't go direct, of course. + * + * If it can't go direct, we still need to copy the priv. level: + * they might want to give userspace access to a software + * interrupt. */ + if (idt_type(gidt->a, gidt->b) == 0xF) + idt[i] = *gidt; else - /* Reset it to the default. */ - default_idt_entry(&idt[i], i, def[i]); + default_idt_entry(&idt[i], i, def[i], gidt); } } -- cgit v1.2.2 From 0a707210aa1b8ac40fe781b2a9d0b203b6ebb921 Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Tue, 8 Jul 2008 10:29:42 +0200 Subject: lguest: fix switcher_page leak on unload map_switcher allocates the array, unmap_switcher has to free it accordingly. Signed-off-by: Johannes Weiner Signed-off-by: Rusty Russell --- drivers/lguest/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c index 5eea4356d703..90663e01a56e 100644 --- a/drivers/lguest/core.c +++ b/drivers/lguest/core.c @@ -135,6 +135,7 @@ static void unmap_switcher(void) /* Now we just need to free the pages we copied the switcher into */ for (i = 0; i < TOTAL_SWITCHER_PAGES; i++) __free_pages(switcher_page[i], 0); + kfree(switcher_page); } /*H:032 -- cgit v1.2.2 From 32c68e5c569fdf016b494ce2fc8eecf59b6881bd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:32 -0500 Subject: lguest: fix verbose printing of device features. %02x is more appropriate for bytes than %08x. Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 82fafe0429fe..6ded39bfdbdb 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -960,10 +960,10 @@ static void update_device_status(struct device *dev) verbose("Device %s OK: offered", dev->name); for (i = 0; i < dev->desc->feature_len; i++) - verbose(" %08x", get_feature_bits(dev)[i]); + verbose(" %02x", get_feature_bits(dev)[i]); verbose(", accepted"); for (i = 0; i < dev->desc->feature_len; i++) - verbose(" %08x", get_feature_bits(dev) + verbose(" %02x", get_feature_bits(dev) [dev->desc->feature_len+i]); if (dev->ready) -- cgit v1.2.2 From 34bdaab44dd5dac861b0d23bc29b147b569e5783 Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Fri, 13 Jun 2008 14:04:58 +0100 Subject: lguest: Don't leak /dev/zero fd Signed-off-by: Mark McLoughlin Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 6ded39bfdbdb..686e2d435c75 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -254,6 +254,7 @@ static void *map_zeroed_pages(unsigned int num) PROT_READ|PROT_WRITE|PROT_EXEC, MAP_PRIVATE, fd, 0); if (addr == MAP_FAILED) err(1, "Mmaping %u pages of /dev/zero", num); + close(fd); return addr; } -- cgit v1.2.2 From dec6a2be085f046d42eb0bdce95ecb73de526429 Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Tue, 29 Jul 2008 09:58:33 -0500 Subject: lguest: Support assigning a MAC address If you've got a nice DHCP configuration which maps MAC addresses to specific IP addresses, then you're going to want to start your guest with one of those MAC addresses. Also, in Fedora, we have persistent network interface naming based on the MAC address, so with randomly assigned addresses you're soon going to hit eth13. Who knows what will happen then! Allow assigning a MAC address to the network interface with e.g. --tunnet=bridge:eth0:00:FF:95:6B:DA:3D or: --tunnet=192.168.121.1:00:FF:95:6B:DA:3D which is pretty unintelligable, but ... (includes Rusty's minor rework) Signed-off-by: Mark McLoughlin Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 122 ++++++++++++++++++++++++++++++------------ 1 file changed, 89 insertions(+), 33 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 686e2d435c75..684d61191bee 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -1265,10 +1265,25 @@ static void setup_console(void) static u32 str2ip(const char *ipaddr) { - unsigned int byte[4]; + unsigned int b[4]; - sscanf(ipaddr, "%u.%u.%u.%u", &byte[0], &byte[1], &byte[2], &byte[3]); - return (byte[0] << 24) | (byte[1] << 16) | (byte[2] << 8) | byte[3]; + if (sscanf(ipaddr, "%u.%u.%u.%u", &b[0], &b[1], &b[2], &b[3]) != 4) + errx(1, "Failed to parse IP address '%s'", ipaddr); + return (b[0] << 24) | (b[1] << 16) | (b[2] << 8) | b[3]; +} + +static void str2mac(const char *macaddr, unsigned char mac[6]) +{ + unsigned int m[6]; + if (sscanf(macaddr, "%02x:%02x:%02x:%02x:%02x:%02x", + &m[0], &m[1], &m[2], &m[3], &m[4], &m[5]) != 6) + errx(1, "Failed to parse mac address '%s'", macaddr); + mac[0] = m[0]; + mac[1] = m[1]; + mac[2] = m[2]; + mac[3] = m[3]; + mac[4] = m[4]; + mac[5] = m[5]; } /* This code is "adapted" from libbridge: it attaches the Host end of the @@ -1289,6 +1304,7 @@ static void add_to_bridge(int fd, const char *if_name, const char *br_name) errx(1, "interface %s does not exist!", if_name); strncpy(ifr.ifr_name, br_name, IFNAMSIZ); + ifr.ifr_name[IFNAMSIZ-1] = '\0'; ifr.ifr_ifindex = ifidx; if (ioctl(fd, SIOCBRADDIF, &ifr) < 0) err(1, "can't add %s to bridge %s", if_name, br_name); @@ -1297,58 +1313,80 @@ static void add_to_bridge(int fd, const char *if_name, const char *br_name) /* This sets up the Host end of the network device with an IP address, brings * it up so packets will flow, the copies the MAC address into the hwaddr * pointer. */ -static void configure_device(int fd, const char *devname, u32 ipaddr, - unsigned char hwaddr[6]) +static void configure_device(int fd, const char *tapif, u32 ipaddr) { struct ifreq ifr; struct sockaddr_in *sin = (struct sockaddr_in *)&ifr.ifr_addr; - /* Don't read these incantations. Just cut & paste them like I did! */ memset(&ifr, 0, sizeof(ifr)); - strcpy(ifr.ifr_name, devname); + strcpy(ifr.ifr_name, tapif); + + /* Don't read these incantations. Just cut & paste them like I did! */ sin->sin_family = AF_INET; sin->sin_addr.s_addr = htonl(ipaddr); if (ioctl(fd, SIOCSIFADDR, &ifr) != 0) - err(1, "Setting %s interface address", devname); + err(1, "Setting %s interface address", tapif); ifr.ifr_flags = IFF_UP; if (ioctl(fd, SIOCSIFFLAGS, &ifr) != 0) - err(1, "Bringing interface %s up", devname); + err(1, "Bringing interface %s up", tapif); +} + +static void get_mac(int fd, const char *tapif, unsigned char hwaddr[6]) +{ + struct ifreq ifr; + + memset(&ifr, 0, sizeof(ifr)); + strcpy(ifr.ifr_name, tapif); /* SIOC stands for Socket I/O Control. G means Get (vs S for Set * above). IF means Interface, and HWADDR is hardware address. * Simple! */ if (ioctl(fd, SIOCGIFHWADDR, &ifr) != 0) - err(1, "getting hw address for %s", devname); + err(1, "getting hw address for %s", tapif); memcpy(hwaddr, ifr.ifr_hwaddr.sa_data, 6); } -/*L:195 Our network is a Host<->Guest network. This can either use bridging or - * routing, but the principle is the same: it uses the "tun" device to inject - * packets into the Host as if they came in from a normal network card. We - * just shunt packets between the Guest and the tun device. */ -static void setup_tun_net(const char *arg) +static int get_tun_device(char tapif[IFNAMSIZ]) { - struct device *dev; struct ifreq ifr; - int netfd, ipfd; - u32 ip; - const char *br_name = NULL; - struct virtio_net_config conf; + int netfd; + + /* Start with this zeroed. Messy but sure. */ + memset(&ifr, 0, sizeof(ifr)); /* We open the /dev/net/tun device and tell it we want a tap device. A * tap device is like a tun device, only somehow different. To tell * the truth, I completely blundered my way through this code, but it * works now! */ netfd = open_or_die("/dev/net/tun", O_RDWR); - memset(&ifr, 0, sizeof(ifr)); ifr.ifr_flags = IFF_TAP | IFF_NO_PI; strcpy(ifr.ifr_name, "tap%d"); if (ioctl(netfd, TUNSETIFF, &ifr) != 0) err(1, "configuring /dev/net/tun"); + /* We don't need checksums calculated for packets coming in this * device: trust us! */ ioctl(netfd, TUNSETNOCSUM, 1); + memcpy(tapif, ifr.ifr_name, IFNAMSIZ); + return netfd; +} + +/*L:195 Our network is a Host<->Guest network. This can either use bridging or + * routing, but the principle is the same: it uses the "tun" device to inject + * packets into the Host as if they came in from a normal network card. We + * just shunt packets between the Guest and the tun device. */ +static void setup_tun_net(char *arg) +{ + struct device *dev; + int netfd, ipfd; + u32 ip = INADDR_ANY; + bool bridging = false; + char tapif[IFNAMSIZ], *p; + struct virtio_net_config conf; + + netfd = get_tun_device(tapif); + /* First we create a new network device. */ dev = new_device("net", VIRTIO_ID_NET, netfd, handle_tun_input); @@ -1365,14 +1403,29 @@ static void setup_tun_net(const char *arg) /* If the command line was --tunnet=bridge: do bridging. */ if (!strncmp(BRIDGE_PFX, arg, strlen(BRIDGE_PFX))) { - ip = INADDR_ANY; - br_name = arg + strlen(BRIDGE_PFX); - add_to_bridge(ipfd, ifr.ifr_name, br_name); - } else /* It is an IP address to set up the device with */ + arg += strlen(BRIDGE_PFX); + bridging = true; + } + + /* A mac address may follow the bridge name or IP address */ + p = strchr(arg, ':'); + if (p) { + str2mac(p+1, conf.mac); + *p = '\0'; + } else { + p = arg + strlen(arg); + /* None supplied; query the randomly assigned mac. */ + get_mac(ipfd, tapif, conf.mac); + } + + /* arg is now either an IP address or a bridge name */ + if (bridging) + add_to_bridge(ipfd, tapif, arg); + else ip = str2ip(arg); - /* Set up the tun device, and get the mac address for the interface. */ - configure_device(ipfd, ifr.ifr_name, ip, conf.mac); + /* Set up the tun device. */ + configure_device(ipfd, tapif, ip); /* Tell Guest what MAC address to use. */ add_feature(dev, VIRTIO_NET_F_MAC); @@ -1382,11 +1435,14 @@ static void setup_tun_net(const char *arg) /* We don't need the socket any more; setup is done. */ close(ipfd); - verbose("device %u: tun net %u.%u.%u.%u\n", - devices.device_num++, - (u8)(ip>>24),(u8)(ip>>16),(u8)(ip>>8),(u8)ip); - if (br_name) - verbose("attached to bridge: %s\n", br_name); + devices.device_num++; + + if (bridging) + verbose("device %u: tun %s attached to bridge: %s\n", + devices.device_num, tapif, arg); + else + verbose("device %u: tun %s: %s\n", + devices.device_num, tapif, arg); } /* Our block (disk) device should be really simple: the Guest asks for a block @@ -1698,7 +1754,7 @@ static struct option opts[] = { static void usage(void) { errx(1, "Usage: lguest [--verbose] " - "[--tunnet=(|bridge:)\n" + "[--tunnet=(:|bridge::)\n" "|--block=|--initrd=]...\n" " vmlinux [args...]"); } -- cgit v1.2.2 From 28fd6d7f953711fbf67496701be05513052d967d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:33 -0500 Subject: lguest: virtio-rng support This is a simple patch to add support for the virtio "hardware random generator" to lguest. It gets about 1.2 MB/sec reading from /dev/hwrng in the guest. Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 90 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 684d61191bee..a1fca9db788e 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -41,6 +41,7 @@ #include "linux/virtio_net.h" #include "linux/virtio_blk.h" #include "linux/virtio_console.h" +#include "linux/virtio_rng.h" #include "linux/virtio_ring.h" #include "asm-x86/bootparam.h" /*L:110 We can ignore the 39 include files we need for this program, but I do @@ -199,6 +200,33 @@ static void *_convert(struct iovec *iov, size_t size, size_t align, #define le32_to_cpu(v32) (v32) #define le64_to_cpu(v64) (v64) +/* Is this iovec empty? */ +static bool iov_empty(const struct iovec iov[], unsigned int num_iov) +{ + unsigned int i; + + for (i = 0; i < num_iov; i++) + if (iov[i].iov_len) + return false; + return true; +} + +/* Take len bytes from the front of this iovec. */ +static void iov_consume(struct iovec iov[], unsigned num_iov, unsigned len) +{ + unsigned int i; + + for (i = 0; i < num_iov; i++) { + unsigned int used; + + used = iov[i].iov_len < len ? iov[i].iov_len : len; + iov[i].iov_base += used; + iov[i].iov_len -= used; + len -= used; + } + assert(len == 0); +} + /* The device virtqueue descriptors are followed by feature bitmasks. */ static u8 *get_feature_bits(struct device *dev) { @@ -1678,6 +1706,64 @@ static void setup_block_file(const char *filename) verbose("device %u: virtblock %llu sectors\n", devices.device_num, le64_to_cpu(conf.capacity)); } + +/* Our random number generator device reads from /dev/random into the Guest's + * input buffers. The usual case is that the Guest doesn't want random numbers + * and so has no buffers although /dev/random is still readable, whereas + * console is the reverse. + * + * The same logic applies, however. */ +static bool handle_rng_input(int fd, struct device *dev) +{ + int len; + unsigned int head, in_num, out_num, totlen = 0; + struct iovec iov[dev->vq->vring.num]; + + /* First we need a buffer from the Guests's virtqueue. */ + head = get_vq_desc(dev->vq, iov, &out_num, &in_num); + + /* If they're not ready for input, stop listening to this file + * descriptor. We'll start again once they add an input buffer. */ + if (head == dev->vq->vring.num) + return false; + + if (out_num) + errx(1, "Output buffers in rng?"); + + /* This is why we convert to iovecs: the readv() call uses them, and so + * it reads straight into the Guest's buffer. We loop to make sure we + * fill it. */ + while (!iov_empty(iov, in_num)) { + len = readv(dev->fd, iov, in_num); + if (len <= 0) + err(1, "Read from /dev/random gave %i", len); + iov_consume(iov, in_num, len); + totlen += len; + } + + /* Tell the Guest about the new input. */ + add_used_and_trigger(fd, dev->vq, head, totlen); + + /* Everything went OK! */ + return true; +} + +/* And this creates a "hardware" random number device for the Guest. */ +static void setup_rng(void) +{ + struct device *dev; + int fd; + + fd = open_or_die("/dev/random", O_RDONLY); + + /* The device responds to return from I/O thread. */ + dev = new_device("rng", VIRTIO_ID_RNG, fd, handle_rng_input); + + /* The device has one virtqueue, where the Guest places inbufs. */ + add_virtqueue(dev, VIRTQUEUE_NUM, enable_fd); + + verbose("device %u: rng\n", devices.device_num++); +} /* That's the end of device setup. */ /*L:230 Reboot is pretty easy: clean up and exec() the Launcher afresh. */ @@ -1748,6 +1834,7 @@ static struct option opts[] = { { "verbose", 0, NULL, 'v' }, { "tunnet", 1, NULL, 't' }, { "block", 1, NULL, 'b' }, + { "rng", 0, NULL, 'r' }, { "initrd", 1, NULL, 'i' }, { NULL }, }; @@ -1822,6 +1909,9 @@ int main(int argc, char *argv[]) case 'b': setup_block_file(optarg); break; + case 'r': + setup_rng(); + break; case 'i': initrd_name = optarg; break; -- cgit v1.2.2 From cf485e566bc4a8098680162e1cc2ac1dfbef8a3c Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Mon, 9 Jun 2008 16:22:48 -0700 Subject: lguest: use cpu capability accessors To support my little make-x86-bitops-use-proper-typechecking projectlet. Cc: Thomas Gleixner Cc: Andrea Arcangeli Signed-off-by: Andrew Morton Acked-by: Ingo Molnar Signed-off-by: Rusty Russell --- drivers/lguest/x86/core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/lguest/x86/core.c b/drivers/lguest/x86/core.c index 95dfda52b4f9..bf7942327bda 100644 --- a/drivers/lguest/x86/core.c +++ b/drivers/lguest/x86/core.c @@ -480,7 +480,7 @@ void __init lguest_arch_host_init(void) * bit on its CPU, depending on the argument (0 == unset). */ on_each_cpu(adjust_pge, (void *)0, 1); /* Turn off the feature in the global feature set. */ - clear_bit(X86_FEATURE_PGE, boot_cpu_data.x86_capability); + clear_cpu_cap(&boot_cpu_data, X86_FEATURE_PGE); } put_online_cpus(); }; @@ -491,7 +491,7 @@ void __exit lguest_arch_host_fini(void) /* If we had PGE before we started, turn it back on now. */ get_online_cpus(); if (cpu_had_pge) { - set_bit(X86_FEATURE_PGE, boot_cpu_data.x86_capability); + set_cpu_cap(&boot_cpu_data, X86_FEATURE_PGE); /* adjust_pge's argument "1" means set PGE. */ on_each_cpu(adjust_pge, (void *)1, 1); } -- cgit v1.2.2 From b5111790fa6695b1502d4f5d389f6b22b9de10c3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:34 -0500 Subject: lguest: wrap last_avail accesses. To simplify the transition to when we publish indices in the ring (and make shuffling my patch queue easier), wrap them in a lg_last_avail() macro. Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index a1fca9db788e..31a688e105ca 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -191,6 +191,9 @@ static void *_convert(struct iovec *iov, size_t size, size_t align, return iov->iov_base; } +/* Wrapper for the last available index. Makes it easier to change. */ +#define lg_last_avail(vq) ((vq)->last_avail_idx) + /* The virtio configuration space is defined to be little-endian. x86 is * little-endian too, but it's nice to be explicit so we have these helpers. */ #define cpu_to_le16(v16) (v16) @@ -690,19 +693,22 @@ static unsigned get_vq_desc(struct virtqueue *vq, unsigned int *out_num, unsigned int *in_num) { unsigned int i, head; + u16 last_avail; /* Check it isn't doing very strange things with descriptor numbers. */ - if ((u16)(vq->vring.avail->idx - vq->last_avail_idx) > vq->vring.num) + last_avail = lg_last_avail(vq); + if ((u16)(vq->vring.avail->idx - last_avail) > vq->vring.num) errx(1, "Guest moved used index from %u to %u", - vq->last_avail_idx, vq->vring.avail->idx); + last_avail, vq->vring.avail->idx); /* If there's nothing new since last we looked, return invalid. */ - if (vq->vring.avail->idx == vq->last_avail_idx) + if (vq->vring.avail->idx == last_avail) return vq->vring.num; /* Grab the next descriptor number they're advertising, and increment * the index we've seen. */ - head = vq->vring.avail->ring[vq->last_avail_idx++ % vq->vring.num]; + head = vq->vring.avail->ring[last_avail % vq->vring.num]; + lg_last_avail(vq)++; /* If their number is silly, that's a fatal mistake. */ if (head >= vq->vring.num) @@ -980,7 +986,7 @@ static void update_device_status(struct device *dev) for (vq = dev->vq; vq; vq = vq->next) { memset(vq->vring.desc, 0, vring_size(vq->config.num, getpagesize())); - vq->last_avail_idx = 0; + lg_last_avail(vq) = 0; } } else if (dev->desc->status & VIRTIO_CONFIG_S_FAILED) { warnx("Device %s configuration FAILED", dev->name); -- cgit v1.2.2 From 5dae785a82c1a8c05b5b4f9709bd9ce658dcf1b6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:35 -0500 Subject: lguest: net block unneeded receive queue update notifications Number of exits transmitting 10GB Guest->Host before: network xmit 7858610 recv 118136 After: network xmit 7750233 recv 1 Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 31a688e105ca..46f4c5b09e9e 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -933,6 +933,11 @@ static bool handle_tun_input(int fd, struct device *dev) /* FIXME: Actually want DRIVER_ACTIVE here. */ if (dev->desc->status & VIRTIO_CONFIG_S_DRIVER_OK) warn("network: no dma buffer!"); + + /* Now tell it we want to know if new things appear. */ + dev->vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY; + wmb(); + /* We'll turn this back on if input buffers are registered. */ return false; } else if (out_num) @@ -969,6 +974,13 @@ static void enable_fd(int fd, struct virtqueue *vq) write(waker_fd, &vq->dev->fd, sizeof(vq->dev->fd)); } +static void net_enable_fd(int fd, struct virtqueue *vq) +{ + /* We don't need to know again when Guest refills receive buffer. */ + vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY; + enable_fd(fd, vq); +} + /* When the Guest tells us they updated the status field, we handle it. */ static void update_device_status(struct device *dev) { @@ -1426,7 +1438,7 @@ static void setup_tun_net(char *arg) /* Network devices need a receive and a send queue, just like * console. */ - add_virtqueue(dev, VIRTQUEUE_NUM, enable_fd); + add_virtqueue(dev, VIRTQUEUE_NUM, net_enable_fd); add_virtqueue(dev, VIRTQUEUE_NUM, handle_net_output); /* We need a socket to perform the magic network ioctls to bring up the -- cgit v1.2.2 From a161883a29bf6100efe7b5346bec274e5023c29c Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:35 -0500 Subject: lguest: Tell Guest net not to notify us on every packet xmit virtio_ring has the ability to suppress notifications. This prevents a guest exit for every packet, but we need to set a timer on packet receipt to re-check if there were any remaining packets. Here are the times for 1G TCP Guest->Host with different timeout settings (it matters because the TCP window doesn't grow big enough to fill the entire buffer): Timeout value Seconds Xmit/Recv/Timeout None (before) 25.3784 xmit 7750233 recv 1 2500 usec 62.5119 xmit 207020 recv 2 timeout 207020 1000 usec 34.5379 xmit 207003 recv 2 timeout 207003 750 usec 29.2305 xmit 207002 recv 1 timeout 207002 500 usec 19.1887 xmit 561141 recv 1 timeout 559657 250 usec 20.0465 xmit 214128 recv 2 timeout 214110 100 usec 19.2583 xmit 561621 recv 1 timeout 560153 (Note that these values are sensitive to the GSO patches which come later, and probably other traffic-related variables, so take with a large grain of salt). Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 106 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 93 insertions(+), 13 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 46f4c5b09e9e..018472cee151 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -36,6 +36,7 @@ #include #include #include +#include #include "linux/lguest_launcher.h" #include "linux/virtio_config.h" #include "linux/virtio_net.h" @@ -81,6 +82,8 @@ static int waker_fd; static void *guest_base; /* The maximum guest physical address allowed, and maximum possible. */ static unsigned long guest_limit, guest_max; +/* The pipe for signal hander to write to. */ +static int timeoutpipe[2]; /* a per-cpu variable indicating whose vcpu is currently running */ static unsigned int __thread cpu_id; @@ -156,11 +159,14 @@ struct virtqueue /* Last available index we saw. */ u16 last_avail_idx; - /* The routine to call when the Guest pings us. */ - void (*handle_output)(int fd, struct virtqueue *me); + /* The routine to call when the Guest pings us, or timeout. */ + void (*handle_output)(int fd, struct virtqueue *me, bool timeout); /* Outstanding buffers */ unsigned int inflight; + + /* Is this blocked awaiting a timer? */ + bool blocked; }; /* Remember the arguments to the program so we can "reboot" */ @@ -874,7 +880,7 @@ static bool handle_console_input(int fd, struct device *dev) /* Handling output for console is simple: we just get all the output buffers * and write them to stdout. */ -static void handle_console_output(int fd, struct virtqueue *vq) +static void handle_console_output(int fd, struct virtqueue *vq, bool timeout) { unsigned int head, out, in; int len; @@ -889,6 +895,21 @@ static void handle_console_output(int fd, struct virtqueue *vq) } } +static void block_vq(struct virtqueue *vq) +{ + struct itimerval itm; + + vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY; + vq->blocked = true; + + itm.it_interval.tv_sec = 0; + itm.it_interval.tv_usec = 0; + itm.it_value.tv_sec = 0; + itm.it_value.tv_usec = 500; + + setitimer(ITIMER_REAL, &itm, NULL); +} + /* * The Network * @@ -896,9 +917,9 @@ static void handle_console_output(int fd, struct virtqueue *vq) * and write them (ignoring the first element) to this device's file descriptor * (/dev/net/tun). */ -static void handle_net_output(int fd, struct virtqueue *vq) +static void handle_net_output(int fd, struct virtqueue *vq, bool timeout) { - unsigned int head, out, in; + unsigned int head, out, in, num = 0; int len; struct iovec iov[vq->vring.num]; @@ -912,7 +933,12 @@ static void handle_net_output(int fd, struct virtqueue *vq) (void)convert(&iov[0], struct virtio_net_hdr); len = writev(vq->dev->fd, iov+1, out-1); add_used_and_trigger(fd, vq, head, len); + num++; } + + /* Block further kicks and set up a timer if we saw anything. */ + if (!timeout && num) + block_vq(vq); } /* This is where we handle a packet coming in from the tun device to our @@ -967,18 +993,18 @@ static bool handle_tun_input(int fd, struct device *dev) /*L:215 This is the callback attached to the network and console input * virtqueues: it ensures we try again, in case we stopped console or net * delivery because Guest didn't have any buffers. */ -static void enable_fd(int fd, struct virtqueue *vq) +static void enable_fd(int fd, struct virtqueue *vq, bool timeout) { add_device_fd(vq->dev->fd); /* Tell waker to listen to it again */ write(waker_fd, &vq->dev->fd, sizeof(vq->dev->fd)); } -static void net_enable_fd(int fd, struct virtqueue *vq) +static void net_enable_fd(int fd, struct virtqueue *vq, bool timeout) { /* We don't need to know again when Guest refills receive buffer. */ vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY; - enable_fd(fd, vq); + enable_fd(fd, vq, timeout); } /* When the Guest tells us they updated the status field, we handle it. */ @@ -1047,7 +1073,7 @@ static void handle_output(int fd, unsigned long addr) if (strcmp(vq->dev->name, "console") != 0) verbose("Output to %s\n", vq->dev->name); if (vq->handle_output) - vq->handle_output(fd, vq); + vq->handle_output(fd, vq, false); return; } } @@ -1061,6 +1087,29 @@ static void handle_output(int fd, unsigned long addr) strnlen(from_guest_phys(addr), guest_limit - addr)); } +static void handle_timeout(int fd) +{ + char buf[32]; + struct device *i; + struct virtqueue *vq; + + /* Clear the pipe */ + read(timeoutpipe[0], buf, sizeof(buf)); + + /* Check each device and virtqueue: flush blocked ones. */ + for (i = devices.dev; i; i = i->next) { + for (vq = i->vq; vq; vq = vq->next) { + if (!vq->blocked) + continue; + + vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY; + vq->blocked = false; + if (vq->handle_output) + vq->handle_output(fd, vq, true); + } + } +} + /* This is called when the Waker wakes us up: check for incoming file * descriptors. */ static void handle_input(int fd) @@ -1071,9 +1120,14 @@ static void handle_input(int fd) for (;;) { struct device *i; fd_set fds = devices.infds; + int num; + num = select(devices.max_infd+1, &fds, NULL, NULL, &poll); + /* Could get interrupted */ + if (num < 0) + continue; /* If nothing is ready, we're done. */ - if (select(devices.max_infd+1, &fds, NULL, NULL, &poll) == 0) + if (num == 0) break; /* Otherwise, call the device(s) which have readable file @@ -1097,6 +1151,10 @@ static void handle_input(int fd) write(waker_fd, &dev_fd, sizeof(dev_fd)); } } + + /* Is this the timeout fd? */ + if (FD_ISSET(timeoutpipe[0], &fds)) + handle_timeout(fd); } } @@ -1145,7 +1203,7 @@ static struct lguest_device_desc *new_dev_desc(u16 type) /* Each device descriptor is followed by the description of its virtqueues. We * specify how many descriptors the virtqueue is to have. */ static void add_virtqueue(struct device *dev, unsigned int num_descs, - void (*handle_output)(int fd, struct virtqueue *me)) + void (*handle_output)(int, struct virtqueue *, bool)) { unsigned int pages; struct virtqueue **i, *vq = malloc(sizeof(*vq)); @@ -1161,6 +1219,7 @@ static void add_virtqueue(struct device *dev, unsigned int num_descs, vq->last_avail_idx = 0; vq->dev = dev; vq->inflight = 0; + vq->blocked = false; /* Initialize the configuration. */ vq->config.num = num_descs; @@ -1293,6 +1352,24 @@ static void setup_console(void) } /*:*/ +static void timeout_alarm(int sig) +{ + write(timeoutpipe[1], "", 1); +} + +static void setup_timeout(void) +{ + if (pipe(timeoutpipe) != 0) + err(1, "Creating timeout pipe"); + + if (fcntl(timeoutpipe[1], F_SETFL, + fcntl(timeoutpipe[1], F_GETFL) | O_NONBLOCK) != 0) + err(1, "Making timeout pipe nonblocking"); + + add_device_fd(timeoutpipe[0]); + signal(SIGALRM, timeout_alarm); +} + /*M:010 Inter-guest networking is an interesting area. Simplest is to have a * --sharenet= option which opens or creates a named pipe. This can be * used to send packets to another guest in a 1:1 manner. @@ -1653,7 +1730,7 @@ static bool handle_io_finish(int fd, struct device *dev) } /* When the Guest submits some I/O, we just need to wake the I/O thread. */ -static void handle_virtblk_output(int fd, struct virtqueue *vq) +static void handle_virtblk_output(int fd, struct virtqueue *vq, bool timeout) { struct vblk_info *vblk = vq->dev->priv; char c = 0; @@ -1824,7 +1901,7 @@ static void __attribute__((noreturn)) run_guest(int lguest_fd) /* ERESTART means that we need to reboot the guest */ } else if (errno == ERESTART) { restart_guest(); - /* EAGAIN means the Waker wanted us to look at some input. + /* EAGAIN means a signal (timeout). * Anything else means a bug or incompatible change. */ } else if (errno != EAGAIN) err(1, "Running guest failed"); @@ -1948,6 +2025,9 @@ int main(int argc, char *argv[]) /* We always have a console device */ setup_console(); + /* We can timeout waiting for Guest network transmit. */ + setup_timeout(); + /* Now we load the kernel */ start = load_kernel(open_or_die(argv[optind+1], O_RDONLY)); -- cgit v1.2.2 From aa1249840bfc8d62431eed5796bf99887b963ab6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:36 -0500 Subject: lguest: Adaptive timeout Since the correct timeout value varies, use a heuristic which adjusts the timeout depending on how many packets we've seen. This gives slightly worse results, but doesn't need tweaking when GSO is introduced. 500 usec 19.1887 xmit 561141 recv 1 timeout 559657 Dynamic (278) 20.1974 xmit 214510 recv 5 timeout 214491 usec 278 Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 018472cee151..b2bbbb7f8c57 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -84,6 +84,7 @@ static void *guest_base; static unsigned long guest_limit, guest_max; /* The pipe for signal hander to write to. */ static int timeoutpipe[2]; +static unsigned int timeout_usec = 500; /* a per-cpu variable indicating whose vcpu is currently running */ static unsigned int __thread cpu_id; @@ -905,7 +906,7 @@ static void block_vq(struct virtqueue *vq) itm.it_interval.tv_sec = 0; itm.it_interval.tv_usec = 0; itm.it_value.tv_sec = 0; - itm.it_value.tv_usec = 500; + itm.it_value.tv_usec = timeout_usec; setitimer(ITIMER_REAL, &itm, NULL); } @@ -922,6 +923,7 @@ static void handle_net_output(int fd, struct virtqueue *vq, bool timeout) unsigned int head, out, in, num = 0; int len; struct iovec iov[vq->vring.num]; + static int last_timeout_num; /* Keep getting output buffers from the Guest until we run out. */ while ((head = get_vq_desc(vq, iov, &out, &in)) != vq->vring.num) { @@ -939,6 +941,14 @@ static void handle_net_output(int fd, struct virtqueue *vq, bool timeout) /* Block further kicks and set up a timer if we saw anything. */ if (!timeout && num) block_vq(vq); + + if (timeout) { + if (num < last_timeout_num) + timeout_usec += 10; + else if (timeout_usec > 1) + timeout_usec--; + last_timeout_num = num; + } } /* This is where we handle a packet coming in from the tun device to our -- cgit v1.2.2 From 9254926f85466979ef5f0e16386c294bf0973a90 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:36 -0500 Subject: lguest: Remove 'network: no dma buffer!' warning This warning can happen a lot under load, and it should be warnx not warn anwyay. Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index b2bbbb7f8c57..0d1b0265d8e2 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -967,8 +967,6 @@ static bool handle_tun_input(int fd, struct device *dev) * early, the Guest won't be ready yet. Wait until the device * status says it's ready. */ /* FIXME: Actually want DRIVER_ACTIVE here. */ - if (dev->desc->status & VIRTIO_CONFIG_S_DRIVER_OK) - warn("network: no dma buffer!"); /* Now tell it we want to know if new things appear. */ dev->vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY; -- cgit v1.2.2 From 398f187d74b89d5ab198fcf9b8d86edbefecec4d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:37 -0500 Subject: lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap Guest -> Host 1GB TCP: Before 20.1974 seconds xmit 214510 recv 5 timeout 214491 usec 278 After 8.43625 seconds xmit 95640 recv 198266 timeout 49771 usec 1252 Host -> Guest 1GB TCP: Before: Seconds 9.98854 xmit 172166 recv 5344 timeout 172157 usec 251 After: Seconds 5.72803 xmit 244322 recv 9919 timeout 244302 usec 156 Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index 0d1b0265d8e2..dc49f50e04ac 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -929,11 +929,9 @@ static void handle_net_output(int fd, struct virtqueue *vq, bool timeout) while ((head = get_vq_desc(vq, iov, &out, &in)) != vq->vring.num) { if (in) errx(1, "Input buffers in output queue?"); - /* Check header, but otherwise ignore it (we told the Guest we - * supported no features, so it shouldn't have anything - * interesting). */ - (void)convert(&iov[0], struct virtio_net_hdr); - len = writev(vq->dev->fd, iov+1, out-1); + len = writev(vq->dev->fd, iov, out); + if (len < 0) + err(1, "Writing network packet to tun"); add_used_and_trigger(fd, vq, head, len); num++; } @@ -958,7 +956,6 @@ static bool handle_tun_input(int fd, struct device *dev) unsigned int head, in_num, out_num; int len; struct iovec iov[dev->vq->vring.num]; - struct virtio_net_hdr *hdr; /* First we need a network buffer from the Guests's recv virtqueue. */ head = get_vq_desc(dev->vq, iov, &out_num, &in_num); @@ -977,18 +974,13 @@ static bool handle_tun_input(int fd, struct device *dev) } else if (out_num) errx(1, "Output buffers in network recv queue?"); - /* First element is the header: we set it to 0 (no features). */ - hdr = convert(&iov[0], struct virtio_net_hdr); - hdr->flags = 0; - hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE; - /* Read the packet from the device directly into the Guest's buffer. */ - len = readv(dev->fd, iov+1, in_num-1); + len = readv(dev->fd, iov, in_num); if (len <= 0) err(1, "reading network"); /* Tell the Guest about the new packet. */ - add_used_and_trigger(fd, dev->vq, head, sizeof(*hdr) + len); + add_used_and_trigger(fd, dev->vq, head, len); verbose("tun input packet len %i [%02x %02x] (%s)\n", len, ((u8 *)iov[1].iov_base)[0], ((u8 *)iov[1].iov_base)[1], @@ -1490,11 +1482,15 @@ static int get_tun_device(char tapif[IFNAMSIZ]) * the truth, I completely blundered my way through this code, but it * works now! */ netfd = open_or_die("/dev/net/tun", O_RDWR); - ifr.ifr_flags = IFF_TAP | IFF_NO_PI; + ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR; strcpy(ifr.ifr_name, "tap%d"); if (ioctl(netfd, TUNSETIFF, &ifr) != 0) err(1, "configuring /dev/net/tun"); + if (ioctl(netfd, TUNSETOFFLOAD, + TUN_F_CSUM|TUN_F_TSO4|TUN_F_TSO6|TUN_F_TSO_ECN) != 0) + err(1, "Could not set features for tun device"); + /* We don't need checksums calculated for packets coming in this * device: trust us! */ ioctl(netfd, TUNSETNOCSUM, 1); @@ -1561,6 +1557,16 @@ static void setup_tun_net(char *arg) /* Tell Guest what MAC address to use. */ add_feature(dev, VIRTIO_NET_F_MAC); add_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY); + /* Expect Guest to handle everything except UFO */ + add_feature(dev, VIRTIO_NET_F_CSUM); + add_feature(dev, VIRTIO_NET_F_GUEST_CSUM); + add_feature(dev, VIRTIO_NET_F_MAC); + add_feature(dev, VIRTIO_NET_F_GUEST_TSO4); + add_feature(dev, VIRTIO_NET_F_GUEST_TSO6); + add_feature(dev, VIRTIO_NET_F_GUEST_ECN); + add_feature(dev, VIRTIO_NET_F_HOST_TSO4); + add_feature(dev, VIRTIO_NET_F_HOST_TSO6); + add_feature(dev, VIRTIO_NET_F_HOST_ECN); set_config(dev, sizeof(conf), &conf); /* We don't need the socket any more; setup is done. */ -- cgit v1.2.2 From 0f0c4fab8284f3b886b2e1e0e317e3bb8de176b3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:37 -0500 Subject: lguest: Enlarge virtio rings With big packets, 128 entries is a little small. Guest -> Host 1GB TCP: Before: 8.43625 seconds xmit 95640 recv 198266 timeout 49771 usec 1252 After: 8.01099 seconds xmit 49200 recv 102263 timeout 26014 usec 2118 Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index dc49f50e04ac..f9bba2d8fee1 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -66,8 +66,8 @@ typedef uint8_t u8; #endif /* We can have up to 256 pages for devices. */ #define DEVICE_PAGES 256 -/* This will occupy 2 pages: it must be a power of 2. */ -#define VIRTQUEUE_NUM 128 +/* This will occupy 3 pages: it must be a power of 2. */ +#define VIRTQUEUE_NUM 256 /*L:120 verbose is both a global flag and a macro. The C preprocessor allows * this, and although I wouldn't recommend it, it works quite nicely here. */ -- cgit v1.2.2 From 8c79873da0d2bedf4ad6b868c54e426bb0a2fe38 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 29 Jul 2008 09:58:38 -0500 Subject: lguest: turn Waker into a thread, not a process lguest uses a Waker process to break it out of the kernel (ie. actually running the guest) when file descriptor needs attention. Changing this from a process to a thread somewhat simplifies things: it can directly access the fd_set of things to watch. More importantly, it means that the Waker can see Guest memory correctly, so /dev/vring file descriptors will work as anticipated (the alternative is to actually mmap MAP_SHARED, but you can't do that with /dev/zero). Signed-off-by: Rusty Russell --- Documentation/lguest/lguest.c | 120 ++++++++++++++++++++---------------------- 1 file changed, 57 insertions(+), 63 deletions(-) diff --git a/Documentation/lguest/lguest.c b/Documentation/lguest/lguest.c index f9bba2d8fee1..b88b0ea54e90 100644 --- a/Documentation/lguest/lguest.c +++ b/Documentation/lguest/lguest.c @@ -76,8 +76,12 @@ static bool verbose; do { if (verbose) printf(args); } while(0) /*:*/ -/* The pipe to send commands to the waker process */ -static int waker_fd; +/* File descriptors for the Waker. */ +struct { + int pipe[2]; + int lguest_fd; +} waker_fds; + /* The pointer to the start of guest memory. */ static void *guest_base; /* The maximum guest physical address allowed, and maximum possible. */ @@ -579,69 +583,64 @@ static void add_device_fd(int fd) * watch, but handing a file descriptor mask through to the kernel is fairly * icky. * - * Instead, we fork off a process which watches the file descriptors and writes + * Instead, we clone off a thread which watches the file descriptors and writes * the LHREQ_BREAK command to the /dev/lguest file descriptor to tell the Host * stop running the Guest. This causes the Launcher to return from the * /dev/lguest read with -EAGAIN, where it will write to /dev/lguest to reset * the LHREQ_BREAK and wake us up again. * * This, of course, is merely a different *kind* of icky. + * + * Given my well-known antipathy to threads, I'd prefer to use processes. But + * it's easier to share Guest memory with threads, and trivial to share the + * devices.infds as the Launcher changes it. */ -static void wake_parent(int pipefd, int lguest_fd) +static int waker(void *unused) { - /* Add the pipe from the Launcher to the fdset in the device_list, so - * we watch it, too. */ - add_device_fd(pipefd); + /* Close the write end of the pipe: only the Launcher has it open. */ + close(waker_fds.pipe[1]); for (;;) { fd_set rfds = devices.infds; unsigned long args[] = { LHREQ_BREAK, 1 }; + unsigned int maxfd = devices.max_infd; + + /* We also listen to the pipe from the Launcher. */ + FD_SET(waker_fds.pipe[0], &rfds); + if (waker_fds.pipe[0] > maxfd) + maxfd = waker_fds.pipe[0]; /* Wait until input is ready from one of the devices. */ - select(devices.max_infd+1, &rfds, NULL, NULL, NULL); - /* Is it a message from the Launcher? */ - if (FD_ISSET(pipefd, &rfds)) { - int fd; - /* If read() returns 0, it means the Launcher has - * exited. We silently follow. */ - if (read(pipefd, &fd, sizeof(fd)) == 0) - exit(0); - /* Otherwise it's telling us to change what file - * descriptors we're to listen to. Positive means - * listen to a new one, negative means stop - * listening. */ - if (fd >= 0) - FD_SET(fd, &devices.infds); - else - FD_CLR(-fd - 1, &devices.infds); - } else /* Send LHREQ_BREAK command. */ - pwrite(lguest_fd, args, sizeof(args), cpu_id); + select(maxfd+1, &rfds, NULL, NULL, NULL); + + /* Message from Launcher? */ + if (FD_ISSET(waker_fds.pipe[0], &rfds)) { + char c; + /* If this fails, then assume Launcher has exited. + * Don't do anything on exit: we're just a thread! */ + if (read(waker_fds.pipe[0], &c, 1) != 1) + _exit(0); + continue; + } + + /* Send LHREQ_BREAK command to snap the Launcher out of it. */ + pwrite(waker_fds.lguest_fd, args, sizeof(args), cpu_id); } + return 0; } /* This routine just sets up a pipe to the Waker process. */ -static int setup_waker(int lguest_fd) -{ - int pipefd[2], child; - - /* We create a pipe to talk to the Waker, and also so it knows when the - * Launcher dies (and closes pipe). */ - pipe(pipefd); - child = fork(); - if (child == -1) - err(1, "forking"); - - if (child == 0) { - /* We are the Waker: close the "writing" end of our copy of the - * pipe and start waiting for input. */ - close(pipefd[1]); - wake_parent(pipefd[0], lguest_fd); - } - /* Close the reading end of our copy of the pipe. */ - close(pipefd[0]); +static void setup_waker(int lguest_fd) +{ + /* This pipe is closed when Launcher dies, telling Waker. */ + if (pipe(waker_fds.pipe) != 0) + err(1, "Creating pipe for Waker"); - /* Here is the fd used to talk to the waker. */ - return pipefd[1]; + /* Waker also needs to know the lguest fd */ + waker_fds.lguest_fd = lguest_fd; + + if (clone(waker, malloc(4096) + 4096, CLONE_VM | SIGCHLD, NULL) == -1) + err(1, "Creating Waker"); } /* @@ -863,8 +862,8 @@ static bool handle_console_input(int fd, struct device *dev) unsigned long args[] = { LHREQ_BREAK, 0 }; /* Close the fd so Waker will know it has to * exit. */ - close(waker_fd); - /* Just in case waker is blocked in BREAK, send + close(waker_fds.pipe[1]); + /* Just in case Waker is blocked in BREAK, send * unbreak now. */ write(fd, args, sizeof(args)); exit(2); @@ -996,8 +995,8 @@ static bool handle_tun_input(int fd, struct device *dev) static void enable_fd(int fd, struct virtqueue *vq, bool timeout) { add_device_fd(vq->dev->fd); - /* Tell waker to listen to it again */ - write(waker_fd, &vq->dev->fd, sizeof(vq->dev->fd)); + /* Snap the Waker out of its select loop. */ + write(waker_fds.pipe[1], "", 1); } static void net_enable_fd(int fd, struct virtqueue *vq, bool timeout) @@ -1134,7 +1133,6 @@ static void handle_input(int fd) * descriptors and a method of handling them. */ for (i = devices.dev; i; i = i->next) { if (i->handle_input && FD_ISSET(i->fd, &fds)) { - int dev_fd; if (i->handle_input(fd, i)) continue; @@ -1144,11 +1142,6 @@ static void handle_input(int fd) * buffers to deliver into. Console also uses * it when it discovers that stdin is closed. */ FD_CLR(i->fd, &devices.infds); - /* Tell waker to ignore it too, by sending a - * negative fd number (-1, since 0 is a valid - * FD number). */ - dev_fd = -i->fd - 1; - write(waker_fd, &dev_fd, sizeof(dev_fd)); } } @@ -1880,11 +1873,12 @@ static void __attribute__((noreturn)) restart_guest(void) { unsigned int i; - /* Closing pipes causes the Waker thread and io_threads to die, and - * closing /dev/lguest cleans up the Guest. Since we don't track all - * open fds, we simply close everything beyond stderr. */ + /* Since we don't track all open fds, we simply close everything beyond + * stderr. */ for (i = 3; i < FD_SETSIZE; i++) close(i); + + /* The exec automatically gets rid of the I/O and Waker threads. */ execv(main_args[0], main_args); err(1, "Could not exec %s", main_args[0]); } @@ -2085,10 +2079,10 @@ int main(int argc, char *argv[]) * /dev/lguest file descriptor. */ lguest_fd = tell_kernel(pgdir, start); - /* We fork off a child process, which wakes the Launcher whenever one - * of the input file descriptors needs attention. We call this the - * Waker, and we'll cover it in a moment. */ - waker_fd = setup_waker(lguest_fd); + /* We clone off a thread, which wakes the Launcher whenever one of the + * input file descriptors needs attention. We call this the Waker, and + * we'll cover it in a moment. */ + setup_waker(lguest_fd); /* Finally, run the Guest. This doesn't return. */ run_guest(lguest_fd); -- cgit v1.2.2