Merge git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-for-linus

* git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux-2.6-for-linus:
  lguest and virtio: cleanup struct definitions to Linux style.
  lguest: update commentry
  lguest: fix comment style
  virtio: refactor find_vqs
  virtio: delete vq from list
  virtio: fix memory leak on device removal
  lguest: fix descriptor corruption in example launcher
  lguest: dereferencing freed mem in add_eventfd()

arch/x86/include/asm/lguest.h

@@ -17,8 +17,7 @@
 /* Pages for switcher itself, then two pages per cpu */
 #define TOTAL_SWITCHER_PAGES (SHARED_SWITCHER_PAGES + 2 * nr_cpu_ids)
 
-/* We map at -4M (-2M when PAE is activated) for ease of mapping
- * into the guest (one PTE page). */
+/* We map at -4M (-2M for PAE) for ease of mapping (one PTE page). */
 #ifdef CONFIG_X86_PAE
 #define SWITCHER_ADDR 0xFFE00000
 #else

arch/x86/include/asm/lguest_hcall.h

@@ -30,27 +30,27 @@
 #include <asm/hw_irq.h>
 #include <asm/kvm_para.h>
 
-/*G:030 But first, how does our Guest contact the Host to ask for privileged
+/*G:030
+ * But first, how does our Guest contact the Host to ask for privileged
  * operations?  There are two ways: the direct way is to make a "hypercall",
  * to make requests of the Host Itself.
  *
- * We use the KVM hypercall mechanism. Seventeen hypercalls are
- * available: the hypercall number is put in the %eax register, and the
- * arguments (when required) are placed in %ebx, %ecx, %edx and %esi.
- * If a return value makes sense, it's returned in %eax.
+ * We use the KVM hypercall mechanism, though completely different hypercall
+ * numbers. Seventeen hypercalls are available: the hypercall number is put in
+ * the %eax register, and the arguments (when required) are placed in %ebx,
+ * %ecx, %edx and %esi.  If a return value makes sense, it's returned in %eax.
  *
  * Grossly invalid calls result in Sudden Death at the hands of the vengeful
  * Host, rather than returning failure.  This reflects Winston Churchill's
- * definition of a gentleman: "someone who is only rude intentionally". */
-/*:*/
+ * definition of a gentleman: "someone who is only rude intentionally".
+:*/
 
 /* Can't use our min() macro here: needs to be a constant */
 #define LGUEST_IRQS (NR_IRQS < 32 ? NR_IRQS: 32)
 
 #define LHCALL_RING_SIZE 64
 struct hcall_args {
-	/* These map directly onto eax, ebx, ecx, edx and esi
-	 * in struct lguest_regs */
+	/* These map directly onto eax/ebx/ecx/edx/esi in struct lguest_regs */
 	unsigned long arg0, arg1, arg2, arg3, arg4;
 };
 

arch/x86/lguest/i386_head.S

@@ -5,7 +5,8 @@
 #include <asm/thread_info.h>
 #include <asm/processor-flags.h>
 
-/*G:020 Our story starts with the kernel booting into startup_32 in
+/*G:020
+ * Our story starts with the kernel booting into startup_32 in
  * arch/x86/kernel/head_32.S.  It expects a boot header, which is created by
  * the bootloader (the Launcher in our case).
  *
@@ -21,11 +22,14 @@
  * data without remembering to subtract __PAGE_OFFSET!
  *
  * The .section line puts this code in .init.text so it will be discarded after
- * boot. */
+ * boot.
+ */
 .section .init.text, "ax", @progbits
 ENTRY(lguest_entry)
-	/* We make the "initialization" hypercall now to tell the Host about
-	 * us, and also find out where it put our page tables. */
+	/*
+	 * We make the "initialization" hypercall now to tell the Host about
+	 * us, and also find out where it put our page tables.
+	 */
 	movl $LHCALL_LGUEST_INIT, %eax
 	movl $lguest_data - __PAGE_OFFSET, %ebx
 	.byte 0x0f,0x01,0xc1 /* KVM_HYPERCALL */
@@ -33,13 +37,14 @@ ENTRY(lguest_entry)
 	/* Set up the initial stack so we can run C code. */
 	movl $(init_thread_union+THREAD_SIZE),%esp
 
-	/* Jumps are relative, and we're running __PAGE_OFFSET too low at the
-	 * moment. */
+	/* Jumps are relative: we're running __PAGE_OFFSET too low. */
 	jmp lguest_init+__PAGE_OFFSET
 
-/*G:055 We create a macro which puts the assembler code between lgstart_ and
- * lgend_ markers.  These templates are put in the .text section: they can't be
- * discarded after boot as we may need to patch modules, too. */
+/*G:055
+ * We create a macro which puts the assembler code between lgstart_ and lgend_
+ * markers.  These templates are put in the .text section: they can't be
+ * discarded after boot as we may need to patch modules, too.
+ */
 .text
 #define LGUEST_PATCH(name, insns...)			\
 	lgstart_##name:	insns; lgend_##name:;		\
@@ -48,83 +53,103 @@ ENTRY(lguest_entry)
 LGUEST_PATCH(cli, movl $0, lguest_data+LGUEST_DATA_irq_enabled)
 LGUEST_PATCH(pushf, movl lguest_data+LGUEST_DATA_irq_enabled, %eax)
 
-/*G:033 But using those wrappers is inefficient (we'll see why that doesn't
- * matter for save_fl and irq_disable later).  If we write our routines
- * carefully in assembler, we can avoid clobbering any registers and avoid
- * jumping through the wrapper functions.
+/*G:033
+ * But using those wrappers is inefficient (we'll see why that doesn't matter
+ * for save_fl and irq_disable later).  If we write our routines carefully in
+ * assembler, we can avoid clobbering any registers and avoid jumping through
+ * the wrapper functions.
  *
  * I skipped over our first piece of assembler, but this one is worth studying
- * in a bit more detail so I'll describe in easy stages.  First, the routine
- * to enable interrupts: */
+ * in a bit more detail so I'll describe in easy stages.  First, the routine to
+ * enable interrupts:
+ */
 ENTRY(lg_irq_enable)
-	/* The reverse of irq_disable, this sets lguest_data.irq_enabled to
-	 * X86_EFLAGS_IF (ie. "Interrupts enabled"). */
+	/*
+	 * The reverse of irq_disable, this sets lguest_data.irq_enabled to
+	 * X86_EFLAGS_IF (ie. "Interrupts enabled").
+	 */
 	movl $X86_EFLAGS_IF, lguest_data+LGUEST_DATA_irq_enabled
-	/* But now we need to check if the Host wants to know: there might have
+	/*
+	 * But now we need to check if the Host wants to know: there might have
 	 * been interrupts waiting to be delivered, in which case it will have
 	 * set lguest_data.irq_pending to X86_EFLAGS_IF.  If it's not zero, we
-	 * jump to send_interrupts, otherwise we're done. */
+	 * jump to send_interrupts, otherwise we're done.
+	 */
 	testl $0, lguest_data+LGUEST_DATA_irq_pending
 	jnz send_interrupts
-	/* One cool thing about x86 is that you can do many things without using
+	/*
+	 * One cool thing about x86 is that you can do many things without using
 	 * a register.  In this case, the normal path hasn't needed to save or
-	 * restore any registers at all! */
+	 * restore any registers at all!
+	 */
 	ret
 send_interrupts:
-	/* OK, now we need a register: eax is used for the hypercall number,
+	/*
+	 * OK, now we need a register: eax is used for the hypercall number,
 	 * which is LHCALL_SEND_INTERRUPTS.
 	 *
 	 * We used not to bother with this pending detection at all, which was
 	 * much simpler.  Sooner or later the Host would realize it had to
 	 * send us an interrupt.  But that turns out to make performance 7
 	 * times worse on a simple tcp benchmark.  So now we do this the hard
-	 * way. */
+	 * way.
+	 */
 	pushl %eax
 	movl $LHCALL_SEND_INTERRUPTS, %eax
-	/* This is a vmcall instruction (same thing that KVM uses).  Older
+	/*
+	 * This is a vmcall instruction (same thing that KVM uses).  Older
 	 * assembler versions might not know the "vmcall" instruction, so we
-	 * create one manually here. */
+	 * create one manually here.
+	 */
 	.byte 0x0f,0x01,0xc1 /* KVM_HYPERCALL */
+	/* Put eax back the way we found it. */
 	popl %eax
 	ret
 
-/* Finally, the "popf" or "restore flags" routine.  The %eax register holds the
+/*
+ * Finally, the "popf" or "restore flags" routine.  The %eax register holds the
  * flags (in practice, either X86_EFLAGS_IF or 0): if it's X86_EFLAGS_IF we're
- * enabling interrupts again, if it's 0 we're leaving them off. */
+ * enabling interrupts again, if it's 0 we're leaving them off.
+ */
 ENTRY(lg_restore_fl)
 	/* This is just "lguest_data.irq_enabled = flags;" */
 	movl %eax, lguest_data+LGUEST_DATA_irq_enabled
-	/* Now, if the %eax value has enabled interrupts and
+	/*
+	 * Now, if the %eax value has enabled interrupts and
 	 * lguest_data.irq_pending is set, we want to tell the Host so it can
 	 * deliver any outstanding interrupts.  Fortunately, both values will
 	 * be X86_EFLAGS_IF (ie. 512) in that case, and the "testl"
 	 * instruction will AND them together for us.  If both are set, we
-	 * jump to send_interrupts. */
+	 * jump to send_interrupts.
+	 */
 	testl lguest_data+LGUEST_DATA_irq_pending, %eax
 	jnz send_interrupts
 	/* Again, the normal path has used no extra registers.  Clever, huh? */
 	ret
+/*:*/
 
 /* These demark the EIP range where host should never deliver interrupts. */
 .global lguest_noirq_start
 .global lguest_noirq_end
 
-/*M:004 When the Host reflects a trap or injects an interrupt into the Guest,
- * it sets the eflags interrupt bit on the stack based on
- * lguest_data.irq_enabled, so the Guest iret logic does the right thing when
- * restoring it.  However, when the Host sets the Guest up for direct traps,
- * such as system calls, the processor is the one to push eflags onto the
- * stack, and the interrupt bit will be 1 (in reality, interrupts are always
- * enabled in the Guest).
+/*M:004
+ * When the Host reflects a trap or injects an interrupt into the Guest, it
+ * sets the eflags interrupt bit on the stack based on lguest_data.irq_enabled,
+ * so the Guest iret logic does the right thing when restoring it.  However,
+ * when the Host sets the Guest up for direct traps, such as system calls, the
+ * processor is the one to push eflags onto the stack, and the interrupt bit
+ * will be 1 (in reality, interrupts are always enabled in the Guest).
  *
  * This turns out to be harmless: the only trap which should happen under Linux
  * with interrupts disabled is Page Fault (due to our lazy mapping of vmalloc
  * regions), which has to be reflected through the Host anyway.  If another
  * trap *does* go off when interrupts are disabled, the Guest will panic, and
- * we'll never get to this iret! :*/
+ * we'll never get to this iret!
+:*/
 
-/*G:045 There is one final paravirt_op that the Guest implements, and glancing
- * at it you can see why I left it to last.  It's *cool*!  It's in *assembler*!
+/*G:045
+ * There is one final paravirt_op that the Guest implements, and glancing at it
+ * you can see why I left it to last.  It's *cool*!  It's in *assembler*!
  *
  * The "iret" instruction is used to return from an interrupt or trap.  The
  * stack looks like this:
@@ -148,15 +173,18 @@ ENTRY(lg_restore_fl)
  * return to userspace or wherever.  Our solution to this is to surround the
  * code with lguest_noirq_start: and lguest_noirq_end: labels.  We tell the
  * Host that it is *never* to interrupt us there, even if interrupts seem to be
- * enabled. */
+ * enabled.
+ */
 ENTRY(lguest_iret)
 	pushl	%eax
 	movl	12(%esp), %eax
 lguest_noirq_start:
-	/* Note the %ss: segment prefix here.  Normal data accesses use the
+	/*
+	 * Note the %ss: segment prefix here.  Normal data accesses use the
 	 * "ds" segment, but that will have already been restored for whatever
 	 * we're returning to (such as userspace): we can't trust it.  The %ss:
-	 * prefix makes sure we use the stack segment, which is still valid. */
+	 * prefix makes sure we use the stack segment, which is still valid.
+	 */
 	movl	%eax,%ss:lguest_data+LGUEST_DATA_irq_enabled
 	popl	%eax
 	iret

drivers/lguest/core.c

@@ -1,6 +1,8 @@
-/*P:400 This contains run_guest() which actually calls into the Host<->Guest
+/*P:400
+ * This contains run_guest() which actually calls into the Host<->Guest
  * Switcher and analyzes the return, such as determining if the Guest wants the
- * Host to do something.  This file also contains useful helper routines. :*/
+ * Host to do something.  This file also contains useful helper routines.
+:*/
 #include <linux/module.h>
 #include <linux/stringify.h>
 #include <linux/stddef.h>
@@ -24,7 +26,8 @@ static struct page **switcher_page;
 /* This One Big lock protects all inter-guest data structures. */
 DEFINE_MUTEX(lguest_lock);
 
-/*H:010 We need to set up the Switcher at a high virtual address.  Remember the
+/*H:010
+ * We need to set up the Switcher at a high virtual address.  Remember the
  * Switcher is a few hundred bytes of assembler code which actually changes the
  * CPU to run the Guest, and then changes back to the Host when a trap or
  * interrupt happens.
@@ -33,7 +36,8 @@ DEFINE_MUTEX(lguest_lock);
  * Host since it will be running as the switchover occurs.
  *
  * Trying to map memory at a particular address is an unusual thing to do, so
- * it's not a simple one-liner. */
+ * it's not a simple one-liner.
+ */
 static __init int map_switcher(void)
 {
 	int i, err;
@@ -47,8 +51,10 @@ static __init int map_switcher(void)
 	 * easy.
 	 */
 
-	/* We allocate an array of struct page pointers.  map_vm_area() wants
-	 * this, rather than just an array of pages. */
+	/*
+	 * We allocate an array of struct page pointers.  map_vm_area() wants
+	 * this, rather than just an array of pages.
+	 */
 	switcher_page = kmalloc(sizeof(switcher_page[0])*TOTAL_SWITCHER_PAGES,
 				GFP_KERNEL);
 	if (!switcher_page) {
@@ -56,8 +62,10 @@ static __init int map_switcher(void)
 		goto out;
 	}
 
-	/* Now we actually allocate the pages.  The Guest will see these pages,
-	 * so we make sure they're zeroed. */
+	/*
+	 * Now we actually allocate the pages.  The Guest will see these pages,
+	 * so we make sure they're zeroed.
+	 */
 	for (i = 0; i < TOTAL_SWITCHER_PAGES; i++) {
 		unsigned long addr = get_zeroed_page(GFP_KERNEL);
 		if (!addr) {
@@ -67,19 +75,23 @@ static __init int map_switcher(void)
 		switcher_page[i] = virt_to_page(addr);
 	}
 
-	/* First we check that the Switcher won't overlap the fixmap area at
+	/*
+	 * First we check that the Switcher won't overlap the fixmap area at
 	 * the top of memory.  It's currently nowhere near, but it could have
-	 * very strange effects if it ever happened. */
+	 * very strange effects if it ever happened.
+	 */
 	if (SWITCHER_ADDR + (TOTAL_SWITCHER_PAGES+1)*PAGE_SIZE > FIXADDR_START){
 		err = -ENOMEM;
 		printk("lguest: mapping switcher would thwack fixmap\n");
 		goto free_pages;
 	}
 
-	/* Now we reserve the "virtual memory area" we want: 0xFFC00000
+	/*
+	 * Now we reserve the "virtual memory area" we want: 0xFFC00000
 	 * (SWITCHER_ADDR).  We might not get it in theory, but in practice
 	 * it's worked so far.  The end address needs +1 because __get_vm_area
-	 * allocates an extra guard page, so we need space for that. */
+	 * allocates an extra guard page, so we need space for that.
+	 */
 	switcher_vma = __get_vm_area(TOTAL_SWITCHER_PAGES * PAGE_SIZE,
 				     VM_ALLOC, SWITCHER_ADDR, SWITCHER_ADDR
 				     + (TOTAL_SWITCHER_PAGES+1) * PAGE_SIZE);
@@ -89,11 +101,13 @@ static __init int map_switcher(void)
 		goto free_pages;
 	}
 
-	/* This code actually sets up the pages we've allocated to appear at
+	/*
+	 * This code actually sets up the pages we've allocated to appear at
 	 * SWITCHER_ADDR.  map_vm_area() takes the vma we allocated above, the
 	 * kind of pages we're mapping (kernel pages), and a pointer to our
 	 * array of struct pages.  It increments that pointer, but we don't
-	 * care. */
+	 * care.
+	 */
 	pagep = switcher_page;
 	err = map_vm_area(switcher_vma, PAGE_KERNEL_EXEC, &pagep);
 	if (err) {
@@ -101,8 +115,10 @@ static __init int map_switcher(void)
 		goto free_vma;
 	}
 
-	/* Now the Switcher is mapped at the right address, we can't fail!
-	 * Copy in the compiled-in Switcher code (from <arch>_switcher.S). */
+	/*
+	 * Now the Switcher is mapped at the right address, we can't fail!
+	 * Copy in the compiled-in Switcher code (from <arch>_switcher.S).
+	 */
 	memcpy(switcher_vma->addr, start_switcher_text,
 	       end_switcher_text - start_switcher_text);
 
@@ -124,8 +140,7 @@ out:
 }
 /*:*/
 
-/* Cleaning up the mapping when the module is unloaded is almost...
- * too easy. */
+/* Cleaning up the mapping when the module is unloaded is almost... too easy. */
 static void unmap_switcher(void)
 {
 	unsigned int i;
@@ -151,16 +166,19 @@ static void unmap_switcher(void)
  * But we can't trust the Guest: it might be trying to access the Launcher
  * code.  We have to check that the range is below the pfn_limit the Launcher
  * gave us.  We have to make sure that addr + len doesn't give us a false
- * positive by overflowing, too. */
+ * positive by overflowing, too.
+ */
 bool lguest_address_ok(const struct lguest *lg,
 		       unsigned long addr, unsigned long len)
 {
 	return (addr+len) / PAGE_SIZE < lg->pfn_limit && (addr+len >= addr);
 }
 
-/* This routine copies memory from the Guest.  Here we can see how useful the
+/*
+ * This routine copies memory from the Guest.  Here we can see how useful the
  * kill_lguest() routine we met in the Launcher can be: we return a random
- * value (all zeroes) instead of needing to return an error. */
+ * value (all zeroes) instead of needing to return an error.
+ */
 void __lgread(struct lg_cpu *cpu, void *b, unsigned long addr, unsigned bytes)
 {
 	if (!lguest_address_ok(cpu->lg, addr, bytes)
@@ -181,9 +199,11 @@ void __lgwrite(struct lg_cpu *cpu, unsigned long addr, const void *b,
 }
 /*:*/
 
-/*H:030 Let's jump straight to the the main loop which runs the Guest.
+/*H:030
+ * Let's jump straight to the the main loop which runs the Guest.
  * Remember, this is called by the Launcher reading /dev/lguest, and we keep
- * going around and around until something interesting happens. */
+ * going around and around until something interesting happens.
+ */
 int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
 {
 	/* We stop running once the Guest is dead. */
@@ -195,10 +215,17 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
 		if (cpu->hcall)
 			do_hypercalls(cpu);
 
-		/* It's possible the Guest did a NOTIFY hypercall to the
-		 * Launcher, in which case we return from the read() now. */
+		/*
+		 * It's possible the Guest did a NOTIFY hypercall to the
+		 * Launcher.
+		 */
 		if (cpu->pending_notify) {
+			/*
+			 * Does it just needs to write to a registered
+			 * eventfd (ie. the appropriate virtqueue thread)?
+			 */
 			if (!send_notify_to_eventfd(cpu)) {
+				/* OK, we tell the main Laucher. */
 				if (put_user(cpu->pending_notify, user))
 					return -EFAULT;
 				return sizeof(cpu->pending_notify);
@@ -209,29 +236,39 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
 		if (signal_pending(current))
 			return -ERESTARTSYS;
 
-		/* Check if there are any interrupts which can be delivered now:
+		/*
+		 * Check if there are any interrupts which can be delivered now:
 		 * if so, this sets up the hander to be executed when we next
-		 * run the Guest. */
+		 * run the Guest.
+		 */
 		irq = interrupt_pending(cpu, &more);
 		if (irq < LGUEST_IRQS)
 			try_deliver_interrupt(cpu, irq, more);
 
-		/* All long-lived kernel loops need to check with this horrible
+		/*
+		 * All long-lived kernel loops need to check with this horrible
 		 * thing called the freezer.  If the Host is trying to suspend,
-		 * it stops us. */
+		 * it stops us.
+		 */
 		try_to_freeze();
 
-		/* Just make absolutely sure the Guest is still alive.  One of
-		 * those hypercalls could have been fatal, for example. */
+		/*
+		 * Just make absolutely sure the Guest is still alive.  One of
+		 * those hypercalls could have been fatal, for example.
+		 */
 		if (cpu->lg->dead)
 			break;
 
-		/* If the Guest asked to be stopped, we sleep.  The Guest's
-		 * clock timer will wake us. */
+		/*
+		 * If the Guest asked to be stopped, we sleep.  The Guest's
+		 * clock timer will wake us.
+		 */
 		if (cpu->halted) {
 			set_current_state(TASK_INTERRUPTIBLE);
-			/* Just before we sleep, make sure no interrupt snuck in
-			 * which we should be doing. */
+			/*
+			 * Just before we sleep, make sure no interrupt snuck in
+			 * which we should be doing.
+			 */
 			if (interrupt_pending(cpu, &more) < LGUEST_IRQS)
 				set_current_state(TASK_RUNNING);
 			else
@@ -239,8 +276,10 @@ int run_guest(struct lg_cpu *cpu, unsigned long __user *user)
 			continue;
 		}
 
-		/* OK, now we're ready to jump into the Guest.  First we put up
-		 * the "Do Not Disturb" sign: */
+		/*
+		 * OK, now we're ready to jump into the Guest.  First we put up
+		 * the "Do Not Disturb" sign:
+		 */
 		local_irq_disable();
 
 		/* Actually run the Guest until something happens. */
@@ -327,8 +366,10 @@ static void __exit fini(void)
 }
 /*:*/
 
-/* The Host side of lguest can be a module.  This is a nice way for people to
- * play with it.  */
+/*
+ * The Host side of lguest can be a module.  This is a nice way for people to
+ * play with it.
+ */
 module_init(init);
 module_exit(fini);
 MODULE_LICENSE("GPL");

drivers/lguest/hypercalls.c

@@ -1,8 +1,10 @@
-/*P:500 Just as userspace programs request kernel operations through a system
+/*P:500
+ * Just as userspace programs request kernel operations through a system
  * call, the Guest requests Host operations through a "hypercall".  You might
  * notice this nomenclature doesn't really follow any logic, but the name has
  * been around for long enough that we're stuck with it.  As you'd expect, this
- * code is basically a one big switch statement. :*/
+ * code is basically a one big switch statement.
+:*/
 
 /*  Copyright (C) 2006 Rusty Russell IBM Corporation
 
@@ -28,30 +30,41 @@
 #include <asm/pgtable.h>
 #include "lg.h"
 
-/*H:120 This is the core hypercall routine: where the Guest gets what it wants.
- * Or gets killed.  Or, in the case of LHCALL_SHUTDOWN, both. */
+/*H:120
+ * This is the core hypercall routine: where the Guest gets what it wants.
+ * Or gets killed.  Or, in the case of LHCALL_SHUTDOWN, both.
+ */
 static void do_hcall(struct lg_cpu *cpu, struct hcall_args *args)
 {
 	switch (args->arg0) {
 	case LHCALL_FLUSH_ASYNC:
-		/* This call does nothing, except by breaking out of the Guest
-		 * it makes us process all the asynchronous hypercalls. */
+		/*
+		 * This call does nothing, except by breaking out of the Guest
+		 * it makes us process all the asynchronous hypercalls.
+		 */
 		break;
 	case LHCALL_SEND_INTERRUPTS:
-		/* This call does nothing too, but by breaking out of the Guest
-		 * it makes us process any pending interrupts. */
+		/*
+		 * This call does nothing too, but by breaking out of the Guest
+		 * it makes us process any pending interrupts.
+		 */
 		break;
 	case LHCALL_LGUEST_INIT:
-		/* You can't get here unless you're already initialized.  Don't
-		 * do that. */
+		/*
+		 * You can't get here unless you're already initialized.  Don't
+		 * do that.
+		 */
 		kill_guest(cpu, "already have lguest_data");
 		break;
 	case LHCALL_SHUTDOWN: {
-		/* Shutdown is such a trivial hypercall that we do it in four
-		 * lines right here. */
 		char msg[128];
-		/* If the lgread fails, it will call kill_guest() itself; the
-		 * kill_guest() with the message will be ignored. */
+		/*
+		 * Shutdown is such a trivial hypercall that we do it in five
+		 * lines right here.
+		 *
+		 * If the lgread fails, it will call kill_guest() itself; the
+		 * kill_guest() with the message will be ignored.
+		 */
 		__lgread(cpu, msg, args->arg1, sizeof(msg));
 		msg[sizeof(msg)-1] = '\0';
 		kill_guest(cpu, "CRASH: %s", msg);
@@ -60,16 +73,17 @@ static void do_hcall(struct lg_cpu *cpu, struct hcall_args *args)
 		break;
 	}
 	case LHCALL_FLUSH_TLB:
-		/* FLUSH_TLB comes in two flavors, depending on the
-		 * argument: */
+		/* FLUSH_TLB comes in two flavors, depending on the argument: */
 		if (args->arg1)
 			guest_pagetable_clear_all(cpu);
 		else
 			guest_pagetable_flush_user(cpu);
 		break;
 
-	/* All these calls simply pass the arguments through to the right
-	 * routines. */
+	/*
+	 * All these calls simply pass the arguments through to the right
+	 * routines.
+	 */
 	case LHCALL_NEW_PGTABLE:
 		guest_new_pagetable(cpu, args->arg1);
 		break;
@@ -112,15 +126,16 @@ static void do_hcall(struct lg_cpu *cpu, struct hcall_args *args)
 			kill_guest(cpu, "Bad hypercall %li\n", args->arg0);
 	}
 }
-/*:*/
 
-/*H:124 Asynchronous hypercalls are easy: we just look in the array in the
+/*H:124
+ * Asynchronous hypercalls are easy: we just look in the array in the
  * Guest's "struct lguest_data" to see if any new ones are marked "ready".
  *
  * We are careful to do these in order: obviously we respect the order the
  * Guest put them in the ring, but we also promise the Guest that they will
  * happen before any normal hypercall (which is why we check this before
- * checking for a normal hcall). */
+ * checking for a normal hcall).
+ */
 static void do_async_hcalls(struct lg_cpu *cpu)
 {
 	unsigned int i;
@@ -133,22 +148,28 @@ static void do_async_hcalls(struct lg_cpu *cpu)
 	/* We process "struct lguest_data"s hcalls[] ring once. */
 	for (i = 0; i < ARRAY_SIZE(st); i++) {
 		struct hcall_args args;
-		/* We remember where we were up to from last time.  This makes
+		/*
+		 * We remember where we were up to from last time.  This makes
 		 * sure that the hypercalls are done in the order the Guest
-		 * places them in the ring. */
+		 * places them in the ring.
+		 */
 		unsigned int n = cpu->next_hcall;
 
 		/* 0xFF means there's no call here (yet). */
 		if (st[n] == 0xFF)
 			break;
 
-		/* OK, we have hypercall.  Increment the "next_hcall" cursor,
-		 * and wrap back to 0 if we reach the end. */
+		/*
+		 * OK, we have hypercall.  Increment the "next_hcall" cursor,
+		 * and wrap back to 0 if we reach the end.
+		 */
 		if (++cpu->next_hcall == LHCALL_RING_SIZE)
 			cpu->next_hcall = 0;
 
-		/* Copy the hypercall arguments into a local copy of
-		 * the hcall_args struct. */
+		/*
+		 * Copy the hypercall arguments into a local copy of the
+		 * hcall_args struct.
+		 */
 		if (copy_from_user(&args, &cpu->lg->lguest_data->hcalls[n],
 				   sizeof(struct hcall_args))) {
 			kill_guest(cpu, "Fetching async hypercalls");
@@ -164,19 +185,25 @@ static void do_async_hcalls(struct lg_cpu *cpu)
 			break;
 		}
 
-		/* Stop doing hypercalls if they want to notify the Launcher:
-		 * it needs to service this first. */
+		/*
+		 * Stop doing hypercalls if they want to notify the Launcher:
+		 * it needs to service this first.
+		 */
 		if (cpu->pending_notify)
 			break;
 	}
 }
 
-/* Last of all, we look at what happens first of all.  The very first time the
- * Guest makes a hypercall, we end up here to set things up: */
+/*
+ * Last of all, we look at what happens first of all.  The very first time the
+ * Guest makes a hypercall, we end up here to set things up:
+ */
 static void initialize(struct lg_cpu *cpu)
 {
-	/* You can't do anything until you're initialized.  The Guest knows the
-	 * rules, so we're unforgiving here. */
+	/*
+	 * You can't do anything until you're initialized.  The Guest knows the
+	 * rules, so we're unforgiving here.
+	 */
 	if (cpu->hcall->arg0 != LHCALL_LGUEST_INIT) {
 		kill_guest(cpu, "hypercall %li before INIT", cpu->hcall->arg0);
 		return;
@@ -185,32 +212,44 @@ static void initialize(struct lg_cpu *cpu)
 	if (lguest_arch_init_hypercalls(cpu))
 		kill_guest(cpu, "bad guest page %p", cpu->lg->lguest_data);
 
-	/* The Guest tells us where we're not to deliver interrupts by putting
-	 * the range of addresses into "struct lguest_data". */
+	/*
+	 * The Guest tells us where we're not to deliver interrupts by putting
+	 * the range of addresses into "struct lguest_data".
+	 */
 	if (get_user(cpu->lg->noirq_start, &cpu->lg->lguest_data->noirq_start)
 	    || get_user(cpu->lg->noirq_end, &cpu->lg->lguest_data->noirq_end))
 		kill_guest(cpu, "bad guest page %p", cpu->lg->lguest_data);
 
-	/* We write the current time into the Guest's data page once so it can
-	 * set its clock. */
+	/*
+	 * We write the current time into the Guest's data page once so it can
+	 * set its clock.
+	 */
 	write_timestamp(cpu);
 
 	/* page_tables.c will also do some setup. */
 	page_table_guest_data_init(cpu);
 
-	/* This is the one case where the above accesses might have been the
+	/*
+	 * This is the one case where the above accesses might have been the
 	 * first write to a Guest page.  This may have caused a copy-on-write
 	 * fault, but the old page might be (read-only) in the Guest
-	 * pagetable. */
+	 * pagetable.
+	 */
 	guest_pagetable_clear_all(cpu);
 }
 /*:*/
 
-/*M:013 If a Guest reads from a page (so creates a mapping) that it has never
+/*M:013
+ * If a Guest reads from a page (so creates a mapping) that it has never
  * written to, and then the Launcher writes to it (ie. the output of a virtual
  * device), the Guest will still see the old page.  In practice, this never
  * happens: why would the Guest read a page which it has never written to?  But
- * a similar scenario might one day bite us, so it's worth mentioning. :*/
+ * a similar scenario might one day bite us, so it's worth mentioning.
+ *
+ * Note that if we used a shared anonymous mapping in the Launcher instead of
+ * mapping /dev/zero private, we wouldn't worry about cop-on-write.  And we
+ * need that to switch the Launcher to processes (away from threads) anyway.
+:*/
 
 /*H:100
  * Hypercalls
@@ -229,17 +268,22 @@ void do_hypercalls(struct lg_cpu *cpu)
 		return;
 	}
 
-	/* The Guest has initialized.
+	/*
+	 * The Guest has initialized.
 	 *
-	 * Look in the hypercall ring for the async hypercalls: */
+	 * Look in the hypercall ring for the async hypercalls:
+	 */
 	do_async_hcalls(cpu);
 
-	/* If we stopped reading the hypercall ring because the Guest did a
+	/*
+	 * If we stopped reading the hypercall ring because the Guest did a
 	 * NOTIFY to the Launcher, we want to return now.  Otherwise we do
-	 * the hypercall. */
+	 * the hypercall.
+	 */
 	if (!cpu->pending_notify) {
 		do_hcall(cpu, cpu->hcall);
-		/* Tricky point: we reset the hcall pointer to mark the
+		/*
+		 * Tricky point: we reset the hcall pointer to mark the
 		 * hypercall as "done".  We use the hcall pointer rather than
 		 * the trap number to indicate a hypercall is pending.
 		 * Normally it doesn't matter: the Guest will run again and
@@ -248,13 +292,16 @@ void do_hypercalls(struct lg_cpu *cpu)
 		 * However, if we are signalled or the Guest sends I/O to the
 		 * Launcher, the run_guest() loop will exit without running the
 		 * Guest.  When it comes back it would try to re-run the
-		 * hypercall.  Finding that bug sucked. */
+		 * hypercall.  Finding that bug sucked.
+		 */
 		cpu->hcall = NULL;
 	}
 }
 
-/* This routine supplies the Guest with time: it's used for wallclock time at
- * initial boot and as a rough time source if the TSC isn't available. */
+/*
+ * This routine supplies the Guest with time: it's used for wallclock time at
+ * initial boot and as a rough time source if the TSC isn't available.
+ */
 void write_timestamp(struct lg_cpu *cpu)
 {
 	struct timespec now;

drivers/lguest/interrupts_and_traps.c

@@ -1,4 +1,5 @@
-/*P:800 Interrupts (traps) are complicated enough to earn their own file.
+/*P:800
+ * Interrupts (traps) are complicated enough to earn their own file.
  * There are three classes of interrupts:
  *
  * 1) Real hardware interrupts which occur while we're running the Guest,
@@ -10,7 +11,8 @@
  * just like real hardware would deliver them.  Traps from the Guest can be set
  * up to go directly back into the Guest, but sometimes the Host wants to see
  * them first, so we also have a way of "reflecting" them into the Guest as if
- * they had been delivered to it directly. :*/
+ * they had been delivered to it directly.
+:*/
 #include <linux/uaccess.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
@@ -26,8 +28,10 @@ static unsigned long idt_address(u32 lo, u32 hi)
 	return (lo & 0x0000FFFF) | (hi & 0xFFFF0000);
 }
 
-/* The "type" of the interrupt handler is a 4 bit field: we only support a
- * couple of types. */
+/*
+ * The "type" of the interrupt handler is a 4 bit field: we only support a
+ * couple of types.
+ */
 static int idt_type(u32 lo, u32 hi)
 {
 	return (hi >> 8) & 0xF;
@@ -39,8 +43,10 @@ static bool idt_present(u32 lo, u32 hi)
 	return (hi & 0x8000);
 }
 
-/* We need a helper to "push" a value onto the Guest's stack, since that's a
- * big part of what delivering an interrupt does. */
+/*
+ * We need a helper to "push" a value onto the Guest's stack, since that's a
+ * big part of what delivering an interrupt does.
+ */
 static void push_guest_stack(struct lg_cpu *cpu, unsigned long *gstack, u32 val)
 {
 	/* Stack grows upwards: move stack then write value. */
@@ -48,7 +54,8 @@ static void push_guest_stack(struct lg_cpu *cpu, unsigned long *gstack, u32 val)
 	lgwrite(cpu, *gstack, u32, val);
 }
 
-/*H:210 The set_guest_interrupt() routine actually delivers the interrupt or
+/*H:210
+ * The set_guest_interrupt() routine actually delivers the interrupt or
  * trap.  The mechanics of delivering traps and interrupts to the Guest are the
  * same, except some traps have an "error code" which gets pushed onto the
  * stack as well: the caller tells us if this is one.
@@ -59,7 +66,8 @@ static void push_guest_stack(struct lg_cpu *cpu, unsigned long *gstack, u32 val)
  *
  * We set up the stack just like the CPU does for a real interrupt, so it's
  * identical for the Guest (and the standard "iret" instruction will undo
- * it). */
+ * it).
+ */
 static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi,
 				bool has_err)
 {
@@ -67,20 +75,26 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi,
 	u32 eflags, ss, irq_enable;
 	unsigned long virtstack;
 
-	/* There are two cases for interrupts: one where the Guest is already
+	/*
+	 * There are two cases for interrupts: one where the Guest is already
 	 * in the kernel, and a more complex one where the Guest is in
-	 * userspace.  We check the privilege level to find out. */
+	 * userspace.  We check the privilege level to find out.
+	 */
 	if ((cpu->regs->ss&0x3) != GUEST_PL) {
-		/* The Guest told us their kernel stack with the SET_STACK
-		 * hypercall: both the virtual address and the segment */
+		/*
+		 * The Guest told us their kernel stack with the SET_STACK
+		 * hypercall: both the virtual address and the segment.
+		 */
 		virtstack = cpu->esp1;
 		ss = cpu->ss1;
 
 		origstack = gstack = guest_pa(cpu, virtstack);
-		/* We push the old stack segment and pointer onto the new
+		/*
+		 * We push the old stack segment and pointer onto the new
 		 * stack: when the Guest does an "iret" back from the interrupt
 		 * handler the CPU will notice they're dropping privilege
-		 * levels and expect these here. */
+		 * levels and expect these here.
+		 */
 		push_guest_stack(cpu, &gstack, cpu->regs->ss);
 		push_guest_stack(cpu, &gstack, cpu->regs->esp);
 	} else {
@@ -91,18 +105,22 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi,
 		origstack = gstack = guest_pa(cpu, virtstack);
 	}
 
-	/* Remember that we never let the Guest actually disable interrupts, so
+	/*
+	 * Remember that we never let the Guest actually disable interrupts, so
 	 * the "Interrupt Flag" bit is always set.  We copy that bit from the
 	 * Guest's "irq_enabled" field into the eflags word: we saw the Guest
-	 * copy it back in "lguest_iret". */
+	 * copy it back in "lguest_iret".
+	 */
 	eflags = cpu->regs->eflags;
 	if (get_user(irq_enable, &cpu->lg->lguest_data->irq_enabled) == 0
 	    && !(irq_enable & X86_EFLAGS_IF))
 		eflags &= ~X86_EFLAGS_IF;
 
-	/* An interrupt is expected to push three things on the stack: the old
+	/*
+	 * An interrupt is expected to push three things on the stack: the old
 	 * "eflags" word, the old code segment, and the old instruction
-	 * pointer. */
+	 * pointer.
+	 */
 	push_guest_stack(cpu, &gstack, eflags);
 	push_guest_stack(cpu, &gstack, cpu->regs->cs);
 	push_guest_stack(cpu, &gstack, cpu->regs->eip);
@@ -111,15 +129,19 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi,
 	if (has_err)
 		push_guest_stack(cpu, &gstack, cpu->regs->errcode);
 
-	/* Now we've pushed all the old state, we change the stack, the code
-	 * segment and the address to execute. */
+	/*
+	 * Now we've pushed all the old state, we change the stack, the code
+	 * segment and the address to execute.
+	 */
 	cpu->regs->ss = ss;
 	cpu->regs->esp = virtstack + (gstack - origstack);
 	cpu->regs->cs = (__KERNEL_CS|GUEST_PL);
 	cpu->regs->eip = idt_address(lo, hi);
 
-	/* There are two kinds of interrupt handlers: 0xE is an "interrupt
-	 * gate" which expects interrupts to be disabled on entry. */
+	/*
+	 * There are two kinds of interrupt handlers: 0xE is an "interrupt
+	 * gate" which expects interrupts to be disabled on entry.
+	 */
 	if (idt_type(lo, hi) == 0xE)
 		if (put_user(0, &cpu->lg->lguest_data->irq_enabled))
 			kill_guest(cpu, "Disabling interrupts");
@@ -130,7 +152,8 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi,
  *
  * interrupt_pending() returns the first pending interrupt which isn't blocked
  * by the Guest.  It is called before every entry to the Guest, and just before
- * we go to sleep when the Guest has halted itself. */
+ * we go to sleep when the Guest has halted itself.
+ */
 unsigned int interrupt_pending(struct lg_cpu *cpu, bool *more)
 {
 	unsigned int irq;
@@ -140,8 +163,10 @@ unsigned int interrupt_pending(struct lg_cpu *cpu, bool *more)
 	if (!cpu->lg->lguest_data)
 		return LGUEST_IRQS;
 
-	/* Take our "irqs_pending" array and remove any interrupts the Guest
-	 * wants blocked: the result ends up in "blk". */
+	/*
+	 * Take our "irqs_pending" array and remove any interrupts the Guest
+	 * wants blocked: the result ends up in "blk".
+	 */
 	if (copy_from_user(&blk, cpu->lg->lguest_data->blocked_interrupts,
 			   sizeof(blk)))
 		return LGUEST_IRQS;
@@ -154,16 +179,20 @@ unsigned int interrupt_pending(struct lg_cpu *cpu, bool *more)
 	return irq;
 }
 
-/* This actually diverts the Guest to running an interrupt handler, once an
- * interrupt has been identified by interrupt_pending(). */
+/*
+ * This actually diverts the Guest to running an interrupt handler, once an
+ * interrupt has been identified by interrupt_pending().
+ */
 void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more)
 {
 	struct desc_struct *idt;
 
 	BUG_ON(irq >= LGUEST_IRQS);
 
-	/* They may be in the middle of an iret, where they asked us never to
-	 * deliver interrupts. */
+	/*
+	 * They may be in the middle of an iret, where they asked us never to
+	 * deliver interrupts.
+	 */
 	if (cpu->regs->eip >= cpu->lg->noirq_start &&
 	   (cpu->regs->eip < cpu->lg->noirq_end))
 		return;
@@ -187,29 +216,37 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more)
 		}
 	}
 
-	/* Look at the IDT entry the Guest gave us for this interrupt.  The
+	/*
+	 * Look at the IDT entry the Guest gave us for this interrupt.  The
 	 * first 32 (FIRST_EXTERNAL_VECTOR) entries are for traps, so we skip
-	 * over them. */
+	 * over them.
+	 */
 	idt = &cpu->arch.idt[FIRST_EXTERNAL_VECTOR+irq];
 	/* If they don't have a handler (yet?), we just ignore it */
 	if (idt_present(idt->a, idt->b)) {
 		/* OK, mark it no longer pending and deliver it. */
 		clear_bit(irq, cpu->irqs_pending);
-		/* set_guest_interrupt() takes the interrupt descriptor and a
+		/*
+		 * set_guest_interrupt() takes the interrupt descriptor and a
 		 * flag to say whether this interrupt pushes an error code onto
-		 * the stack as well: virtual interrupts never do. */
+		 * the stack as well: virtual interrupts never do.
+		 */
 		set_guest_interrupt(cpu, idt->a, idt->b, false);
 	}
 
-	/* Every time we deliver an interrupt, we update the timestamp in the
+	/*
+	 * Every time we deliver an interrupt, we update the timestamp in the
 	 * Guest's lguest_data struct.  It would be better for the Guest if we
 	 * did this more often, but it can actually be quite slow: doing it
 	 * here is a compromise which means at least it gets updated every
-	 * timer interrupt. */
+	 * timer interrupt.
+	 */
 	write_timestamp(cpu);
 
-	/* If there are no other interrupts we want to deliver, clear
-	 * the pending flag. */
+	/*
+	 * If there are no other interrupts we want to deliver, clear
+	 * the pending flag.
+	 */
 	if (!more)
 		put_user(0, &cpu->lg->lguest_data->irq_pending);
 }
@@ -217,24 +254,29 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more)
 /* And this is the routine when we want to set an interrupt for the Guest. */
 void set_interrupt(struct lg_cpu *cpu, unsigned int irq)
 {
-	/* Next time the Guest runs, the core code will see if it can deliver
-	 * this interrupt. */
+	/*
+	 * Next time the Guest runs, the core code will see if it can deliver
+	 * this interrupt.
+	 */
 	set_bit(irq, cpu->irqs_pending);
 
-	/* Make sure it sees it; it might be asleep (eg. halted), or
-	 * running the Guest right now, in which case kick_process()
-	 * will knock it out. */
+	/*
+	 * Make sure it sees it; it might be asleep (eg. halted), or running
+	 * the Guest right now, in which case kick_process() will knock it out.
+	 */
 	if (!wake_up_process(cpu->tsk))
 		kick_process(cpu->tsk);
 }
 /*:*/
 
-/* Linux uses trap 128 for system calls.  Plan9 uses 64, and Ron Minnich sent
+/*
+ * Linux uses trap 128 for system calls.  Plan9 uses 64, and Ron Minnich sent
  * me a patch, so we support that too.  It'd be a big step for lguest if half
  * the Plan 9 user base were to start using it.
  *
  * Actually now I think of it, it's possible that Ron *is* half the Plan 9
- * userbase.  Oh well. */
+ * userbase.  Oh well.
+ */
 static bool could_be_syscall(unsigned int num)
 {
 	/* Normal Linux SYSCALL_VECTOR or reserved vector? */
@@ -274,9 +316,11 @@ void free_interrupts(void)
 		clear_bit(syscall_vector, used_vectors);
 }
 
-/*H:220 Now we've got the routines to deliver interrupts, delivering traps like
+/*H:220
+ * Now we've got the routines to deliver interrupts, delivering traps like
  * page fault is easy.  The only trick is that Intel decided that some traps
- * should have error codes: */
+ * should have error codes:
+ */
 static bool has_err(unsigned int trap)
 {
 	return (trap == 8 || (trap >= 10 && trap <= 14) || trap == 17);
@@ -285,13 +329,17 @@ static bool has_err(unsigned int trap)
 /* deliver_trap() returns true if it could deliver the trap. */
 bool deliver_trap(struct lg_cpu *cpu, unsigned int num)
 {
-	/* Trap numbers are always 8 bit, but we set an impossible trap number
-	 * for traps inside the Switcher, so check that here. */
+	/*
+	 * Trap numbers are always 8 bit, but we set an impossible trap number
+	 * for traps inside the Switcher, so check that here.
+	 */
 	if (num >= ARRAY_SIZE(cpu->arch.idt))
 		return false;
 
-	/* Early on the Guest hasn't set the IDT entries (or maybe it put a
-	 * bogus one in): if we fail here, the Guest will be killed. */
+	/*
+	 * Early on the Guest hasn't set the IDT entries (or maybe it put a
+	 * bogus one in): if we fail here, the Guest will be killed.
+	 */
 	if (!idt_present(cpu->arch.idt[num].a, cpu->arch.idt[num].b))
 		return false;
 	set_guest_interrupt(cpu, cpu->arch.idt[num].a,
@@ -299,7 +347,8 @@ bool deliver_trap(struct lg_cpu *cpu, unsigned int num)
 	return true;
 }
 
-/*H:250 Here's the hard part: returning to the Host every time a trap happens
+/*H:250
+ * Here's the hard part: returning to the Host every time a trap happens
  * and then calling deliver_trap() and re-entering the Guest is slow.
  * Particularly because Guest userspace system calls are traps (usually trap
  * 128).
@@ -311,69 +360,87 @@ bool deliver_trap(struct lg_cpu *cpu, unsigned int num)
  * the other hypervisors would beat it up at lunchtime.
  *
  * This routine indicates if a particular trap number could be delivered
- * directly. */
+ * directly.
+ */
 static bool direct_trap(unsigned int num)
 {
-	/* Hardware interrupts don't go to the Guest at all (except system
-	 * call). */
+	/*
+	 * Hardware interrupts don't go to the Guest at all (except system
+	 * call).
+	 */
 	if (num >= FIRST_EXTERNAL_VECTOR && !could_be_syscall(num))
 		return false;
 
-	/* The Host needs to see page faults (for shadow paging and to save the
+	/*
+	 * The Host needs to see page faults (for shadow paging and to save the
 	 * fault address), general protection faults (in/out emulation) and
 	 * device not available (TS handling), invalid opcode fault (kvm hcall),
-	 * and of course, the hypercall trap. */
+	 * and of course, the hypercall trap.
+	 */
 	return num != 14 && num != 13 && num != 7 &&
 			num != 6 && num != LGUEST_TRAP_ENTRY;
 }
 /*:*/
 
-/*M:005 The Guest has the ability to turn its interrupt gates into trap gates,
+/*M:005
+ * The Guest has the ability to turn its interrupt gates into trap gates,
  * if it is careful.  The Host will let trap gates can go directly to the
  * Guest, but the Guest needs the interrupts atomically disabled for an
  * interrupt gate.  It can do this by pointing the trap gate at instructions
- * within noirq_start and noirq_end, where it can safely disable interrupts. */
+ * within noirq_start and noirq_end, where it can safely disable interrupts.
+ */
 
-/*M:006 The Guests do not use the sysenter (fast system call) instruction,
+/*M:006
+ * The Guests do not use the sysenter (fast system call) instruction,
  * because it's hardcoded to enter privilege level 0 and so can't go direct.
  * It's about twice as fast as the older "int 0x80" system call, so it might
  * still be worthwhile to handle it in the Switcher and lcall down to the
  * Guest.  The sysenter semantics are hairy tho: search for that keyword in
- * entry.S :*/
+ * entry.S
+:*/
 
-/*H:260 When we make traps go directly into the Guest, we need to make sure
+/*H:260
+ * When we make traps go directly into the Guest, we need to make sure
  * the kernel stack is valid (ie. mapped in the page tables).  Otherwise, the
  * CPU trying to deliver the trap will fault while trying to push the interrupt
  * words on the stack: this is called a double fault, and it forces us to kill
  * the Guest.
  *
- * Which is deeply unfair, because (literally!) it wasn't the Guests' fault. */
+ * Which is deeply unfair, because (literally!) it wasn't the Guests' fault.
+ */
 void pin_stack_pages(struct lg_cpu *cpu)
 {
 	unsigned int i;
 
-	/* Depending on the CONFIG_4KSTACKS option, the Guest can have one or
-	 * two pages of stack space. */
+	/*
+	 * Depending on the CONFIG_4KSTACKS option, the Guest can have one or
+	 * two pages of stack space.
+	 */
 	for (i = 0; i < cpu->lg->stack_pages; i++)
-		/* The stack grows *upwards*, so the address we're given is the
+		/*
+		 * The stack grows *upwards*, so the address we're given is the
 		 * start of the page after the kernel stack.  Subtract one to
 		 * get back onto the first stack page, and keep subtracting to
-		 * get to the rest of the stack pages. */
+		 * get to the rest of the stack pages.
+		 */
 		pin_page(cpu, cpu->esp1 - 1 - i * PAGE_SIZE);
 }
 
-/* Direct traps also mean that we need to know whenever the Guest wants to use
+/*
+ * Direct traps also mean that we need to know whenever the Guest wants to use
  * a different kernel stack, so we can change the IDT entries to use that
  * stack.  The IDT entries expect a virtual address, so unlike most addresses
  * the Guest gives us, the "esp" (stack pointer) value here is virtual, not
  * physical.
  *
  * In Linux each process has its own kernel stack, so this happens a lot: we
- * change stacks on each context switch. */
+ * change stacks on each context switch.
+ */
 void guest_set_stack(struct lg_cpu *cpu, u32 seg, u32 esp, unsigned int pages)
 {
-	/* You are not allowed have a stack segment with privilege level 0: bad
-	 * Guest! */
+	/*
+	 * You're not allowed a stack segment with privilege level 0: bad Guest!
+	 */
 	if ((seg & 0x3) != GUEST_PL)
 		kill_guest(cpu, "bad stack segment %i", seg);
 	/* We only expect one or two stack pages. */
@@ -387,11 +454,15 @@ void guest_set_stack(struct lg_cpu *cpu, u32 seg, u32 esp, unsigned int pages)
 	pin_stack_pages(cpu);
 }
 
-/* All this reference to mapping stacks leads us neatly into the other complex
- * part of the Host: page table handling. */
+/*
+ * All this reference to mapping stacks leads us neatly into the other complex
+ * part of the Host: page table handling.
+ */
 
-/*H:235 This is the routine which actually checks the Guest's IDT entry and
- * transfers it into the entry in "struct lguest": */
+/*H:235
+ * This is the routine which actually checks the Guest's IDT entry and
+ * transfers it into the entry in "struct lguest":
+ */
 static void set_trap(struct lg_cpu *cpu, struct desc_struct *trap,
 		     unsigned int num, u32 lo, u32 hi)
 {
@@ -407,30 +478,38 @@ static void set_trap(struct lg_cpu *cpu, struct desc_struct *trap,
 	if (type != 0xE && type != 0xF)
 		kill_guest(cpu, "bad IDT type %i", type);
 
-	/* We only copy the handler address, present bit, privilege level and
+	/*
+	 * We only copy the handler address, present bit, privilege level and
 	 * type.  The privilege level controls where the trap can be triggered
 	 * manually with an "int" instruction.  This is usually GUEST_PL,
-	 * except for system calls which userspace can use. */
+	 * except for system calls which userspace can use.
+	 */
 	trap->a = ((__KERNEL_CS|GUEST_PL)<<16) | (lo&0x0000FFFF);
 	trap->b = (hi&0xFFFFEF00);
 }
 
-/*H:230 While we're here, dealing with delivering traps and interrupts to the
+/*H:230
+ * While we're here, dealing with delivering traps and interrupts to the
  * Guest, we might as well complete the picture: how the Guest tells us where
  * it wants them to go.  This would be simple, except making traps fast
  * requires some tricks.
  *
  * We saw the Guest setting Interrupt Descriptor Table (IDT) entries with the
- * LHCALL_LOAD_IDT_ENTRY hypercall before: that comes here. */
+ * LHCALL_LOAD_IDT_ENTRY hypercall before: that comes here.
+ */
 void load_guest_idt_entry(struct lg_cpu *cpu, unsigned int num, u32 lo, u32 hi)
 {
-	/* Guest never handles: NMI, doublefault, spurious interrupt or
-	 * hypercall.  We ignore when it tries to set them. */
+	/*
+	 * Guest never handles: NMI, doublefault, spurious interrupt or
+	 * hypercall.  We ignore when it tries to set them.
+	 */
 	if (num == 2 || num == 8 || num == 15 || num == LGUEST_TRAP_ENTRY)
 		return;
 
-	/* Mark the IDT as changed: next time the Guest runs we'll know we have
-	 * to copy this again. */
+	/*
+	 * Mark the IDT as changed: next time the Guest runs we'll know we have
+	 * to copy this again.
+	 */
 	cpu->changed |= CHANGED_IDT;
 
 	/* Check that the Guest doesn't try to step outside the bounds. */
@@ -440,9 +519,11 @@ void load_guest_idt_entry(struct lg_cpu *cpu, unsigned int num, u32 lo, u32 hi)
 		set_trap(cpu, &cpu->arch.idt[num], num, lo, hi);
 }
 
-/* The default entry for each interrupt points into the Switcher routines which
+/*
+ * The default entry for each interrupt points into the Switcher routines which
  * simply return to the Host.  The run_guest() loop will then call
- * deliver_trap() to bounce it back into the Guest. */
+ * deliver_trap() to bounce it back into the Guest.
+ */
 static void default_idt_entry(struct desc_struct *idt,
 			      int trap,
 			      const unsigned long handler,
@@ -451,13 +532,17 @@ static void default_idt_entry(struct desc_struct *idt,
 	/* A present interrupt gate. */
 	u32 flags = 0x8e00;
 
-	/* Set the privilege level on the entry for the hypercall: this allows
-	 * the Guest to use the "int" instruction to trigger it. */
+	/*
+	 * Set the privilege level on the entry for the hypercall: this allows
+	 * 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. */
+		/*
+		 * Copy privilege 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. */
@@ -475,16 +560,20 @@ void setup_default_idt_entries(struct lguest_ro_state *state,
 		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
+/*H:240
+ * We don't use the IDT entries in the "struct lguest" directly, instead
  * we copy them into the IDT which we've set up for Guests on this CPU, just
- * before we run the Guest.  This routine does that copy. */
+ * before we run the Guest.  This routine does that copy.
+ */
 void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		const unsigned long *def)
 {
 	unsigned int i;
 
-	/* We can simply copy the direct traps, otherwise we use the default
-	 * ones in the Switcher: they will return to the Host. */
+	/*
+	 * 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];
 
@@ -492,14 +581,16 @@ void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		if (!direct_trap(i))
 			continue;
 
-		/* Only trap gates (type 15) can go direct to the Guest.
+		/*
+		 * 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 it can't go direct, we still need to copy the priv. level:
 		 * they might want to give userspace access to a software
-		 * interrupt. */
+		 * interrupt.
+		 */
 		if (idt_type(gidt->a, gidt->b) == 0xF)
 			idt[i] = *gidt;
 		else
@@ -518,7 +609,8 @@ void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
  * the next timer interrupt (in nanoseconds).  We use the high-resolution timer
  * infrastructure to set a callback at that time.
  *
- * 0 means "turn off the clock". */
+ * 0 means "turn off the clock".
+ */
 void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta)
 {
 	ktime_t expires;
@@ -529,9 +621,11 @@ void guest_set_clockevent(struct lg_cpu *cpu, unsigned long delta)
 		return;
 	}
 
-	/* We use wallclock time here, so the Guest might not be running for
+	/*
+	 * We use wallclock time here, so the Guest might not be running for
 	 * all the time between now and the timer interrupt it asked for.  This
-	 * is almost always the right thing to do. */
+	 * is almost always the right thing to do.
+	 */
 	expires = ktime_add_ns(ktime_get_real(), delta);
 	hrtimer_start(&cpu->hrt, expires, HRTIMER_MODE_ABS);
 }

drivers/lguest/lg.h

@@ -16,15 +16,13 @@
 void free_pagetables(void);
 int init_pagetables(struct page **switcher_page, unsigned int pages);
 
-struct pgdir
-{
+struct pgdir {
 	unsigned long gpgdir;
 	pgd_t *pgdir;
 };
 
 /* We have two pages shared with guests, per cpu.  */
-struct lguest_pages
-{
+struct lguest_pages {
 	/* This is the stack page mapped rw in guest */
 	char spare[PAGE_SIZE - sizeof(struct lguest_regs)];
 	struct lguest_regs regs;
@@ -54,13 +52,13 @@ struct lg_cpu {
 
 	unsigned long pending_notify; /* pfn from LHCALL_NOTIFY */
 
-	/* At end of a page shared mapped over lguest_pages in guest.  */
+	/* At end of a page shared mapped over lguest_pages in guest. */
 	unsigned long regs_page;
 	struct lguest_regs *regs;
 
 	struct lguest_pages *last_pages;
 
-	int cpu_pgd; /* which pgd this cpu is currently using */
+	int cpu_pgd; /* Which pgd this cpu is currently using */
 
 	/* If a hypercall was asked for, this points to the arguments. */
 	struct hcall_args *hcall;
@@ -89,15 +87,17 @@ struct lg_eventfd_map {
 };
 
 /* The private info the thread maintains about the guest. */
-struct lguest
-{
+struct lguest {
 	struct lguest_data __user *lguest_data;
 	struct lg_cpu cpus[NR_CPUS];
 	unsigned int nr_cpus;
 
 	u32 pfn_limit;
-	/* This provides the offset to the base of guest-physical
-	 * memory in the Launcher. */
+
+	/*
+	 * This provides the offset to the base of guest-physical memory in the
+	 * Launcher.
+	 */
 	void __user *mem_base;
 	unsigned long kernel_address;
 
@@ -122,11 +122,13 @@ bool lguest_address_ok(const struct lguest *lg,
 void __lgread(struct lg_cpu *, void *, unsigned long, unsigned);
 void __lgwrite(struct lg_cpu *, unsigned long, const void *, unsigned);
 
-/*H:035 Using memory-copy operations like that is usually inconvient, so we
+/*H:035
+ * Using memory-copy operations like that is usually inconvient, so we
  * have the following helper macros which read and write a specific type (often
  * an unsigned long).
  *
- * This reads into a variable of the given type then returns that. */
+ * This reads into a variable of the given type then returns that.
+ */
 #define lgread(cpu, addr, type)						\
 	({ type _v; __lgread((cpu), &_v, (addr), sizeof(_v)); _v; })
 
@@ -140,9 +142,11 @@ void __lgwrite(struct lg_cpu *, unsigned long, const void *, unsigned);
 
 int run_guest(struct lg_cpu *cpu, unsigned long __user *user);
 
-/* Helper macros to obtain the first 12 or the last 20 bits, this is only the
+/*
+ * Helper macros to obtain the first 12 or the last 20 bits, this is only the
  * first step in the migration to the kernel types.  pte_pfn is already defined
- * in the kernel. */
+ * in the kernel.
+ */
 #define pgd_flags(x)	(pgd_val(x) & ~PAGE_MASK)
 #define pgd_pfn(x)	(pgd_val(x) >> PAGE_SHIFT)
 #define pmd_flags(x)    (pmd_val(x) & ~PAGE_MASK)

drivers/lguest/lguest_device.c

@@ -1,10 +1,12 @@
-/*P:050 Lguest guests use a very simple method to describe devices.  It's a
+/*P:050
+ * Lguest guests use a very simple method to describe devices.  It's a
  * series of device descriptors contained just above the top of normal Guest
  * memory.
  *
  * We use the standard "virtio" device infrastructure, which provides us with a
  * console, a network and a block driver.  Each one expects some configuration
- * information and a "virtqueue" or two to send and receive data. :*/
+ * information and a "virtqueue" or two to send and receive data.
+:*/
 #include <linux/init.h>
 #include <linux/bootmem.h>
 #include <linux/lguest_launcher.h>
@@ -20,8 +22,10 @@
 /* The pointer to our (page) of device descriptions. */
 static void *lguest_devices;
 
-/* For Guests, device memory can be used as normal memory, so we cast away the
- * __iomem to quieten sparse. */
+/*
+ * For Guests, device memory can be used as normal memory, so we cast away the
+ * __iomem to quieten sparse.
+ */
 static inline void *lguest_map(unsigned long phys_addr, unsigned long pages)
 {
 	return (__force void *)ioremap_cache(phys_addr, PAGE_SIZE*pages);
@@ -32,8 +36,10 @@ static inline void lguest_unmap(void *addr)
 	iounmap((__force void __iomem *)addr);
 }
 
-/*D:100 Each lguest device is just a virtio device plus a pointer to its entry
- * in the lguest_devices page. */
+/*D:100
+ * Each lguest device is just a virtio device plus a pointer to its entry
+ * in the lguest_devices page.
+ */
 struct lguest_device {
 	struct virtio_device vdev;
 
@@ -41,9 +47,11 @@ struct lguest_device {
 	struct lguest_device_desc *desc;
 };
 
-/* Since the virtio infrastructure hands us a pointer to the virtio_device all
+/*
+ * Since the virtio infrastructure hands us a pointer to the virtio_device all
  * the time, it helps to have a curt macro to get a pointer to the struct
- * lguest_device it's enclosed in.  */
+ * lguest_device it's enclosed in.
+ */
 #define to_lgdev(vd) container_of(vd, struct lguest_device, vdev)
 
 /*D:130
@@ -55,7 +63,8 @@ struct lguest_device {
  * the driver will look at them during setup.
  *
  * A convenient routine to return the device's virtqueue config array:
- * immediately after the descriptor. */
+ * immediately after the descriptor.
+ */
 static struct lguest_vqconfig *lg_vq(const struct lguest_device_desc *desc)
 {
 	return (void *)(desc + 1);
@@ -98,10 +107,12 @@ static u32 lg_get_features(struct virtio_device *vdev)
 	return features;
 }
 
-/* The virtio core takes the features the Host offers, and copies the
- * ones supported by the driver into the vdev->features array.  Once
- * that's all sorted out, this routine is called so we can tell the
- * Host which features we understand and accept. */
+/*
+ * The virtio core takes the features the Host offers, and copies the ones
+ * supported by the driver into the vdev->features array.  Once that's all
+ * sorted out, this routine is called so we can tell the Host which features we
+ * understand and accept.
+ */
 static void lg_finalize_features(struct virtio_device *vdev)
 {
 	unsigned int i, bits;
@@ -112,10 +123,11 @@ static void lg_finalize_features(struct virtio_device *vdev)
 	/* Give virtio_ring a chance to accept features. */
 	vring_transport_features(vdev);
 
-	/* The vdev->feature array is a Linux bitmask: this isn't the
-	 * same as a the simple array of bits used by lguest devices
-	 * for features.  So we do this slow, manual conversion which is
-	 * completely general. */
+	/*
+	 * The vdev->feature array is a Linux bitmask: this isn't the same as a
+	 * the simple array of bits used by lguest devices for features.  So we
+	 * do this slow, manual conversion which is completely general.
+	 */
 	memset(out_features, 0, desc->feature_len);
 	bits = min_t(unsigned, desc->feature_len, sizeof(vdev->features)) * 8;
 	for (i = 0; i < bits; i++) {
@@ -146,15 +158,19 @@ static void lg_set(struct virtio_device *vdev, unsigned int offset,
 	memcpy(lg_config(desc) + offset, buf, len);
 }
 
-/* The operations to get and set the status word just access the status field
- * of the device descriptor. */
+/*
+ * The operations to get and set the status word just access the status field
+ * of the device descriptor.
+ */
 static u8 lg_get_status(struct virtio_device *vdev)
 {
 	return to_lgdev(vdev)->desc->status;
 }
 
-/* To notify on status updates, we (ab)use the NOTIFY hypercall, with the
- * descriptor address of the device.  A zero status means "reset". */
+/*
+ * To notify on status updates, we (ab)use the NOTIFY hypercall, with the
+ * descriptor address of the device.  A zero status means "reset".
+ */
 static void set_status(struct virtio_device *vdev, u8 status)
 {
 	unsigned long offset = (void *)to_lgdev(vdev)->desc - lguest_devices;
@@ -191,8 +207,7 @@ static void lg_reset(struct virtio_device *vdev)
  */
 
 /*D:140 This is the information we remember about each virtqueue. */
-struct lguest_vq_info
-{
+struct lguest_vq_info {
 	/* A copy of the information contained in the device config. */
 	struct lguest_vqconfig config;
 
@@ -200,13 +215,17 @@ struct lguest_vq_info
 	void *pages;
 };
 
-/* When the virtio_ring code wants to prod the Host, it calls us here and we
+/*
+ * When the virtio_ring code wants to prod the Host, it calls us here and we
  * make a hypercall.  We hand the physical address of the virtqueue so the Host
- * knows which virtqueue we're talking about. */
+ * knows which virtqueue we're talking about.
+ */
 static void lg_notify(struct virtqueue *vq)
 {
-	/* We store our virtqueue information in the "priv" pointer of the
-	 * virtqueue structure. */
+	/*
+	 * We store our virtqueue information in the "priv" pointer of the
+	 * virtqueue structure.
+	 */
 	struct lguest_vq_info *lvq = vq->priv;
 
 	kvm_hypercall1(LHCALL_NOTIFY, lvq->config.pfn << PAGE_SHIFT);
@@ -215,7 +234,8 @@ static void lg_notify(struct virtqueue *vq)
 /* An extern declaration inside a C file is bad form.  Don't do it. */
 extern void lguest_setup_irq(unsigned int irq);
 
-/* This routine finds the first virtqueue described in the configuration of
+/*
+ * This routine finds the Nth virtqueue described in the configuration of
  * this device and sets it up.
  *
  * This is kind of an ugly duckling.  It'd be nicer to have a standard
@@ -223,9 +243,7 @@ extern void lguest_setup_irq(unsigned int irq);
  * everyone wants to do it differently.  The KVM coders want the Guest to
  * allocate its own pages and tell the Host where they are, but for lguest it's
  * simpler for the Host to simply tell us where the pages are.
- *
- * So we provide drivers with a "find the Nth virtqueue and set it up"
- * function. */
+ */
 static struct virtqueue *lg_find_vq(struct virtio_device *vdev,
 				    unsigned index,
 				    void (*callback)(struct virtqueue *vq),
@@ -244,9 +262,11 @@ static struct virtqueue *lg_find_vq(struct virtio_device *vdev,
 	if (!lvq)
 		return ERR_PTR(-ENOMEM);
 
-	/* Make a copy of the "struct lguest_vqconfig" entry, which sits after
+	/*
+	 * Make a copy of the "struct lguest_vqconfig" entry, which sits after
 	 * the descriptor.  We need a copy because the config space might not
-	 * be aligned correctly. */
+	 * be aligned correctly.
+	 */
 	memcpy(&lvq->config, lg_vq(ldev->desc)+index, sizeof(lvq->config));
 
 	printk("Mapping virtqueue %i addr %lx\n", index,
@@ -261,8 +281,10 @@ static struct virtqueue *lg_find_vq(struct virtio_device *vdev,
 		goto free_lvq;
 	}
 
-	/* OK, tell virtio_ring.c to set up a virtqueue now we know its size
-	 * and we've got a pointer to its pages. */
+	/*
+	 * OK, tell virtio_ring.c to set up a virtqueue now we know its size
+	 * and we've got a pointer to its pages.
+	 */
 	vq = vring_new_virtqueue(lvq->config.num, LGUEST_VRING_ALIGN,
 				 vdev, lvq->pages, lg_notify, callback, name);
 	if (!vq) {
@@ -273,18 +295,23 @@ static struct virtqueue *lg_find_vq(struct virtio_device *vdev,
 	/* Make sure the interrupt is allocated. */
 	lguest_setup_irq(lvq->config.irq);
 
-	/* Tell the interrupt for this virtqueue to go to the virtio_ring
-	 * interrupt handler. */
-	/* FIXME: We used to have a flag for the Host to tell us we could use
+	/*
+	 * Tell the interrupt for this virtqueue to go to the virtio_ring
+	 * interrupt handler.
+	 *
+	 * FIXME: We used to have a flag for the Host to tell us we could use
 	 * the interrupt as a source of randomness: it'd be nice to have that
-	 * back.. */
+	 * back.
+	 */
 	err = request_irq(lvq->config.irq, vring_interrupt, IRQF_SHARED,
 			  dev_name(&vdev->dev), vq);
 	if (err)
 		goto destroy_vring;
 
-	/* Last of all we hook up our 'struct lguest_vq_info" to the
-	 * virtqueue's priv pointer. */
+	/*
+	 * Last of all we hook up our 'struct lguest_vq_info" to the
+	 * virtqueue's priv pointer.
+	 */
 	vq->priv = lvq;
 	return vq;
 
@@ -358,11 +385,14 @@ static struct virtio_config_ops lguest_config_ops = {
 	.del_vqs = lg_del_vqs,
 };
 
-/* The root device for the lguest virtio devices.  This makes them appear as
- * /sys/devices/lguest/0,1,2 not /sys/devices/0,1,2. */
+/*
+ * The root device for the lguest virtio devices.  This makes them appear as
+ * /sys/devices/lguest/0,1,2 not /sys/devices/0,1,2.
+ */
 static struct device *lguest_root;
 
-/*D:120 This is the core of the lguest bus: actually adding a new device.
+/*D:120
+ * This is the core of the lguest bus: actually adding a new device.
  * It's a separate function because it's neater that way, and because an
  * earlier version of the code supported hotplug and unplug.  They were removed
  * early on because they were never used.
@@ -371,14 +401,14 @@ static struct device *lguest_root;
  *
  * It's worth reading this carefully: we start with a pointer to the new device
  * descriptor in the "lguest_devices" page, and the offset into the device
- * descriptor page so we can uniquely identify it if things go badly wrong. */
+ * descriptor page so we can uniquely identify it if things go badly wrong.
+ */
 static void add_lguest_device(struct lguest_device_desc *d,
 			      unsigned int offset)
 {
 	struct lguest_device *ldev;
 
-	/* Start with zeroed memory; Linux's device layer seems to count on
-	 * it. */
+	/* Start with zeroed memory; Linux's device layer counts on it. */
 	ldev = kzalloc(sizeof(*ldev), GFP_KERNEL);
 	if (!ldev) {
 		printk(KERN_EMERG "Cannot allocate lguest dev %u type %u\n",
@@ -388,17 +418,25 @@ static void add_lguest_device(struct lguest_device_desc *d,
 
 	/* This devices' parent is the lguest/ dir. */
 	ldev->vdev.dev.parent = lguest_root;
-	/* We have a unique device index thanks to the dev_index counter. */
+	/*
+	 * The device type comes straight from the descriptor.  There's also a
+	 * device vendor field in the virtio_device struct, which we leave as
+	 * 0.
+	 */
 	ldev->vdev.id.device = d->type;
-	/* We have a simple set of routines for querying the device's
-	 * configuration information and setting its status. */
+	/*
+	 * We have a simple set of routines for querying the device's
+	 * configuration information and setting its status.
+	 */
 	ldev->vdev.config = &lguest_config_ops;
 	/* And we remember the device's descriptor for lguest_config_ops. */
 	ldev->desc = d;
 
-	/* register_virtio_device() sets up the generic fields for the struct
+	/*
+	 * register_virtio_device() sets up the generic fields for the struct
 	 * virtio_device and calls device_register().  This makes the bus
-	 * infrastructure look for a matching driver. */
+	 * infrastructure look for a matching driver.
+	 */
 	if (register_virtio_device(&ldev->vdev) != 0) {
 		printk(KERN_ERR "Failed to register lguest dev %u type %u\n",
 		       offset, d->type);
@@ -406,8 +444,10 @@ static void add_lguest_device(struct lguest_device_desc *d,
 	}
 }
 
-/*D:110 scan_devices() simply iterates through the device page.  The type 0 is
- * reserved to mean "end of devices". */
+/*D:110
+ * scan_devices() simply iterates through the device page.  The type 0 is
+ * reserved to mean "end of devices".
+ */
 static void scan_devices(void)
 {
 	unsigned int i;
@@ -426,7 +466,8 @@ static void scan_devices(void)
 	}
 }
 
-/*D:105 Fairly early in boot, lguest_devices_init() is called to set up the
+/*D:105
+ * Fairly early in boot, lguest_devices_init() is called to set up the
  * lguest device infrastructure.  We check that we are a Guest by checking
  * pv_info.name: there are other ways of checking, but this seems most
  * obvious to me.
@@ -437,7 +478,8 @@ static void scan_devices(void)
  * correct sysfs incantation).
  *
  * Finally we call scan_devices() which adds all the devices found in the
- * lguest_devices page. */
+ * lguest_devices page.
+ */
 static int __init lguest_devices_init(void)
 {
 	if (strcmp(pv_info.name, "lguest") != 0)
@@ -456,11 +498,13 @@ static int __init lguest_devices_init(void)
 /* We do this after core stuff, but before the drivers. */
 postcore_initcall(lguest_devices_init);
 
-/*D:150 At this point in the journey we used to now wade through the lguest
+/*D:150
+ * At this point in the journey we used to now wade through the lguest
  * devices themselves: net, block and console.  Since they're all now virtio
  * devices rather than lguest-specific, I've decided to ignore them.  Mostly,
  * they're kind of boring.  But this does mean you'll never experience the
  * thrill of reading the forbidden love scene buried deep in the block driver.
  *
  * "make Launcher" beckons, where we answer questions like "Where do Guests
- * come from?", and "What do you do when someone asks for optimization?". */
+ * come from?", and "What do you do when someone asks for optimization?".
+ */

drivers/lguest/lguest_user.c

@@ -1,8 +1,9 @@
 /*P:200 This contains all the /dev/lguest code, whereby the userspace launcher
  * controls and communicates with the Guest.  For example, the first write will
- * tell us the Guest's memory layout, pagetable, entry point and kernel address
- * offset.  A read will run the Guest until something happens, such as a signal
- * or the Guest doing a NOTIFY out to the Launcher. :*/
+ * tell us the Guest's memory layout and entry point.  A read will run the
+ * Guest until something happens, such as a signal or the Guest doing a NOTIFY
+ * out to the Launcher.
+:*/
 #include <linux/uaccess.h>
 #include <linux/miscdevice.h>
 #include <linux/fs.h>
@@ -11,14 +12,41 @@
 #include <linux/file.h>
 #include "lg.h"
 
+/*L:056
+ * Before we move on, let's jump ahead and look at what the kernel does when
+ * it needs to look up the eventfds.  That will complete our picture of how we
+ * use RCU.
+ *
+ * The notification value is in cpu->pending_notify: we return true if it went
+ * to an eventfd.
+ */
 bool send_notify_to_eventfd(struct lg_cpu *cpu)
 {
 	unsigned int i;
 	struct lg_eventfd_map *map;
 
-	/* lg->eventfds is RCU-protected */
+	/*
+	 * This "rcu_read_lock()" helps track when someone is still looking at
+	 * the (RCU-using) eventfds array.  It's not actually a lock at all;
+	 * indeed it's a noop in many configurations.  (You didn't expect me to
+	 * explain all the RCU secrets here, did you?)
+	 */
 	rcu_read_lock();
+	/*
+	 * rcu_dereference is the counter-side of rcu_assign_pointer(); it
+	 * makes sure we don't access the memory pointed to by
+	 * cpu->lg->eventfds before cpu->lg->eventfds is set.  Sounds crazy,
+	 * but Alpha allows this!  Paul McKenney points out that a really
+	 * aggressive compiler could have the same effect:
+	 *   http://lists.ozlabs.org/pipermail/lguest/2009-July/001560.html
+	 *
+	 * So play safe, use rcu_dereference to get the rcu-protected pointer:
+	 */
 	map = rcu_dereference(cpu->lg->eventfds);
+	/*
+	 * Simple array search: even if they add an eventfd while we do this,
+	 * we'll continue to use the old array and just won't see the new one.
+	 */
 	for (i = 0; i < map->num; i++) {
 		if (map->map[i].addr == cpu->pending_notify) {
 			eventfd_signal(map->map[i].event, 1);
@@ -26,19 +54,50 @@ bool send_notify_to_eventfd(struct lg_cpu *cpu)
 			break;
 		}
 	}
+	/* We're done with the rcu-protected variable cpu->lg->eventfds. */
 	rcu_read_unlock();
+
+	/* If we cleared the notification, it's because we found a match. */
 	return cpu->pending_notify == 0;
 }
 
+/*L:055
+ * One of the more tricksy tricks in the Linux Kernel is a technique called
+ * Read Copy Update.  Since one point of lguest is to teach lguest journeyers
+ * about kernel coding, I use it here.  (In case you're curious, other purposes
+ * include learning about virtualization and instilling a deep appreciation for
+ * simplicity and puppies).
+ *
+ * We keep a simple array which maps LHCALL_NOTIFY values to eventfds, but we
+ * add new eventfds without ever blocking readers from accessing the array.
+ * The current Launcher only does this during boot, so that never happens.  But
+ * Read Copy Update is cool, and adding a lock risks damaging even more puppies
+ * than this code does.
+ *
+ * We allocate a brand new one-larger array, copy the old one and add our new
+ * element.  Then we make the lg eventfd pointer point to the new array.
+ * That's the easy part: now we need to free the old one, but we need to make
+ * sure no slow CPU somewhere is still looking at it.  That's what
+ * synchronize_rcu does for us: waits until every CPU has indicated that it has
+ * moved on to know it's no longer using the old one.
+ *
+ * If that's unclear, see http://en.wikipedia.org/wiki/Read-copy-update.
+ */
 static int add_eventfd(struct lguest *lg, unsigned long addr, int fd)
 {
 	struct lg_eventfd_map *new, *old = lg->eventfds;
 
+	/*
+	 * We don't allow notifications on value 0 anyway (pending_notify of
+	 * 0 means "nothing pending").
+	 */
 	if (!addr)
 		return -EINVAL;
 
-	/* Replace the old array with the new one, carefully: others can
-	 * be accessing it at the same time */
+	/*
+	 * Replace the old array with the new one, carefully: others can
+	 * be accessing it at the same time.
+	 */
 	new = kmalloc(sizeof(*new) + sizeof(new->map[0]) * (old->num + 1),
 		      GFP_KERNEL);
 	if (!new)
@@ -52,22 +111,41 @@ static int add_eventfd(struct lguest *lg, unsigned long addr, int fd)
 	new->map[new->num].addr = addr;
 	new->map[new->num].event = eventfd_ctx_fdget(fd);
 	if (IS_ERR(new->map[new->num].event)) {
+		int err =  PTR_ERR(new->map[new->num].event);
 		kfree(new);
-		return PTR_ERR(new->map[new->num].event);
+		return err;
 	}
 	new->num++;
 
-	/* Now put new one in place. */
+	/*
+	 * Now put new one in place: rcu_assign_pointer() is a fancy way of
+	 * doing "lg->eventfds = new", but it uses memory barriers to make
+	 * absolutely sure that the contents of "new" written above is nailed
+	 * down before we actually do the assignment.
+	 *
+	 * We have to think about these kinds of things when we're operating on
+	 * live data without locks.
+	 */
 	rcu_assign_pointer(lg->eventfds, new);
 
-	/* We're not in a big hurry.  Wait until noone's looking at old
-	 * version, then delete it. */
+	/*
+	 * We're not in a big hurry.  Wait until noone's looking at old
+	 * version, then free it.
+	 */
 	synchronize_rcu();
 	kfree(old);
 
 	return 0;
 }
 
+/*L:052
+ * Receiving notifications from the Guest is usually done by attaching a
+ * particular LHCALL_NOTIFY value to an event filedescriptor.  The eventfd will
+ * become readable when the Guest does an LHCALL_NOTIFY with that value.
+ *
+ * This is really convenient for processing each virtqueue in a separate
+ * thread.
+ */
 static int attach_eventfd(struct lguest *lg, const unsigned long __user *input)
 {
 	unsigned long addr, fd;
@@ -79,15 +157,22 @@ static int attach_eventfd(struct lguest *lg, const unsigned long __user *input)
 	if (get_user(fd, input) != 0)
 		return -EFAULT;
 
+	/*
+	 * Just make sure two callers don't add eventfds at once.  We really
+	 * only need to lock against callers adding to the same Guest, so using
+	 * the Big Lguest Lock is overkill.  But this is setup, not a fast path.
+	 */
 	mutex_lock(&lguest_lock);
 	err = add_eventfd(lg, addr, fd);
 	mutex_unlock(&lguest_lock);
 
-	return 0;
+	return err;
 }
 
-/*L:050 Sending an interrupt is done by writing LHREQ_IRQ and an interrupt
- * number to /dev/lguest. */
+/*L:050
+ * Sending an interrupt is done by writing LHREQ_IRQ and an interrupt
+ * number to /dev/lguest.
+ */
 static int user_send_irq(struct lg_cpu *cpu, const unsigned long __user *input)
 {
 	unsigned long irq;
@@ -97,12 +182,18 @@ static int user_send_irq(struct lg_cpu *cpu, const unsigned long __user *input)
 	if (irq >= LGUEST_IRQS)
 		return -EINVAL;
 
+	/*
+	 * Next time the Guest runs, the core code will see if it can deliver
+	 * this interrupt.
+	 */
 	set_interrupt(cpu, irq);
 	return 0;
 }
 
-/*L:040 Once our Guest is initialized, the Launcher makes it run by reading
- * from /dev/lguest. */
+/*L:040
+ * Once our Guest is initialized, the Launcher makes it run by reading
+ * from /dev/lguest.
+ */
 static ssize_t read(struct file *file, char __user *user, size_t size,loff_t*o)
 {
 	struct lguest *lg = file->private_data;
@@ -138,8 +229,10 @@ static ssize_t read(struct file *file, char __user *user, size_t size,loff_t*o)
 		return len;
 	}
 
-	/* If we returned from read() last time because the Guest sent I/O,
-	 * clear the flag. */
+	/*
+	 * If we returned from read() last time because the Guest sent I/O,
+	 * clear the flag.
+	 */
 	if (cpu->pending_notify)
 		cpu->pending_notify = 0;
 
@@ -147,8 +240,10 @@ static ssize_t read(struct file *file, char __user *user, size_t size,loff_t*o)
 	return run_guest(cpu, (unsigned long __user *)user);
 }
 
-/*L:025 This actually initializes a CPU.  For the moment, a Guest is only
- * uniprocessor, so "id" is always 0. */
+/*L:025
+ * This actually initializes a CPU.  For the moment, a Guest is only
+ * uniprocessor, so "id" is always 0.
+ */
 static int lg_cpu_start(struct lg_cpu *cpu, unsigned id, unsigned long start_ip)
 {
 	/* We have a limited number the number of CPUs in the lguest struct. */
@@ -163,8 +258,10 @@ static int lg_cpu_start(struct lg_cpu *cpu, unsigned id, unsigned long start_ip)
 	/* Each CPU has a timer it can set. */
 	init_clockdev(cpu);
 
-	/* We need a complete page for the Guest registers: they are accessible
-	 * to the Guest and we can only grant it access to whole pages. */
+	/*
+	 * We need a complete page for the Guest registers: they are accessible
+	 * to the Guest and we can only grant it access to whole pages.
+	 */
 	cpu->regs_page = get_zeroed_page(GFP_KERNEL);
 	if (!cpu->regs_page)
 		return -ENOMEM;
@@ -172,29 +269,38 @@ static int lg_cpu_start(struct lg_cpu *cpu, unsigned id, unsigned long start_ip)
 	/* We actually put the registers at the bottom of the page. */
 	cpu->regs = (void *)cpu->regs_page + PAGE_SIZE - sizeof(*cpu->regs);
 
-	/* Now we initialize the Guest's registers, handing it the start
-	 * address. */
+	/*
+	 * Now we initialize the Guest's registers, handing it the start
+	 * address.
+	 */
 	lguest_arch_setup_regs(cpu, start_ip);
 
-	/* We keep a pointer to the Launcher task (ie. current task) for when
-	 * other Guests want to wake this one (eg. console input). */
+	/*
+	 * We keep a pointer to the Launcher task (ie. current task) for when
+	 * other Guests want to wake this one (eg. console input).
+	 */
 	cpu->tsk = current;
 
-	/* We need to keep a pointer to the Launcher's memory map, because if
+	/*
+	 * We need to keep a pointer to the Launcher's memory map, because if
 	 * the Launcher dies we need to clean it up.  If we don't keep a
-	 * reference, it is destroyed before close() is called. */
+	 * reference, it is destroyed before close() is called.
+	 */
 	cpu->mm = get_task_mm(cpu->tsk);
 
-	/* We remember which CPU's pages this Guest used last, for optimization
-	 * when the same Guest runs on the same CPU twice. */
+	/*
+	 * We remember which CPU's pages this Guest used last, for optimization
+	 * when the same Guest runs on the same CPU twice.
+	 */
 	cpu->last_pages = NULL;
 
 	/* No error == success. */
 	return 0;
 }
 
-/*L:020 The initialization write supplies 3 pointer sized (32 or 64 bit)
- * values (in addition to the LHREQ_INITIALIZE value).  These are:
+/*L:020
+ * The initialization write supplies 3 pointer sized (32 or 64 bit) values (in
+ * addition to the LHREQ_INITIALIZE value).  These are:
  *
  * base: The start of the Guest-physical memory inside the Launcher memory.
  *
@@ -206,14 +312,15 @@ static int lg_cpu_start(struct lg_cpu *cpu, unsigned id, unsigned long start_ip)
  */
 static int initialize(struct file *file, const unsigned long __user *input)
 {
-	/* "struct lguest" contains everything we (the Host) know about a
-	 * Guest. */
+	/* "struct lguest" contains all we (the Host) know about a Guest. */
 	struct lguest *lg;
 	int err;
 	unsigned long args[3];
 
-	/* We grab the Big Lguest lock, which protects against multiple
-	 * simultaneous initializations. */
+	/*
+	 * We grab the Big Lguest lock, which protects against multiple
+	 * simultaneous initializations.
+	 */
 	mutex_lock(&lguest_lock);
 	/* You can't initialize twice!  Close the device and start again... */
 	if (file->private_data) {
@@ -248,8 +355,10 @@ static int initialize(struct file *file, const unsigned long __user *input)
 	if (err)
 		goto free_eventfds;
 
-	/* Initialize the Guest's shadow page tables, using the toplevel
-	 * address the Launcher gave us.  This allocates memory, so can fail. */
+	/*
+	 * Initialize the Guest's shadow page tables, using the toplevel
+	 * address the Launcher gave us.  This allocates memory, so can fail.
+	 */
 	err = init_guest_pagetable(lg);
 	if (err)
 		goto free_regs;
@@ -274,20 +383,24 @@ unlock:
 	return err;
 }
 
-/*L:010 The first operation the Launcher does must be a write.  All writes
+/*L:010
+ * The first operation the Launcher does must be a write.  All writes
  * start with an unsigned long number: for the first write this must be
  * LHREQ_INITIALIZE to set up the Guest.  After that the Launcher can use
- * writes of other values to send interrupts.
+ * writes of other values to send interrupts or set up receipt of notifications.
  *
  * Note that we overload the "offset" in the /dev/lguest file to indicate what
- * CPU number we're dealing with.  Currently this is always 0, since we only
+ * CPU number we're dealing with.  Currently this is always 0 since we only
  * support uniprocessor Guests, but you can see the beginnings of SMP support
- * here. */
+ * here.
+ */
 static ssize_t write(struct file *file, const char __user *in,
 		     size_t size, loff_t *off)
 {
-	/* Once the Guest is initialized, we hold the "struct lguest" in the
-	 * file private data. */
+	/*
+	 * Once the Guest is initialized, we hold the "struct lguest" in the
+	 * file private data.
+	 */
 	struct lguest *lg = file->private_data;
 	const unsigned long __user *input = (const unsigned long __user *)in;
 	unsigned long req;
@@ -322,13 +435,15 @@ static ssize_t write(struct file *file, const char __user *in,
 	}
 }
 
-/*L:060 The final piece of interface code is the close() routine.  It reverses
+/*L:060
+ * The final piece of interface code is the close() routine.  It reverses
  * everything done in initialize().  This is usually called because the
  * Launcher exited.
  *
  * Note that the close routine returns 0 or a negative error number: it can't
  * really fail, but it can whine.  I blame Sun for this wart, and K&R C for
- * letting them do it. :*/
+ * letting them do it.
+:*/
 static int close(struct inode *inode, struct file *file)
 {
 	struct lguest *lg = file->private_data;
@@ -338,8 +453,10 @@ static int close(struct inode *inode, struct file *file)
 	if (!lg)
 		return 0;
 
-	/* We need the big lock, to protect from inter-guest I/O and other
-	 * Launchers initializing guests. */
+	/*
+	 * We need the big lock, to protect from inter-guest I/O and other
+	 * Launchers initializing guests.
+	 */
 	mutex_lock(&lguest_lock);
 
 	/* Free up the shadow page tables for the Guest. */
@@ -350,8 +467,10 @@ static int close(struct inode *inode, struct file *file)
 		hrtimer_cancel(&lg->cpus[i].hrt);
 		/* We can free up the register page we allocated. */
 		free_page(lg->cpus[i].regs_page);
-		/* Now all the memory cleanups are done, it's safe to release
-		 * the Launcher's memory management structure. */
+		/*
+		 * Now all the memory cleanups are done, it's safe to release
+		 * the Launcher's memory management structure.
+		 */
 		mmput(lg->cpus[i].mm);
 	}
 
@@ -360,8 +479,10 @@ static int close(struct inode *inode, struct file *file)
 		eventfd_ctx_put(lg->eventfds->map[i].event);
 	kfree(lg->eventfds);
 
-	/* If lg->dead doesn't contain an error code it will be NULL or a
-	 * kmalloc()ed string, either of which is ok to hand to kfree(). */
+	/*
+	 * If lg->dead doesn't contain an error code it will be NULL or a
+	 * kmalloc()ed string, either of which is ok to hand to kfree().
+	 */
 	if (!IS_ERR(lg->dead))
 		kfree(lg->dead);
 	/* Free the memory allocated to the lguest_struct */
@@ -385,7 +506,8 @@ static int close(struct inode *inode, struct file *file)
  *
  * We begin our understanding with the Host kernel interface which the Launcher
  * uses: reading and writing a character device called /dev/lguest.  All the
- * work happens in the read(), write() and close() routines: */
+ * work happens in the read(), write() and close() routines:
+ */
 static struct file_operations lguest_fops = {
 	.owner	 = THIS_MODULE,
 	.release = close,
@@ -393,8 +515,10 @@ static struct file_operations lguest_fops = {
 	.read	 = read,
 };
 
-/* This is a textbook example of a "misc" character device.  Populate a "struct
- * miscdevice" and register it with misc_register(). */
+/*
+ * This is a textbook example of a "misc" character device.  Populate a "struct
+ * miscdevice" and register it with misc_register().
+ */
 static struct miscdevice lguest_dev = {
 	.minor	= MISC_DYNAMIC_MINOR,
 	.name	= "lguest",

drivers/lguest/segments.c

@@ -1,4 +1,5 @@
-/*P:600 The x86 architecture has segments, which involve a table of descriptors
+/*P:600
+ * The x86 architecture has segments, which involve a table of descriptors
  * which can be used to do funky things with virtual address interpretation.
  * We originally used to use segments so the Guest couldn't alter the
  * Guest<->Host Switcher, and then we had to trim Guest segments, and restore
@@ -8,7 +9,8 @@
  *
  * In these modern times, the segment handling code consists of simple sanity
  * checks, and the worst you'll experience reading this code is butterfly-rash
- * from frolicking through its parklike serenity. :*/
+ * from frolicking through its parklike serenity.
+:*/
 #include "lg.h"
 
 /*H:600
@@ -41,10 +43,12 @@
  * begin.
  */
 
-/* There are several entries we don't let the Guest set.  The TSS entry is the
+/*
+ * There are several entries we don't let the Guest set.  The TSS entry is the
  * "Task State Segment" which controls all kinds of delicate things.  The
  * LGUEST_CS and LGUEST_DS entries are reserved for the Switcher, and the
- * the Guest can't be trusted to deal with double faults. */
+ * the Guest can't be trusted to deal with double faults.
+ */
 static bool ignored_gdt(unsigned int num)
 {
 	return (num == GDT_ENTRY_TSS
@@ -53,42 +57,52 @@ static bool ignored_gdt(unsigned int num)
 		|| num == GDT_ENTRY_DOUBLEFAULT_TSS);
 }
 
-/*H:630 Once the Guest gave us new GDT entries, we fix them up a little.  We
+/*H:630
+ * Once the Guest gave us new GDT entries, we fix them up a little.  We
  * don't care if they're invalid: the worst that can happen is a General
  * Protection Fault in the Switcher when it restores a Guest segment register
  * which tries to use that entry.  Then we kill the Guest for causing such a
- * mess: the message will be "unhandled trap 256". */
+ * mess: the message will be "unhandled trap 256".
+ */
 static void fixup_gdt_table(struct lg_cpu *cpu, unsigned start, unsigned end)
 {
 	unsigned int i;
 
 	for (i = start; i < end; i++) {
-		/* We never copy these ones to real GDT, so we don't care what
-		 * they say */
+		/*
+		 * We never copy these ones to real GDT, so we don't care what
+		 * they say
+		 */
 		if (ignored_gdt(i))
 			continue;
 
-		/* Segment descriptors contain a privilege level: the Guest is
+		/*
+		 * Segment descriptors contain a privilege level: the Guest is
 		 * sometimes careless and leaves this as 0, even though it's
-		 * running at privilege level 1.  If so, we fix it here. */
+		 * running at privilege level 1.  If so, we fix it here.
+		 */
 		if ((cpu->arch.gdt[i].b & 0x00006000) == 0)
 			cpu->arch.gdt[i].b |= (GUEST_PL << 13);
 
-		/* Each descriptor has an "accessed" bit.  If we don't set it
+		/*
+		 * Each descriptor has an "accessed" bit.  If we don't set it
 		 * now, the CPU will try to set it when the Guest first loads
 		 * that entry into a segment register.  But the GDT isn't
-		 * writable by the Guest, so bad things can happen. */
+		 * writable by the Guest, so bad things can happen.
+		 */
 		cpu->arch.gdt[i].b |= 0x00000100;
 	}
 }
 
-/*H:610 Like the IDT, we never simply use the GDT the Guest gives us.  We keep
+/*H:610
+ * Like the IDT, we never simply use the GDT the Guest gives us.  We keep
  * a GDT for each CPU, and copy across the Guest's entries each time we want to
  * run the Guest on that CPU.
  *
  * This routine is called at boot or modprobe time for each CPU to set up the
  * constant GDT entries: the ones which are the same no matter what Guest we're
- * running. */
+ * running.
+ */
 void setup_default_gdt_entries(struct lguest_ro_state *state)
 {
 	struct desc_struct *gdt = state->guest_gdt;
@@ -98,30 +112,37 @@ void setup_default_gdt_entries(struct lguest_ro_state *state)
 	gdt[GDT_ENTRY_LGUEST_CS] = FULL_EXEC_SEGMENT;
 	gdt[GDT_ENTRY_LGUEST_DS] = FULL_SEGMENT;
 
-	/* The TSS segment refers to the TSS entry for this particular CPU.
+	/*
+	 * The TSS segment refers to the TSS entry for this particular CPU.
 	 * Forgive the magic flags: the 0x8900 means the entry is Present, it's
 	 * privilege level 0 Available 386 TSS system segment, and the 0x67
-	 * means Saturn is eclipsed by Mercury in the twelfth house. */
+	 * means Saturn is eclipsed by Mercury in the twelfth house.
+	 */
 	gdt[GDT_ENTRY_TSS].a = 0x00000067 | (tss << 16);
 	gdt[GDT_ENTRY_TSS].b = 0x00008900 | (tss & 0xFF000000)
 		| ((tss >> 16) & 0x000000FF);
 }
 
-/* This routine sets up the initial Guest GDT for booting.  All entries start
- * as 0 (unusable). */
+/*
+ * This routine sets up the initial Guest GDT for booting.  All entries start
+ * as 0 (unusable).
+ */
 void setup_guest_gdt(struct lg_cpu *cpu)
 {
-	/* Start with full 0-4G segments... */
+	/*
+	 * Start with full 0-4G segments...except the Guest is allowed to use
+	 * them, so set the privilege level appropriately in the flags.
+	 */
 	cpu->arch.gdt[GDT_ENTRY_KERNEL_CS] = FULL_EXEC_SEGMENT;
 	cpu->arch.gdt[GDT_ENTRY_KERNEL_DS] = FULL_SEGMENT;
-	/* ...except the Guest is allowed to use them, so set the privilege
-	 * level appropriately in the flags. */
 	cpu->arch.gdt[GDT_ENTRY_KERNEL_CS].b |= (GUEST_PL << 13);
 	cpu->arch.gdt[GDT_ENTRY_KERNEL_DS].b |= (GUEST_PL << 13);
 }
 
-/*H:650 An optimization of copy_gdt(), for just the three "thead-local storage"
- * entries. */
+/*H:650
+ * An optimization of copy_gdt(), for just the three "thead-local storage"
+ * entries.
+ */
 void copy_gdt_tls(const struct lg_cpu *cpu, struct desc_struct *gdt)
 {
 	unsigned int i;
@@ -130,26 +151,34 @@ void copy_gdt_tls(const struct lg_cpu *cpu, struct desc_struct *gdt)
 		gdt[i] = cpu->arch.gdt[i];
 }
 
-/*H:640 When the Guest is run on a different CPU, or the GDT entries have
- * changed, copy_gdt() is called to copy the Guest's GDT entries across to this
- * CPU's GDT. */
+/*H:640
+ * When the Guest is run on a different CPU, or the GDT entries have changed,
+ * copy_gdt() is called to copy the Guest's GDT entries across to this CPU's
+ * GDT.
+ */
 void copy_gdt(const struct lg_cpu *cpu, struct desc_struct *gdt)
 {
 	unsigned int i;
 
-	/* The default entries from setup_default_gdt_entries() are not
-	 * replaced.  See ignored_gdt() above. */
+	/*
+	 * The default entries from setup_default_gdt_entries() are not
+	 * replaced.  See ignored_gdt() above.
+	 */
 	for (i = 0; i < GDT_ENTRIES; i++)
 		if (!ignored_gdt(i))
 			gdt[i] = cpu->arch.gdt[i];
 }
 
-/*H:620 This is where the Guest asks us to load a new GDT entry
- * (LHCALL_LOAD_GDT_ENTRY).  We tweak the entry and copy it in. */
+/*H:620
+ * This is where the Guest asks us to load a new GDT entry
+ * (LHCALL_LOAD_GDT_ENTRY).  We tweak the entry and copy it in.
+ */
 void load_guest_gdt_entry(struct lg_cpu *cpu, u32 num, u32 lo, u32 hi)
 {
-	/* We assume the Guest has the same number of GDT entries as the
-	 * Host, otherwise we'd have to dynamically allocate the Guest GDT. */
+	/*
+	 * We assume the Guest has the same number of GDT entries as the
+	 * Host, otherwise we'd have to dynamically allocate the Guest GDT.
+	 */
 	if (num >= ARRAY_SIZE(cpu->arch.gdt))
 		kill_guest(cpu, "too many gdt entries %i", num);
 
@@ -157,15 +186,19 @@ void load_guest_gdt_entry(struct lg_cpu *cpu, u32 num, u32 lo, u32 hi)
 	cpu->arch.gdt[num].a = lo;
 	cpu->arch.gdt[num].b = hi;
 	fixup_gdt_table(cpu, num, num+1);
-	/* Mark that the GDT changed so the core knows it has to copy it again,
-	 * even if the Guest is run on the same CPU. */
+	/*
+	 * Mark that the GDT changed so the core knows it has to copy it again,
+	 * even if the Guest is run on the same CPU.
+	 */
 	cpu->changed |= CHANGED_GDT;
 }
 
-/* This is the fast-track version for just changing the three TLS entries.
+/*
+ * This is the fast-track version for just changing the three TLS entries.
  * Remember that this happens on every context switch, so it's worth
  * optimizing.  But wouldn't it be neater to have a single hypercall to cover
- * both cases? */
+ * both cases?
+ */
 void guest_load_tls(struct lg_cpu *cpu, unsigned long gtls)
 {
 	struct desc_struct *tls = &cpu->arch.gdt[GDT_ENTRY_TLS_MIN];
@@ -175,7 +208,6 @@ void guest_load_tls(struct lg_cpu *cpu, unsigned long gtls)
 	/* Note that just the TLS entries have changed. */
 	cpu->changed |= CHANGED_GDT_TLS;
 }
-/*:*/
 
 /*H:660
  * With this, we have finished the Host.

drivers/lguest/x86/core.c

@@ -17,13 +17,15 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
-/*P:450 This file contains the x86-specific lguest code.  It used to be all
+/*P:450
+ * This file contains the x86-specific lguest code.  It used to be all
  * mixed in with drivers/lguest/core.c but several foolhardy code slashers
  * wrestled most of the dependencies out to here in preparation for porting
  * lguest to other architectures (see what I mean by foolhardy?).
  *
  * This also contains a couple of non-obvious setup and teardown pieces which
- * were implemented after days of debugging pain. :*/
+ * were implemented after days of debugging pain.
+:*/
 #include <linux/kernel.h>
 #include <linux/start_kernel.h>
 #include <linux/string.h>
@@ -82,25 +84,33 @@ static DEFINE_PER_CPU(struct lg_cpu *, last_cpu);
  */
 static void copy_in_guest_info(struct lg_cpu *cpu, struct lguest_pages *pages)
 {
-	/* Copying all this data can be quite expensive.  We usually run the
+	/*
+	 * Copying all this data can be quite expensive.  We usually run the
 	 * same Guest we ran last time (and that Guest hasn't run anywhere else
 	 * meanwhile).  If that's not the case, we pretend everything in the
-	 * Guest has changed. */
+	 * Guest has changed.
+	 */
 	if (__get_cpu_var(last_cpu) != cpu || cpu->last_pages != pages) {
 		__get_cpu_var(last_cpu) = cpu;
 		cpu->last_pages = pages;
 		cpu->changed = CHANGED_ALL;
 	}
 
-	/* These copies are pretty cheap, so we do them unconditionally: */
-	/* Save the current Host top-level page directory. */
+	/*
+	 * These copies are pretty cheap, so we do them unconditionally: */
+	/* Save the current Host top-level page directory.
+	 */
 	pages->state.host_cr3 = __pa(current->mm->pgd);
-	/* Set up the Guest's page tables to see this CPU's pages (and no
-	 * other CPU's pages). */
+	/*
+	 * Set up the Guest's page tables to see this CPU's pages (and no
+	 * other CPU's pages).
+	 */
 	map_switcher_in_guest(cpu, pages);
-	/* Set up the two "TSS" members which tell the CPU what stack to use
+	/*
+	 * Set up the two "TSS" members which tell the CPU what stack to use
 	 * for traps which do directly into the Guest (ie. traps at privilege
-	 * level 1). */
+	 * level 1).
+	 */
 	pages->state.guest_tss.sp1 = cpu->esp1;
 	pages->state.guest_tss.ss1 = cpu->ss1;
 
@@ -125,97 +135,126 @@ static void run_guest_once(struct lg_cpu *cpu, struct lguest_pages *pages)
 	/* This is a dummy value we need for GCC's sake. */
 	unsigned int clobber;
 
-	/* Copy the guest-specific information into this CPU's "struct
-	 * lguest_pages". */
+	/*
+	 * Copy the guest-specific information into this CPU's "struct
+	 * lguest_pages".
+	 */
 	copy_in_guest_info(cpu, pages);
 
-	/* Set the trap number to 256 (impossible value).  If we fault while
+	/*
+	 * Set the trap number to 256 (impossible value).  If we fault while
 	 * switching to the Guest (bad segment registers or bug), this will
-	 * cause us to abort the Guest. */
+	 * cause us to abort the Guest.
+	 */
 	cpu->regs->trapnum = 256;
 
-	/* Now: we push the "eflags" register on the stack, then do an "lcall".
+	/*
+	 * Now: we push the "eflags" register on the stack, then do an "lcall".
 	 * This is how we change from using the kernel code segment to using
 	 * the dedicated lguest code segment, as well as jumping into the
 	 * Switcher.
 	 *
 	 * The lcall also pushes the old code segment (KERNEL_CS) onto the
 	 * stack, then the address of this call.  This stack layout happens to
-	 * exactly match the stack layout created by an interrupt... */
+	 * exactly match the stack layout created by an interrupt...
+	 */
 	asm volatile("pushf; lcall *lguest_entry"
-		     /* This is how we tell GCC that %eax ("a") and %ebx ("b")
-		      * are changed by this routine.  The "=" means output. */
+		     /*
+		      * This is how we tell GCC that %eax ("a") and %ebx ("b")
+		      * are changed by this routine.  The "=" means output.
+		      */
 		     : "=a"(clobber), "=b"(clobber)
-		     /* %eax contains the pages pointer.  ("0" refers to the
+		     /*
+		      * %eax contains the pages pointer.  ("0" refers to the
 		      * 0-th argument above, ie "a").  %ebx contains the
 		      * physical address of the Guest's top-level page
-		      * directory. */
+		      * directory.
+		      */
 		     : "0"(pages), "1"(__pa(cpu->lg->pgdirs[cpu->cpu_pgd].pgdir))
-		     /* We tell gcc that all these registers could change,
+		     /*
+		      * We tell gcc that all these registers could change,
 		      * which means we don't have to save and restore them in
-		      * the Switcher. */
+		      * the Switcher.
+		      */
 		     : "memory", "%edx", "%ecx", "%edi", "%esi");
 }
 /*:*/
 
-/*M:002 There are hooks in the scheduler which we can register to tell when we
+/*M:002
+ * There are hooks in the scheduler which we can register to tell when we
  * get kicked off the CPU (preempt_notifier_register()).  This would allow us
  * to lazily disable SYSENTER which would regain some performance, and should
  * also simplify copy_in_guest_info().  Note that we'd still need to restore
  * things when we exit to Launcher userspace, but that's fairly easy.
  *
- * We could also try using this hooks for PGE, but that might be too expensive.
+ * We could also try using these hooks for PGE, but that might be too expensive.
  *
- * The hooks were designed for KVM, but we can also put them to good use. :*/
+ * The hooks were designed for KVM, but we can also put them to good use.
+:*/
 
-/*H:040 This is the i386-specific code to setup and run the Guest.  Interrupts
- * are disabled: we own the CPU. */
+/*H:040
+ * This is the i386-specific code to setup and run the Guest.  Interrupts
+ * are disabled: we own the CPU.
+ */
 void lguest_arch_run_guest(struct lg_cpu *cpu)
 {
-	/* Remember the awfully-named TS bit?  If the Guest has asked to set it
+	/*
+	 * Remember the awfully-named TS bit?  If the Guest has asked to set it
 	 * we set it now, so we can trap and pass that trap to the Guest if it
-	 * uses the FPU. */
+	 * uses the FPU.
+	 */
 	if (cpu->ts)
 		unlazy_fpu(current);
 
-	/* SYSENTER is an optimized way of doing system calls.  We can't allow
+	/*
+	 * SYSENTER is an optimized way of doing system calls.  We can't allow
 	 * it because it always jumps to privilege level 0.  A normal Guest
 	 * won't try it because we don't advertise it in CPUID, but a malicious
 	 * Guest (or malicious Guest userspace program) could, so we tell the
-	 * CPU to disable it before running the Guest. */
+	 * CPU to disable it before running the Guest.
+	 */
 	if (boot_cpu_has(X86_FEATURE_SEP))
 		wrmsr(MSR_IA32_SYSENTER_CS, 0, 0);
 
-	/* Now we actually run the Guest.  It will return when something
+	/*
+	 * Now we actually run the Guest.  It will return when something
 	 * interesting happens, and we can examine its registers to see what it
-	 * was doing. */
+	 * was doing.
+	 */
 	run_guest_once(cpu, lguest_pages(raw_smp_processor_id()));
 
-	/* Note that the "regs" structure contains two extra entries which are
+	/*
+	 * Note that the "regs" structure contains two extra entries which are
 	 * not really registers: a trap number which says what interrupt or
 	 * trap made the switcher code come back, and an error code which some
-	 * traps set.  */
+	 * traps set.
+	 */
 
 	 /* Restore SYSENTER if it's supposed to be on. */
 	 if (boot_cpu_has(X86_FEATURE_SEP))
 		wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
 
-	/* If the Guest page faulted, then the cr2 register will tell us the
+	/*
+	 * If the Guest page faulted, then the cr2 register will tell us the
 	 * bad virtual address.  We have to grab this now, because once we
 	 * re-enable interrupts an interrupt could fault and thus overwrite
-	 * cr2, or we could even move off to a different CPU. */
+	 * cr2, or we could even move off to a different CPU.
+	 */
 	if (cpu->regs->trapnum == 14)
 		cpu->arch.last_pagefault = read_cr2();
-	/* Similarly, if we took a trap because the Guest used the FPU,
+	/*
+	 * Similarly, if we took a trap because the Guest used the FPU,
 	 * we have to restore the FPU it expects to see.
 	 * math_state_restore() may sleep and we may even move off to
 	 * a different CPU. So all the critical stuff should be done
-	 * before this.  */
+	 * before this.
+	 */
 	else if (cpu->regs->trapnum == 7)
 		math_state_restore();
 }
 
-/*H:130 Now we've examined the hypercall code; our Guest can make requests.
+/*H:130
+ * Now we've examined the hypercall code; our Guest can make requests.
  * Our Guest is usually so well behaved; it never tries to do things it isn't
  * allowed to, and uses hypercalls instead.  Unfortunately, Linux's paravirtual
  * infrastructure isn't quite complete, because it doesn't contain replacements
@@ -225,26 +264,33 @@ void lguest_arch_run_guest(struct lg_cpu *cpu)
  *
  * When the Guest uses one of these instructions, we get a trap (General
  * Protection Fault) and come here.  We see if it's one of those troublesome
- * instructions and skip over it.  We return true if we did. */
+ * instructions and skip over it.  We return true if we did.
+ */
 static int emulate_insn(struct lg_cpu *cpu)
 {
 	u8 insn;
 	unsigned int insnlen = 0, in = 0, shift = 0;
-	/* The eip contains the *virtual* address of the Guest's instruction:
-	 * guest_pa just subtracts the Guest's page_offset. */
+	/*
+	 * The eip contains the *virtual* address of the Guest's instruction:
+	 * guest_pa just subtracts the Guest's page_offset.
+	 */
 	unsigned long physaddr = guest_pa(cpu, cpu->regs->eip);
 
-	/* This must be the Guest kernel trying to do something, not userspace!
+	/*
+	 * This must be the Guest kernel trying to do something, not userspace!
 	 * The bottom two bits of the CS segment register are the privilege
-	 * level. */
+	 * level.
+	 */
 	if ((cpu->regs->cs & 3) != GUEST_PL)
 		return 0;
 
 	/* Decoding x86 instructions is icky. */
 	insn = lgread(cpu, physaddr, u8);
 
-	/* 0x66 is an "operand prefix".  It means it's using the upper 16 bits
-	   of the eax register. */
+	/*
+	 * 0x66 is an "operand prefix".  It means it's using the upper 16 bits
+	 * of the eax register.
+	 */
 	if (insn == 0x66) {
 		shift = 16;
 		/* The instruction is 1 byte so far, read the next byte. */
@@ -252,8 +298,10 @@ static int emulate_insn(struct lg_cpu *cpu)
 		insn = lgread(cpu, physaddr + insnlen, u8);
 	}
 
-	/* We can ignore the lower bit for the moment and decode the 4 opcodes
-	 * we need to emulate. */
+	/*
+	 * We can ignore the lower bit for the moment and decode the 4 opcodes
+	 * we need to emulate.
+	 */
 	switch (insn & 0xFE) {
 	case 0xE4: /* in     <next byte>,%al */
 		insnlen += 2;
@@ -274,9 +322,11 @@ static int emulate_insn(struct lg_cpu *cpu)
 		return 0;
 	}
 
-	/* If it was an "IN" instruction, they expect the result to be read
+	/*
+	 * If it was an "IN" instruction, they expect the result to be read
 	 * into %eax, so we change %eax.  We always return all-ones, which
-	 * traditionally means "there's nothing there". */
+	 * traditionally means "there's nothing there".
+	 */
 	if (in) {
 		/* Lower bit tells is whether it's a 16 or 32 bit access */
 		if (insn & 0x1)
@@ -290,7 +340,8 @@ static int emulate_insn(struct lg_cpu *cpu)
 	return 1;
 }
 
-/* Our hypercalls mechanism used to be based on direct software interrupts.
+/*
+ * Our hypercalls mechanism used to be based on direct software interrupts.
  * After Anthony's "Refactor hypercall infrastructure" kvm patch, we decided to
  * change over to using kvm hypercalls.
  *
@@ -318,16 +369,20 @@ static int emulate_insn(struct lg_cpu *cpu)
  */
 static void rewrite_hypercall(struct lg_cpu *cpu)
 {
-	/* This are the opcodes we use to patch the Guest.  The opcode for "int
+	/*
+	 * This are the opcodes we use to patch the Guest.  The opcode for "int
 	 * $0x1f" is "0xcd 0x1f" but vmcall instruction is 3 bytes long, so we
-	 * complete the sequence with a NOP (0x90). */
+	 * complete the sequence with a NOP (0x90).
+	 */
 	u8 insn[3] = {0xcd, 0x1f, 0x90};
 
 	__lgwrite(cpu, guest_pa(cpu, cpu->regs->eip), insn, sizeof(insn));
-	/* The above write might have caused a copy of that page to be made
+	/*
+	 * The above write might have caused a copy of that page to be made
 	 * (if it was read-only).  We need to make sure the Guest has
 	 * up-to-date pagetables.  As this doesn't happen often, we can just
-	 * drop them all. */
+	 * drop them all.
+	 */
 	guest_pagetable_clear_all(cpu);
 }
 
@@ -335,9 +390,11 @@ static bool is_hypercall(struct lg_cpu *cpu)
 {
 	u8 insn[3];
 
-	/* This must be the Guest kernel trying to do something.
+	/*
+	 * This must be the Guest kernel trying to do something.
 	 * The bottom two bits of the CS segment register are the privilege
-	 * level. */
+	 * level.
+	 */
 	if ((cpu->regs->cs & 3) != GUEST_PL)
 		return false;
 
@@ -351,86 +408,105 @@ void lguest_arch_handle_trap(struct lg_cpu *cpu)
 {
 	switch (cpu->regs->trapnum) {
 	case 13: /* We've intercepted a General Protection Fault. */
-		/* Check if this was one of those annoying IN or OUT
+		/*
+		 * Check if this was one of those annoying IN or OUT
 		 * instructions which we need to emulate.  If so, we just go
-		 * back into the Guest after we've done it. */
+		 * back into the Guest after we've done it.
+		 */
 		if (cpu->regs->errcode == 0) {
 			if (emulate_insn(cpu))
 				return;
 		}
-		/* If KVM is active, the vmcall instruction triggers a
-		 * General Protection Fault.  Normally it triggers an
-		 * invalid opcode fault (6): */
+		/*
+		 * If KVM is active, the vmcall instruction triggers a General
+		 * Protection Fault.  Normally it triggers an invalid opcode
+		 * fault (6):
+		 */
 	case 6:
-		/* We need to check if ring == GUEST_PL and
-		 * faulting instruction == vmcall. */
+		/*
+		 * We need to check if ring == GUEST_PL and faulting
+		 * instruction == vmcall.
+		 */
 		if (is_hypercall(cpu)) {
 			rewrite_hypercall(cpu);
 			return;
 		}
 		break;
 	case 14: /* We've intercepted a Page Fault. */
-		/* The Guest accessed a virtual address that wasn't mapped.
+		/*
+		 * The Guest accessed a virtual address that wasn't mapped.
 		 * This happens a lot: we don't actually set up most of the page
 		 * tables for the Guest at all when we start: as it runs it asks
 		 * for more and more, and we set them up as required. In this
 		 * case, we don't even tell the Guest that the fault happened.
 		 *
 		 * The errcode tells whether this was a read or a write, and
-		 * whether kernel or userspace code. */
+		 * whether kernel or userspace code.
+		 */
 		if (demand_page(cpu, cpu->arch.last_pagefault,
 				cpu->regs->errcode))
 			return;
 
-		/* OK, it's really not there (or not OK): the Guest needs to
+		/*
+		 * OK, it's really not there (or not OK): the Guest needs to
 		 * know.  We write out the cr2 value so it knows where the
 		 * fault occurred.
 		 *
 		 * Note that if the Guest were really messed up, this could
 		 * happen before it's done the LHCALL_LGUEST_INIT hypercall, so
-		 * lg->lguest_data could be NULL */
+		 * lg->lguest_data could be NULL
+		 */
 		if (cpu->lg->lguest_data &&
 		    put_user(cpu->arch.last_pagefault,
 			     &cpu->lg->lguest_data->cr2))
 			kill_guest(cpu, "Writing cr2");
 		break;
 	case 7: /* We've intercepted a Device Not Available fault. */
-		/* If the Guest doesn't want to know, we already restored the
-		 * Floating Point Unit, so we just continue without telling
-		 * it. */
+		/*
+		 * If the Guest doesn't want to know, we already restored the
+		 * Floating Point Unit, so we just continue without telling it.
+		 */
 		if (!cpu->ts)
 			return;
 		break;
 	case 32 ... 255:
-		/* These values mean a real interrupt occurred, in which case
+		/*
+		 * These values mean a real interrupt occurred, in which case
 		 * the Host handler has already been run. We just do a
 		 * friendly check if another process should now be run, then
-		 * return to run the Guest again */
+		 * return to run the Guest again
+		 */
 		cond_resched();
 		return;
 	case LGUEST_TRAP_ENTRY:
-		/* Our 'struct hcall_args' maps directly over our regs: we set
-		 * up the pointer now to indicate a hypercall is pending. */
+		/*
+		 * Our 'struct hcall_args' maps directly over our regs: we set
+		 * up the pointer now to indicate a hypercall is pending.
+		 */
 		cpu->hcall = (struct hcall_args *)cpu->regs;
 		return;
 	}
 
 	/* We didn't handle the trap, so it needs to go to the Guest. */
 	if (!deliver_trap(cpu, cpu->regs->trapnum))
-		/* If the Guest doesn't have a handler (either it hasn't
+		/*
+		 * If the Guest doesn't have a handler (either it hasn't
 		 * registered any yet, or it's one of the faults we don't let
-		 * it handle), it dies with this cryptic error message. */
+		 * it handle), it dies with this cryptic error message.
+		 */
 		kill_guest(cpu, "unhandled trap %li at %#lx (%#lx)",
 			   cpu->regs->trapnum, cpu->regs->eip,
 			   cpu->regs->trapnum == 14 ? cpu->arch.last_pagefault
 			   : cpu->regs->errcode);
 }
 
-/* Now we can look at each of the routines this calls, in increasing order of
+/*
+ * Now we can look at each of the routines this calls, in increasing order of
  * complexity: do_hypercalls(), emulate_insn(), maybe_do_interrupt(),
  * deliver_trap() and demand_page().  After all those, we'll be ready to
  * examine the Switcher, and our philosophical understanding of the Host/Guest
- * duality will be complete. :*/
+ * duality will be complete.
+:*/
 static void adjust_pge(void *on)
 {
 	if (on)
@@ -439,13 +515,16 @@ static void adjust_pge(void *on)
 		write_cr4(read_cr4() & ~X86_CR4_PGE);
 }
 
-/*H:020 Now the Switcher is mapped and every thing else is ready, we need to do
- * some more i386-specific initialization. */
+/*H:020
+ * Now the Switcher is mapped and every thing else is ready, we need to do
+ * some more i386-specific initialization.
+ */
 void __init lguest_arch_host_init(void)
 {
 	int i;
 
-	/* Most of the i386/switcher.S doesn't care that it's been moved; on
+	/*
+	 * Most of the i386/switcher.S doesn't care that it's been moved; on
 	 * Intel, jumps are relative, and it doesn't access any references to
 	 * external code or data.
 	 *
@@ -453,7 +532,8 @@ void __init lguest_arch_host_init(void)
 	 * addresses are placed in a table (default_idt_entries), so we need to
 	 * update the table with the new addresses.  switcher_offset() is a
 	 * convenience function which returns the distance between the
-	 * compiled-in switcher code and the high-mapped copy we just made. */
+	 * compiled-in switcher code and the high-mapped copy we just made.
+	 */
 	for (i = 0; i < IDT_ENTRIES; i++)
 		default_idt_entries[i] += switcher_offset();
 
@@ -468,63 +548,81 @@ void __init lguest_arch_host_init(void)
 	for_each_possible_cpu(i) {
 		/* lguest_pages() returns this CPU's two pages. */
 		struct lguest_pages *pages = lguest_pages(i);
-		/* This is a convenience pointer to make the code fit one
-		 * statement to a line. */
+		/* This is a convenience pointer to make the code neater. */
 		struct lguest_ro_state *state = &pages->state;
 
-		/* The Global Descriptor Table: the Host has a different one
+		/*
+		 * The Global Descriptor Table: the Host has a different one
 		 * for each CPU.  We keep a descriptor for the GDT which says
 		 * where it is and how big it is (the size is actually the last
-		 * byte, not the size, hence the "-1"). */
+		 * byte, not the size, hence the "-1").
+		 */
 		state->host_gdt_desc.size = GDT_SIZE-1;
 		state->host_gdt_desc.address = (long)get_cpu_gdt_table(i);
 
-		/* All CPUs on the Host use the same Interrupt Descriptor
+		/*
+		 * All CPUs on the Host use the same Interrupt Descriptor
 		 * Table, so we just use store_idt(), which gets this CPU's IDT
-		 * descriptor. */
+		 * descriptor.
+		 */
 		store_idt(&state->host_idt_desc);
 
-		/* The descriptors for the Guest's GDT and IDT can be filled
+		/*
+		 * The descriptors for the Guest's GDT and IDT can be filled
 		 * out now, too.  We copy the GDT & IDT into ->guest_gdt and
-		 * ->guest_idt before actually running the Guest. */
+		 * ->guest_idt before actually running the Guest.
+		 */
 		state->guest_idt_desc.size = sizeof(state->guest_idt)-1;
 		state->guest_idt_desc.address = (long)&state->guest_idt;
 		state->guest_gdt_desc.size = sizeof(state->guest_gdt)-1;
 		state->guest_gdt_desc.address = (long)&state->guest_gdt;
 
-		/* We know where we want the stack to be when the Guest enters
+		/*
+		 * We know where we want the stack to be when the Guest enters
 		 * the Switcher: in pages->regs.  The stack grows upwards, so
-		 * we start it at the end of that structure. */
+		 * we start it at the end of that structure.
+		 */
 		state->guest_tss.sp0 = (long)(&pages->regs + 1);
-		/* And this is the GDT entry to use for the stack: we keep a
-		 * couple of special LGUEST entries. */
+		/*
+		 * And this is the GDT entry to use for the stack: we keep a
+		 * couple of special LGUEST entries.
+		 */
 		state->guest_tss.ss0 = LGUEST_DS;
 
-		/* x86 can have a finegrained bitmap which indicates what I/O
+		/*
+		 * x86 can have a finegrained bitmap which indicates what I/O
 		 * ports the process can use.  We set it to the end of our
-		 * structure, meaning "none". */
+		 * structure, meaning "none".
+		 */
 		state->guest_tss.io_bitmap_base = sizeof(state->guest_tss);
 
-		/* Some GDT entries are the same across all Guests, so we can
-		 * set them up now. */
+		/*
+		 * Some GDT entries are the same across all Guests, so we can
+		 * set them up now.
+		 */
 		setup_default_gdt_entries(state);
 		/* Most IDT entries are the same for all Guests, too.*/
 		setup_default_idt_entries(state, default_idt_entries);
 
-		/* The Host needs to be able to use the LGUEST segments on this
-		 * CPU, too, so put them in the Host GDT. */
+		/*
+		 * The Host needs to be able to use the LGUEST segments on this
+		 * CPU, too, so put them in the Host GDT.
+		 */
 		get_cpu_gdt_table(i)[GDT_ENTRY_LGUEST_CS] = FULL_EXEC_SEGMENT;
 		get_cpu_gdt_table(i)[GDT_ENTRY_LGUEST_DS] = FULL_SEGMENT;
 	}
 
-	/* In the Switcher, we want the %cs segment register to use the
+	/*
+	 * In the Switcher, we want the %cs segment register to use the
 	 * LGUEST_CS GDT entry: we've put that in the Host and Guest GDTs, so
 	 * it will be undisturbed when we switch.  To change %cs and jump we
-	 * need this structure to feed to Intel's "lcall" instruction. */
+	 * need this structure to feed to Intel's "lcall" instruction.
+	 */
 	lguest_entry.offset = (long)switch_to_guest + switcher_offset();
 	lguest_entry.segment = LGUEST_CS;
 
-	/* Finally, we need to turn off "Page Global Enable".  PGE is an
+	/*
+	 * Finally, we need to turn off "Page Global Enable".  PGE is an
 	 * optimization where page table entries are specially marked to show
 	 * they never change.  The Host kernel marks all the kernel pages this
 	 * way because it's always present, even when userspace is running.
@@ -534,16 +632,21 @@ void __init lguest_arch_host_init(void)
 	 * you'll get really weird bugs that you'll chase for two days.
 	 *
 	 * I used to turn PGE off every time we switched to the Guest and back
-	 * on when we return, but that slowed the Switcher down noticibly. */
+	 * on when we return, but that slowed the Switcher down noticibly.
+	 */
 
-	/* We don't need the complexity of CPUs coming and going while we're
-	 * doing this. */
+	/*
+	 * We don't need the complexity of CPUs coming and going while we're
+	 * doing this.
+	 */
 	get_online_cpus();
 	if (cpu_has_pge) { /* We have a broader idea of "global". */
 		/* Remember that this was originally set (for cleanup). */
 		cpu_had_pge = 1;
-		/* adjust_pge is a helper function which sets or unsets the PGE
-		 * bit on its CPU, depending on the argument (0 == unset). */
+		/*
+		 * adjust_pge is a helper function which sets or unsets the PGE
+		 * 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_cpu_cap(&boot_cpu_data, X86_FEATURE_PGE);
@@ -590,26 +693,32 @@ int lguest_arch_init_hypercalls(struct lg_cpu *cpu)
 {
 	u32 tsc_speed;
 
-	/* The pointer to the Guest's "struct lguest_data" is the only argument.
-	 * We check that address now. */
+	/*
+	 * The pointer to the Guest's "struct lguest_data" is the only argument.
+	 * We check that address now.
+	 */
 	if (!lguest_address_ok(cpu->lg, cpu->hcall->arg1,
 			       sizeof(*cpu->lg->lguest_data)))
 		return -EFAULT;
 
-	/* Having checked it, we simply set lg->lguest_data to point straight
+	/*
+	 * Having checked it, we simply set lg->lguest_data to point straight
 	 * into the Launcher's memory at the right place and then use
 	 * copy_to_user/from_user from now on, instead of lgread/write.  I put
 	 * this in to show that I'm not immune to writing stupid
-	 * optimizations. */
+	 * optimizations.
+	 */
 	cpu->lg->lguest_data = cpu->lg->mem_base + cpu->hcall->arg1;
 
-	/* We insist that the Time Stamp Counter exist and doesn't change with
+	/*
+	 * We insist that the Time Stamp Counter exist and doesn't change with
 	 * cpu frequency.  Some devious chip manufacturers decided that TSC
 	 * changes could be handled in software.  I decided that time going
 	 * backwards might be good for benchmarks, but it's bad for users.
 	 *
 	 * We also insist that the TSC be stable: the kernel detects unreliable
-	 * TSCs for its own purposes, and we use that here. */
+	 * TSCs for its own purposes, and we use that here.
+	 */
 	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) && !check_tsc_unstable())
 		tsc_speed = tsc_khz;
 	else
@@ -625,38 +734,47 @@ int lguest_arch_init_hypercalls(struct lg_cpu *cpu)
 }
 /*:*/
 
-/*L:030 lguest_arch_setup_regs()
+/*L:030
+ * lguest_arch_setup_regs()
  *
  * Most of the Guest's registers are left alone: we used get_zeroed_page() to
- * allocate the structure, so they will be 0. */
+ * allocate the structure, so they will be 0.
+ */
 void lguest_arch_setup_regs(struct lg_cpu *cpu, unsigned long start)
 {
 	struct lguest_regs *regs = cpu->regs;
 
-	/* There are four "segment" registers which the Guest needs to boot:
+	/*
+	 * There are four "segment" registers which the Guest needs to boot:
 	 * The "code segment" register (cs) refers to the kernel code segment
 	 * __KERNEL_CS, and the "data", "extra" and "stack" segment registers
 	 * refer to the kernel data segment __KERNEL_DS.
 	 *
 	 * The privilege level is packed into the lower bits.  The Guest runs
-	 * at privilege level 1 (GUEST_PL).*/
+	 * at privilege level 1 (GUEST_PL).
+	 */
 	regs->ds = regs->es = regs->ss = __KERNEL_DS|GUEST_PL;
 	regs->cs = __KERNEL_CS|GUEST_PL;
 
-	/* The "eflags" register contains miscellaneous flags.  Bit 1 (0x002)
+	/*
+	 * The "eflags" register contains miscellaneous flags.  Bit 1 (0x002)
 	 * is supposed to always be "1".  Bit 9 (0x200) controls whether
 	 * interrupts are enabled.  We always leave interrupts enabled while
-	 * running the Guest. */
+	 * running the Guest.
+	 */
 	regs->eflags = X86_EFLAGS_IF | 0x2;
 
-	/* The "Extended Instruction Pointer" register says where the Guest is
-	 * running. */
+	/*
+	 * The "Extended Instruction Pointer" register says where the Guest is
+	 * running.
+	 */
 	regs->eip = start;
 
-	/* %esi points to our boot information, at physical address 0, so don't
-	 * touch it. */
+	/*
+	 * %esi points to our boot information, at physical address 0, so don't
+	 * touch it.
+	 */
 
-	/* There are a couple of GDT entries the Guest expects when first
-	 * booting. */
+	/* There are a couple of GDT entries the Guest expects at boot. */
 	setup_guest_gdt(cpu);
 }

drivers/lguest/x86/switcher_32.S

@@ -1,12 +1,15 @@
-/*P:900 This is the Switcher: code which sits at 0xFFC00000 astride both the
- * Host and Guest to do the low-level Guest<->Host switch.  It is as simple as
- * it can be made, but it's naturally very specific to x86.
+/*P:900
+ * This is the Switcher: code which sits at 0xFFC00000 (or 0xFFE00000) astride
+ * both the Host and Guest to do the low-level Guest<->Host switch.  It is as
+ * simple as it can be made, but it's naturally very specific to x86.
  *
  * You have now completed Preparation.  If this has whet your appetite; if you
  * are feeling invigorated and refreshed then the next, more challenging stage
- * can be found in "make Guest". :*/
+ * can be found in "make Guest".
+ :*/
 
-/*M:012 Lguest is meant to be simple: my rule of thumb is that 1% more LOC must
+/*M:012
+ * Lguest is meant to be simple: my rule of thumb is that 1% more LOC must
  * gain at least 1% more performance.  Since neither LOC nor performance can be
  * measured beforehand, it generally means implementing a feature then deciding
  * if it's worth it.  And once it's implemented, who can say no?
@@ -31,11 +34,14 @@
  * Host (which is actually really easy).
  *
  * Two questions remain.  Would the performance gain outweigh the complexity?
- * And who would write the verse documenting it? :*/
+ * And who would write the verse documenting it?
+:*/
 
-/*M:011 Lguest64 handles NMI.  This gave me NMI envy (until I looked at their
+/*M:011
+ * Lguest64 handles NMI.  This gave me NMI envy (until I looked at their
  * code).  It's worth doing though, since it would let us use oprofile in the
- * Host when a Guest is running. :*/
+ * Host when a Guest is running.
+:*/
 
 /*S:100
  * Welcome to the Switcher itself!

drivers/virtio/virtio_pci.c

@@ -52,8 +52,10 @@ struct virtio_pci_device
 	char (*msix_names)[256];
 	/* Number of available vectors */
 	unsigned msix_vectors;
-	/* Vectors allocated */
+	/* Vectors allocated, excluding per-vq vectors if any */
 	unsigned msix_used_vectors;
+	/* Whether we have vector per vq */
+	bool per_vq_vectors;
 };
 
 /* Constants for MSI-X */
@@ -258,7 +260,6 @@ static void vp_free_vectors(struct virtio_device *vdev)
 
 	for (i = 0; i < vp_dev->msix_used_vectors; ++i)
 		free_irq(vp_dev->msix_entries[i].vector, vp_dev);
-	vp_dev->msix_used_vectors = 0;
 
 	if (vp_dev->msix_enabled) {
 		/* Disable the vector used for configuration */
@@ -267,80 +268,77 @@ static void vp_free_vectors(struct virtio_device *vdev)
 		/* Flush the write out to device */
 		ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
 
-		vp_dev->msix_enabled = 0;
 		pci_disable_msix(vp_dev->pci_dev);
+		vp_dev->msix_enabled = 0;
+		vp_dev->msix_vectors = 0;
 	}
-}
 
-static int vp_enable_msix(struct pci_dev *dev, struct msix_entry *entries,
-			  int *options, int noptions)
-{
-	int i;
-	for (i = 0; i < noptions; ++i)
-		if (!pci_enable_msix(dev, entries, options[i]))
-			return options[i];
-	return -EBUSY;
+	vp_dev->msix_used_vectors = 0;
+	kfree(vp_dev->msix_names);
+	vp_dev->msix_names = NULL;
+	kfree(vp_dev->msix_entries);
+	vp_dev->msix_entries = NULL;
 }
 
-static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs)
+static int vp_request_vectors(struct virtio_device *vdev, int nvectors,
+			      bool per_vq_vectors)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	const char *name = dev_name(&vp_dev->vdev.dev);
 	unsigned i, v;
 	int err = -ENOMEM;
-	/* We want at most one vector per queue and one for config changes.
-	 * Fallback to separate vectors for config and a shared for queues.
-	 * Finally fall back to regular interrupts. */
-	int options[] = { max_vqs + 1, 2 };
-	int nvectors = max(options[0], options[1]);
+
+	if (!nvectors) {
+		/* Can't allocate MSI-X vectors, use regular interrupt */
+		vp_dev->msix_vectors = 0;
+		err = request_irq(vp_dev->pci_dev->irq, vp_interrupt,
+				  IRQF_SHARED, name, vp_dev);
+		if (err)
+			return err;
+		vp_dev->intx_enabled = 1;
+		return 0;
+	}
 
 	vp_dev->msix_entries = kmalloc(nvectors * sizeof *vp_dev->msix_entries,
 				       GFP_KERNEL);
 	if (!vp_dev->msix_entries)
-		goto error_entries;
+		goto error;
 	vp_dev->msix_names = kmalloc(nvectors * sizeof *vp_dev->msix_names,
 				     GFP_KERNEL);
 	if (!vp_dev->msix_names)
-		goto error_names;
+		goto error;
 
 	for (i = 0; i < nvectors; ++i)
 		vp_dev->msix_entries[i].entry = i;
 
-	err = vp_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries,
-			     options, ARRAY_SIZE(options));
-	if (err < 0) {
-		/* Can't allocate enough MSI-X vectors, use regular interrupt */
-		vp_dev->msix_vectors = 0;
-		err = request_irq(vp_dev->pci_dev->irq, vp_interrupt,
-				  IRQF_SHARED, name, vp_dev);
-		if (err)
-			goto error_irq;
-		vp_dev->intx_enabled = 1;
-	} else {
-		vp_dev->msix_vectors = err;
-		vp_dev->msix_enabled = 1;
-
-		/* Set the vector used for configuration */
-		v = vp_dev->msix_used_vectors;
-		snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
-			 "%s-config", name);
-		err = request_irq(vp_dev->msix_entries[v].vector,
-				  vp_config_changed, 0, vp_dev->msix_names[v],
-				  vp_dev);
-		if (err)
-			goto error_irq;
-		++vp_dev->msix_used_vectors;
-
-		iowrite16(v, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
-		/* Verify we had enough resources to assign the vector */
-		v = ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
-		if (v == VIRTIO_MSI_NO_VECTOR) {
-			err = -EBUSY;
-			goto error_irq;
-		}
+	err = pci_enable_msix(vp_dev->pci_dev, vp_dev->msix_entries, nvectors);
+	if (err > 0)
+		err = -ENOSPC;
+	if (err)
+		goto error;
+	vp_dev->msix_vectors = nvectors;
+	vp_dev->msix_enabled = 1;
+
+	/* Set the vector used for configuration */
+	v = vp_dev->msix_used_vectors;
+	snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
+		 "%s-config", name);
+	err = request_irq(vp_dev->msix_entries[v].vector,
+			  vp_config_changed, 0, vp_dev->msix_names[v],
+			  vp_dev);
+	if (err)
+		goto error;
+	++vp_dev->msix_used_vectors;
+
+	iowrite16(v, vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
+	/* Verify we had enough resources to assign the vector */
+	v = ioread16(vp_dev->ioaddr + VIRTIO_MSI_CONFIG_VECTOR);
+	if (v == VIRTIO_MSI_NO_VECTOR) {
+		err = -EBUSY;
+		goto error;
 	}
 
-	if (vp_dev->msix_vectors && vp_dev->msix_vectors != max_vqs + 1) {
+	if (!per_vq_vectors) {
 		/* Shared vector for all VQs */
 		v = vp_dev->msix_used_vectors;
 		snprintf(vp_dev->msix_names[v], sizeof *vp_dev->msix_names,
@@ -349,28 +347,25 @@ static int vp_request_vectors(struct virtio_device *vdev, unsigned max_vqs)
 				  vp_vring_interrupt, 0, vp_dev->msix_names[v],
 				  vp_dev);
 		if (err)
-			goto error_irq;
+			goto error;
 		++vp_dev->msix_used_vectors;
 	}
 	return 0;
-error_irq:
+error:
 	vp_free_vectors(vdev);
-	kfree(vp_dev->msix_names);
-error_names:
-	kfree(vp_dev->msix_entries);
-error_entries:
 	return err;
 }
 
 static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 				    void (*callback)(struct virtqueue *vq),
-				    const char *name)
+				    const char *name,
+				    u16 vector)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	struct virtio_pci_vq_info *info;
 	struct virtqueue *vq;
 	unsigned long flags, size;
-	u16 num, vector;
+	u16 num;
 	int err;
 
 	/* Select the queue we're interested in */
@@ -389,7 +384,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 
 	info->queue_index = index;
 	info->num = num;
-	info->vector = VIRTIO_MSI_NO_VECTOR;
+	info->vector = vector;
 
 	size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));
 	info->queue = alloc_pages_exact(size, GFP_KERNEL|__GFP_ZERO);
@@ -413,22 +408,7 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 	vq->priv = info;
 	info->vq = vq;
 
-	/* allocate per-vq vector if available and necessary */
-	if (callback && vp_dev->msix_used_vectors < vp_dev->msix_vectors) {
-		vector = vp_dev->msix_used_vectors;
-		snprintf(vp_dev->msix_names[vector], sizeof *vp_dev->msix_names,
-			 "%s-%s", dev_name(&vp_dev->vdev.dev), name);
-		err = request_irq(vp_dev->msix_entries[vector].vector,
-				  vring_interrupt, 0,
-				  vp_dev->msix_names[vector], vq);
-		if (err)
-			goto out_request_irq;
-		info->vector = vector;
-		++vp_dev->msix_used_vectors;
-	} else
-		vector = VP_MSIX_VQ_VECTOR;
-
-	 if (callback && vp_dev->msix_enabled) {
+	 if (vector != VIRTIO_MSI_NO_VECTOR) {
 		iowrite16(vector, vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
 		vector = ioread16(vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
 		if (vector == VIRTIO_MSI_NO_VECTOR) {
@@ -444,11 +424,6 @@ static struct virtqueue *vp_find_vq(struct virtio_device *vdev, unsigned index,
 	return vq;
 
 out_assign:
-	if (info->vector != VIRTIO_MSI_NO_VECTOR) {
-		free_irq(vp_dev->msix_entries[info->vector].vector, vq);
-		--vp_dev->msix_used_vectors;
-	}
-out_request_irq:
 	vring_del_virtqueue(vq);
 out_activate_queue:
 	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
@@ -462,12 +437,13 @@ static void vp_del_vq(struct virtqueue *vq)
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
 	struct virtio_pci_vq_info *info = vq->priv;
-	unsigned long size;
+	unsigned long flags, size;
 
-	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
+	spin_lock_irqsave(&vp_dev->lock, flags);
+	list_del(&info->node);
+	spin_unlock_irqrestore(&vp_dev->lock, flags);
 
-	if (info->vector != VIRTIO_MSI_NO_VECTOR)
-		free_irq(vp_dev->msix_entries[info->vector].vector, vq);
+	iowrite16(info->queue_index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
 
 	if (vp_dev->msix_enabled) {
 		iowrite16(VIRTIO_MSI_NO_VECTOR,
@@ -489,36 +465,62 @@ static void vp_del_vq(struct virtqueue *vq)
 /* the config->del_vqs() implementation */
 static void vp_del_vqs(struct virtio_device *vdev)
 {
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	struct virtqueue *vq, *n;
+	struct virtio_pci_vq_info *info;
 
-	list_for_each_entry_safe(vq, n, &vdev->vqs, list)
+	list_for_each_entry_safe(vq, n, &vdev->vqs, list) {
+		info = vq->priv;
+		if (vp_dev->per_vq_vectors)
+			free_irq(vp_dev->msix_entries[info->vector].vector, vq);
 		vp_del_vq(vq);
+	}
+	vp_dev->per_vq_vectors = false;
 
 	vp_free_vectors(vdev);
 }
 
-/* the config->find_vqs() implementation */
-static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
-		       struct virtqueue *vqs[],
-		       vq_callback_t *callbacks[],
-		       const char *names[])
+static int vp_try_to_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+			      struct virtqueue *vqs[],
+			      vq_callback_t *callbacks[],
+			      const char *names[],
+			      int nvectors,
+			      bool per_vq_vectors)
 {
-	int vectors = 0;
-	int i, err;
-
-	/* How many vectors would we like? */
-	for (i = 0; i < nvqs; ++i)
-		if (callbacks[i])
-			++vectors;
+	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
+	u16 vector;
+	int i, err, allocated_vectors;
 
-	err = vp_request_vectors(vdev, vectors);
+	err = vp_request_vectors(vdev, nvectors, per_vq_vectors);
 	if (err)
 		goto error_request;
 
+	vp_dev->per_vq_vectors = per_vq_vectors;
+	allocated_vectors = vp_dev->msix_used_vectors;
 	for (i = 0; i < nvqs; ++i) {
-		vqs[i] = vp_find_vq(vdev, i, callbacks[i], names[i]);
-		if (IS_ERR(vqs[i]))
+		if (!callbacks[i] || !vp_dev->msix_enabled)
+			vector = VIRTIO_MSI_NO_VECTOR;
+		else if (vp_dev->per_vq_vectors)
+			vector = allocated_vectors++;
+		else
+			vector = VP_MSIX_VQ_VECTOR;
+		vqs[i] = vp_find_vq(vdev, i, callbacks[i], names[i], vector);
+		if (IS_ERR(vqs[i])) {
+			err = PTR_ERR(vqs[i]);
 			goto error_find;
+		}
+		/* allocate per-vq irq if available and necessary */
+		if (vp_dev->per_vq_vectors && vector != VIRTIO_MSI_NO_VECTOR) {
+			snprintf(vp_dev->msix_names[vector], sizeof *vp_dev->msix_names,
+				 "%s-%s", dev_name(&vp_dev->vdev.dev), names[i]);
+			err = request_irq(vp_dev->msix_entries[vector].vector,
+					  vring_interrupt, 0,
+					  vp_dev->msix_names[vector], vqs[i]);
+			if (err) {
+				vp_del_vq(vqs[i]);
+				goto error_find;
+			}
+		}
 	}
 	return 0;
 
@@ -526,7 +528,37 @@ error_find:
 	vp_del_vqs(vdev);
 
 error_request:
-	return PTR_ERR(vqs[i]);
+	return err;
+}
+
+/* the config->find_vqs() implementation */
+static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,
+		       struct virtqueue *vqs[],
+		       vq_callback_t *callbacks[],
+		       const char *names[])
+{
+	int vectors = 0;
+	int i, uninitialized_var(err);
+
+	/* How many vectors would we like? */
+	for (i = 0; i < nvqs; ++i)
+		if (callbacks[i])
+			++vectors;
+
+	/* We want at most one vector per queue and one for config changes. */
+	err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names,
+				 vectors + 1, true);
+	if (!err)
+		return 0;
+	/* Fallback to separate vectors for config and a shared for queues. */
+	err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names,
+				 2, false);
+	if (!err)
+		return 0;
+	/* Finally fall back to regular interrupts. */
+	err = vp_try_to_find_vqs(vdev, nvqs, vqs, callbacks, names,
+				 0, false);
+	return err;
 }
 
 static struct virtio_config_ops virtio_pci_config_ops = {

include/linux/lguest.h

@@ -1,5 +1,7 @@
-/* Things the lguest guest needs to know.  Note: like all lguest interfaces,
- * this is subject to wild and random change between versions. */
+/*
+ * Things the lguest guest needs to know.  Note: like all lguest interfaces,
+ * this is subject to wild and random change between versions.
+ */
 #ifndef _LINUX_LGUEST_H
 #define _LINUX_LGUEST_H
 
@@ -11,32 +13,41 @@
 #define LG_CLOCK_MIN_DELTA	100UL
 #define LG_CLOCK_MAX_DELTA	ULONG_MAX
 
-/*G:031 The second method of communicating with the Host is to via "struct
+/*G:031
+ * The second method of communicating with the Host is to via "struct
  * lguest_data".  Once the Guest's initialization hypercall tells the Host where
- * this is, the Guest and Host both publish information in it. :*/
-struct lguest_data
-{
-	/* 512 == enabled (same as eflags in normal hardware).  The Guest
-	 * changes interrupts so often that a hypercall is too slow. */
+ * this is, the Guest and Host both publish information in it.
+:*/
+struct lguest_data {
+	/*
+	 * 512 == enabled (same as eflags in normal hardware).  The Guest
+	 * changes interrupts so often that a hypercall is too slow.
+	 */
 	unsigned int irq_enabled;
 	/* Fine-grained interrupt disabling by the Guest */
 	DECLARE_BITMAP(blocked_interrupts, LGUEST_IRQS);
 
-	/* The Host writes the virtual address of the last page fault here,
+	/*
+	 * The Host writes the virtual address of the last page fault here,
 	 * which saves the Guest a hypercall.  CR2 is the native register where
-	 * this address would normally be found. */
+	 * this address would normally be found.
+	 */
 	unsigned long cr2;
 
 	/* Wallclock time set by the Host. */
 	struct timespec time;
 
-	/* Interrupt pending set by the Host.  The Guest should do a hypercall
-	 * if it re-enables interrupts and sees this set (to X86_EFLAGS_IF). */
+	/*
+	 * Interrupt pending set by the Host.  The Guest should do a hypercall
+	 * if it re-enables interrupts and sees this set (to X86_EFLAGS_IF).
+	 */
 	int irq_pending;
 
-	/* Async hypercall ring.  Instead of directly making hypercalls, we can
+	/*
+	 * Async hypercall ring.  Instead of directly making hypercalls, we can
 	 * place them in here for processing the next time the Host wants.
-	 * This batching can be quite efficient. */
+	 * This batching can be quite efficient.
+	 */
 
 	/* 0xFF == done (set by Host), 0 == pending (set by Guest). */
 	u8 hcall_status[LHCALL_RING_SIZE];

include/linux/lguest_launcher.h

@@ -29,8 +29,10 @@ struct lguest_device_desc {
 	__u8 type;
 	/* The number of virtqueues (first in config array) */
 	__u8 num_vq;
-	/* The number of bytes of feature bits.  Multiply by 2: one for host
-	 * features and one for Guest acknowledgements. */
+	/*
+	 * The number of bytes of feature bits.  Multiply by 2: one for host
+	 * features and one for Guest acknowledgements.
+	 */
 	__u8 feature_len;
 	/* The number of bytes of the config array after virtqueues. */
 	__u8 config_len;
@@ -39,8 +41,10 @@ struct lguest_device_desc {
 	__u8 config[0];
 };
 
-/*D:135 This is how we expect the device configuration field for a virtqueue
- * to be laid out in config space. */
+/*D:135
+ * This is how we expect the device configuration field for a virtqueue
+ * to be laid out in config space.
+ */
 struct lguest_vqconfig {
 	/* The number of entries in the virtio_ring */
 	__u16 num;
@@ -61,7 +65,9 @@ enum lguest_req
 	LHREQ_EVENTFD, /* + address, fd. */
 };
 
-/* The alignment to use between consumer and producer parts of vring.
- * x86 pagesize for historical reasons. */
+/*
+ * The alignment to use between consumer and producer parts of vring.
+ * x86 pagesize for historical reasons.
+ */
 #define LGUEST_VRING_ALIGN	4096
 #endif /* _LINUX_LGUEST_LAUNCHER */

include/linux/virtio_blk.h

@@ -20,8 +20,7 @@
 
 #define VIRTIO_BLK_ID_BYTES	(sizeof(__u16[256]))	/* IDENTIFY DATA */
 
-struct virtio_blk_config
-{
+struct virtio_blk_config {
 	/* The capacity (in 512-byte sectors). */
 	__u64 capacity;
 	/* The maximum segment size (if VIRTIO_BLK_F_SIZE_MAX) */
@@ -50,8 +49,7 @@ struct virtio_blk_config
 #define VIRTIO_BLK_T_BARRIER	0x80000000
 
 /* This is the first element of the read scatter-gather list. */
-struct virtio_blk_outhdr
-{
+struct virtio_blk_outhdr {
 	/* VIRTIO_BLK_T* */
 	__u32 type;
 	/* io priority. */

include/linux/virtio_config.h

@@ -79,8 +79,7 @@
  *	the dev->feature bits if it wants.
  */
 typedef void vq_callback_t(struct virtqueue *);
-struct virtio_config_ops
-{
+struct virtio_config_ops {
 	void (*get)(struct virtio_device *vdev, unsigned offset,
 		    void *buf, unsigned len);
 	void (*set)(struct virtio_device *vdev, unsigned offset,

include/linux/virtio_net.h

@@ -31,8 +31,7 @@
 
 #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
 
-struct virtio_net_config
-{
+struct virtio_net_config {
 	/* The config defining mac address (if VIRTIO_NET_F_MAC) */
 	__u8 mac[6];
 	/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */
@@ -41,8 +40,7 @@ struct virtio_net_config
 
 /* This is the first element of the scatter-gather list.  If you don't
  * specify GSO or CSUM features, you can simply ignore the header. */
-struct virtio_net_hdr
-{
+struct virtio_net_hdr {
 #define VIRTIO_NET_HDR_F_NEEDS_CSUM	1	// Use csum_start, csum_offset
 	__u8 flags;
 #define VIRTIO_NET_HDR_GSO_NONE		0	// Not a GSO frame

include/linux/virtio_ring.h

@@ -30,8 +30,7 @@
 #define VIRTIO_RING_F_INDIRECT_DESC	28
 
 /* Virtio ring descriptors: 16 bytes.  These can chain together via "next". */
-struct vring_desc
-{
+struct vring_desc {
 	/* Address (guest-physical). */
 	__u64 addr;
 	/* Length. */
@@ -42,24 +41,21 @@ struct vring_desc
 	__u16 next;
 };
 
-struct vring_avail
-{
+struct vring_avail {
 	__u16 flags;
 	__u16 idx;
 	__u16 ring[];
 };
 
 /* u32 is used here for ids for padding reasons. */
-struct vring_used_elem
-{
+struct vring_used_elem {
 	/* Index of start of used descriptor chain. */
 	__u32 id;
 	/* Total length of the descriptor chain which was used (written to) */
 	__u32 len;
 };
 
-struct vring_used
-{
+struct vring_used {
 	__u16 flags;
 	__u16 idx;
 	struct vring_used_elem ring[];