lguest: documentation V: Host

Documentation: The Host

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

6 files changed, 924 insertions, 85 deletions

diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c
index 1eb05f9a56b..c0f50b4dd2f 100644
--- a/drivers/lguest/core.c
+++ b/drivers/lguest/core.c
@@ -64,11 +64,33 @@ static struct lguest_pages *lguest_pages(unsigned int cpu)
 		  (SWITCHER_ADDR + SHARED_SWITCHER_PAGES*PAGE_SIZE))[cpu]);
 }
 
+/*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.
+ *
+ * The Switcher code must be at the same virtual address in the Guest as the
+ * 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.  We also set up the per-cpu parts of the
+ * Switcher here.
+ */
 static __init int map_switcher(void)
 {
 	int i, err;
 	struct page **pagep;
 
+	/*
+	 * Map the Switcher in to high memory.
+	 *
+	 * It turns out that if we choose the address 0xFFC00000 (4MB under the
+	 * top virtual address), it makes setting up the page tables really
+	 * easy.
+	 */
+
+	/* We allocate an array of "struct page"s.  map_vm_area() wants the
+	 * pages in this form, rather than just an array of pointers. */
 	switcher_page = kmalloc(sizeof(switcher_page[0])*TOTAL_SWITCHER_PAGES,
 				GFP_KERNEL);
 	if (!switcher_page) {
@@ -76,6 +98,8 @@ 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. */
 	for (i = 0; i < TOTAL_SWITCHER_PAGES; i++) {
 		unsigned long addr = get_zeroed_page(GFP_KERNEL);
 		if (!addr) {
@@ -85,6 +109,9 @@ static __init int map_switcher(void)
 		switcher_page[i] = virt_to_page(addr);
 	}
 
+	/* 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. */
 	switcher_vma = __get_vm_area(TOTAL_SWITCHER_PAGES * PAGE_SIZE,
 				       VM_ALLOC, SWITCHER_ADDR, VMALLOC_END);
 	if (!switcher_vma) {
@@ -93,49 +120,105 @@ static __init int map_switcher(void)
 		goto free_pages;
 	}
 
+	/* 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. */
 	pagep = switcher_page;
 	err = map_vm_area(switcher_vma, PAGE_KERNEL, &pagep);
 	if (err) {
 		printk("lguest: map_vm_area failed: %i\n", err);
 		goto free_vma;
 	}
+
+	/* Now the switcher is mapped at the right address, we can't fail!
+	 * Copy in the compiled-in Switcher code (from switcher.S). */
 	memcpy(switcher_vma->addr, start_switcher_text,
 	       end_switcher_text - start_switcher_text);
 
-	/* Fix up IDT entries to point into copied text. */
+	/* Most of the 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.
+	 *
+	 * The only exception is the interrupt handlers in switcher.S: their
+	 * 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 builtin
+	 * switcher code and the high-mapped copy we just made. */
 	for (i = 0; i < IDT_ENTRIES; i++)
 		default_idt_entries[i] += switcher_offset();
 
+	/*
+	 * Set up the Switcher's per-cpu areas.
+	 *
+	 * Each CPU gets two pages of its own within the high-mapped region
+	 * (aka. "struct lguest_pages").  Much of this can be initialized now,
+	 * but some depends on what Guest we are running (which is set up in
+	 * copy_in_guest_info()).
+	 */
 	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. */
 		struct lguest_ro_state *state = &pages->state;
 
-		/* These fields are static: rest done in copy_in_guest_info */
+		/* 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"). */
 		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
+		 * Table, so we just use store_idt(), which gets this CPU's IDT
+		 * descriptor. */
 		store_idt(&state->host_idt_desc);
+
+		/* 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. */
 		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
+		 * the switcher: in pages->regs.  The stack grows upwards, so
+		 * we start it at the end of that structure. */
 		state->guest_tss.esp0 = (long)(&pages->regs + 1);
+		/* 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;
-		/* No I/O for you! */
+
+		/* 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". */
 		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. */
 		setup_default_gdt_entries(state);
+		/* Most IDT entries are the same for all Guests, too.*/
 		setup_default_idt_entries(state, default_idt_entries);
 
-		/* Setup LGUEST segments on all cpus */
+		/* 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;
 	}
 
-	/* Initialize entry point into switcher. */
+	/* 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. */
 	lguest_entry.offset = (long)switch_to_guest + switcher_offset();
 	lguest_entry.segment = LGUEST_CS;
 
 	printk(KERN_INFO "lguest: mapped switcher at %p\n",
 	       switcher_vma->addr);
+	/* And we succeeded... */
 	return 0;
 
 free_vma:
@@ -149,35 +232,58 @@ free_some_pages:
 out:
 	return err;
 }
+/*:*/
 
+/* Cleaning up the mapping when the module is unloaded is almost...
+ * too easy. */
 static void unmap_switcher(void)
 {
 	unsigned int i;
 
+	/* vunmap() undoes *both* map_vm_area() and __get_vm_area(). */
 	vunmap(switcher_vma->addr);
+	/* Now we just need to free the pages we copied the switcher into */
 	for (i = 0; i < TOTAL_SWITCHER_PAGES; i++)
 		__free_pages(switcher_page[i], 0);
 }
 
-/* IN/OUT insns: enough to get us past boot-time probing. */
+/*H:130 Our Guest is usually so well behaved; it never tries to do things it
+ * isn't allowed to.  Unfortunately, "struct paravirt_ops" isn't quite
+ * complete, because it doesn't contain replacements for the Intel I/O
+ * instructions.  As a result, the Guest sometimes fumbles across one during
+ * the boot process as it probes for various things which are usually attached
+ * to a PC.
+ *
+ * When the Guest uses one of these instructions, we get trap #13 (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. */
 static int emulate_insn(struct lguest *lg)
 {
 	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. */
 	unsigned long physaddr = guest_pa(lg, lg->regs->eip);
 
-	/* This only works for addresses in linear mapping... */
+	/* The guest_pa() function only works for Guest kernel addresses, but
+	 * that's all we're trying to do anyway. */
 	if (lg->regs->eip < lg->page_offset)
 		return 0;
+
+	/* Decoding x86 instructions is icky. */
 	lgread(lg, &insn, physaddr, 1);
 
-	/* Operand size prefix means it's actually for ax. */
+	/* 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. */
 		insnlen = 1;
 		lgread(lg, &insn, physaddr + insnlen, 1);
 	}
 
+	/* 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;
@@ -194,9 +300,13 @@ static int emulate_insn(struct lguest *lg)
 		insnlen += 1;
 		break;
 	default:
+		/* OK, we don't know what this is, can't emulate. */
 		return 0;
 	}
 
+	/* 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". */
 	if (in) {
 		/* Lower bit tells is whether it's a 16 or 32 bit access */
 		if (insn & 0x1)
@@ -204,9 +314,12 @@ static int emulate_insn(struct lguest *lg)
 		else
 			lg->regs->eax |= (0xFFFF << shift);
 	}
+	/* Finally, we've "done" the instruction, so move past it. */
 	lg->regs->eip += insnlen;
+	/* Success! */
 	return 1;
 }
+/*:*/
 
 /*L:305
  * Dealing With Guest Memory.
@@ -321,13 +434,24 @@ static void run_guest_once(struct lguest *lg, struct lguest_pages *pages)
 		     : "memory", "%edx", "%ecx", "%edi", "%esi");
 }
 
+/*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. */
 int run_guest(struct lguest *lg, unsigned long __user *user)
 {
+	/* We stop running once the Guest is dead. */
 	while (!lg->dead) {
+		/* We need to initialize this, otherwise gcc complains.  It's
+		 * not (yet) clever enough to see that it's initialized when we
+		 * need it. */
 		unsigned int cr2 = 0; /* Damn gcc */
 
-		/* Hypercalls first: we might have been out to userspace */
+		/* First we run any hypercalls the Guest wants done: either in
+		 * the hypercall ring in "struct lguest_data", or directly by
+		 * using int 31 (LGUEST_TRAP_ENTRY). */
 		do_hypercalls(lg);
+		/* It's possible the Guest did a SEND_DMA hypercall to the
+		 * Launcher, in which case we return from the read() now. */
 		if (lg->dma_is_pending) {
 			if (put_user(lg->pending_dma, user) ||
 			    put_user(lg->pending_key, user+1))
@@ -335,6 +459,7 @@ int run_guest(struct lguest *lg, unsigned long __user *user)
 			return sizeof(unsigned long)*2;
 		}
 
+		/* Check for signals */
 		if (signal_pending(current))
 			return -ERESTARTSYS;
 
@@ -342,77 +467,154 @@ int run_guest(struct lguest *lg, unsigned long __user *user)
 		if (lg->break_out)
 			return -EAGAIN;
 
+		/* 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. */
 		maybe_do_interrupt(lg);
 
+		/* 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. */
 		try_to_freeze();
 
+		/* Just make absolutely sure the Guest is still alive.  One of
+		 * those hypercalls could have been fatal, for example. */
 		if (lg->dead)
 			break;
 
+		/* If the Guest asked to be stopped, we sleep.  The Guest's
+		 * clock timer or LHCALL_BREAK from the Waker will wake us. */
 		if (lg->halted) {
 			set_current_state(TASK_INTERRUPTIBLE);
 			schedule();
 			continue;
 		}
 
+		/* OK, now we're ready to jump into the Guest.  First we put up
+		 * the "Do Not Disturb" sign: */
 		local_irq_disable();
 
-		/* Even if *we* don't want FPU trap, guest might... */
+		/* 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. */
 		if (lg->ts)
 			set_ts();
 
-		/* Don't let Guest do SYSENTER: we can't handle it. */
+		/* 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. */
 		if (boot_cpu_has(X86_FEATURE_SEP))
 			wrmsr(MSR_IA32_SYSENTER_CS, 0, 0);
 
+		/* Now we actually run the Guest.  It will pop back out when
+		 * something interesting happens, and we can examine its
+		 * registers to see what it was doing. */
 		run_guest_once(lg, lguest_pages(raw_smp_processor_id()));
 
-		/* Save cr2 now if we page-faulted. */
+		/* The "regs" pointer 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.  */
+
+		/* 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. */
 		if (lg->regs->trapnum == 14)
 			cr2 = read_cr2();
+		/* Similarly, if we took a trap because the Guest used the FPU,
+		 * we have to restore the FPU it expects to see. */
 		else if (lg->regs->trapnum == 7)
 			math_state_restore();
 
+		/* Restore SYSENTER if it's supposed to be on. */
 		if (boot_cpu_has(X86_FEATURE_SEP))
 			wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
+
+		/* Now we're ready to be interrupted or moved to other CPUs */
 		local_irq_enable();
 
+		/* OK, so what happened? */
 		switch (lg->regs->trapnum) {
 		case 13: /* We've intercepted a GPF. */
+			/* 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. */
 			if (lg->regs->errcode == 0) {
 				if (emulate_insn(lg))
 					continue;
 			}
 			break;
 		case 14: /* We've intercepted a page fault. */
+			/* 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. */
 			if (demand_page(lg, cr2, lg->regs->errcode))
 				continue;
 
-			/* If lguest_data is NULL, this won't hurt. */
+			/* 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 INITIALIZE
+			 * hypercall, so lg->lguest_data will be NULL, so
+			 * &lg->lguest_data->cr2 will be address 8.  Writing
+			 * into that address won't hurt the Host at all,
+			 * though. */
 			if (put_user(cr2, &lg->lguest_data->cr2))
 				kill_guest(lg, "Writing cr2");
 			break;
 		case 7: /* We've intercepted a Device Not Available fault. */
-			/* If they don't want to know, just absorb it. */
+			/* If the Guest doesn't want to know, we already
+			 * restored the Floating Point Unit, so we just
+			 * continue without telling it. */
 			if (!lg->ts)
 				continue;
 			break;
-		case 32 ... 255: /* Real interrupt, fall thru */
+		case 32 ... 255:
+			/* 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 fall through to loop
+			 * around: */
 			cond_resched();
 		case LGUEST_TRAP_ENTRY: /* Handled at top of loop */
 			continue;
 		}
 
+		/* If we get here, it's a trap the Guest wants to know
+		 * about. */
 		if (deliver_trap(lg, lg->regs->trapnum))
 			continue;
 
+		/* 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 a cryptic error message. */
 		kill_guest(lg, "unhandled trap %li at %#lx (%#lx)",
 			   lg->regs->trapnum, lg->regs->eip,
 			   lg->regs->trapnum == 14 ? cr2 : lg->regs->errcode);
 	}
+	/* The Guest is dead => "No such file or directory" */
 	return -ENOENT;
 }
 
+/* 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. :*/
+
 int find_free_guest(void)
 {
 	unsigned int i;
@@ -430,55 +632,96 @@ static void adjust_pge(void *on)
 		write_cr4(read_cr4() & ~X86_CR4_PGE);
 }
 
+/*H:000
+ * Welcome to the Host!
+ *
+ * By this point your brain has been tickled by the Guest code and numbed by
+ * the Launcher code; prepare for it to be stretched by the Host code.  This is
+ * the heart.  Let's begin at the initialization routine for the Host's lg
+ * module.
+ */
 static int __init init(void)
 {
 	int err;
 
+	/* Lguest can't run under Xen, VMI or itself.  It does Tricky Stuff. */
 	if (paravirt_enabled()) {
 		printk("lguest is afraid of %s\n", paravirt_ops.name);
 		return -EPERM;
 	}
 
+	/* First we put the Switcher up in very high virtual memory. */
 	err = map_switcher();
 	if (err)
 		return err;
 
+	/* Now we set up the pagetable implementation for the Guests. */
 	err = init_pagetables(switcher_page, SHARED_SWITCHER_PAGES);
 	if (err) {
 		unmap_switcher();
 		return err;
 	}
+
+	/* The I/O subsystem needs some things initialized. */
 	lguest_io_init();
 
+	/* /dev/lguest needs to be registered. */
 	err = lguest_device_init();
 	if (err) {
 		free_pagetables();
 		unmap_switcher();
 		return err;
 	}
+
+	/* 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.
+	 *
+	 * Lguest breaks this: unbeknownst to the rest of the Host kernel, we
+	 * switch to the Guest kernel.  If you don't disable this on all CPUs,
+	 * 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. */
+
+	/* We don't need the complexity of CPUs coming and going while we're
+	 * doing this. */
 	lock_cpu_hotplug();
 	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). */
 		on_each_cpu(adjust_pge, (void *)0, 0, 1);
+		/* Turn off the feature in the global feature set. */
 		clear_bit(X86_FEATURE_PGE, boot_cpu_data.x86_capability);
 	}
 	unlock_cpu_hotplug();
+
+	/* All good! */
 	return 0;
 }
 
+/* Cleaning up is just the same code, backwards.  With a little French. */
 static void __exit fini(void)
 {
 	lguest_device_remove();
 	free_pagetables();
 	unmap_switcher();
+
+	/* If we had PGE before we started, turn it back on now. */
 	lock_cpu_hotplug();
 	if (cpu_had_pge) {
 		set_bit(X86_FEATURE_PGE, boot_cpu_data.x86_capability);
+		/* adjust_pge's argument "1" means set PGE. */
 		on_each_cpu(adjust_pge, (void *)1, 0, 1);
 	}
 	unlock_cpu_hotplug();
 }
 
+/* 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");
diff --git a/drivers/lguest/hypercalls.c b/drivers/lguest/hypercalls.c
index fb546b04644..7a5299f9679 100644
--- a/drivers/lguest/hypercalls.c
+++ b/drivers/lguest/hypercalls.c
@@ -28,37 +28,63 @@
 #include <irq_vectors.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_CRASH, both.
+ *
+ * Remember from the Guest: %eax == which call to make, and the arguments are
+ * packed into %edx, %ebx and %ecx if needed. */
 static void do_hcall(struct lguest *lg, struct lguest_regs *regs)
 {
 	switch (regs->eax) {
 	case LHCALL_FLUSH_ASYNC:
+		/* This call does nothing, except by breaking out of the Guest
+		 * it makes us process all the asynchronous hypercalls. */
 		break;
 	case LHCALL_LGUEST_INIT:
+		/* You can't get here unless you're already initialized.  Don't
+		 * do that. */
 		kill_guest(lg, "already have lguest_data");
 		break;
 	case LHCALL_CRASH: {
+		/* Crash 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. */
 		lgread(lg, msg, regs->edx, sizeof(msg));
 		msg[sizeof(msg)-1] = '\0';
 		kill_guest(lg, "CRASH: %s", msg);
 		break;
 	}
 	case LHCALL_FLUSH_TLB:
+		/* FLUSH_TLB comes in two flavors, depending on the
+		 * argument: */
 		if (regs->edx)
 			guest_pagetable_clear_all(lg);
 		else
 			guest_pagetable_flush_user(lg);
 		break;
 	case LHCALL_GET_WALLCLOCK: {
+		/* The Guest wants to know the real time in seconds since 1970,
+		 * in good Unix tradition. */
 		struct timespec ts;
 		ktime_get_real_ts(&ts);
 		regs->eax = ts.tv_sec;
 		break;
 	}
 	case LHCALL_BIND_DMA:
+		/* BIND_DMA really wants four arguments, but it's the only call
+		 * which does.  So the Guest packs the number of buffers and
+		 * the interrupt number into the final argument, and we decode
+		 * it here.  This can legitimately fail, since we currently
+		 * place a limit on the number of DMA pools a Guest can have.
+		 * So we return true or false from this call. */
 		regs->eax = bind_dma(lg, regs->edx, regs->ebx,
 				     regs->ecx >> 8, regs->ecx & 0xFF);
 		break;
+
+	/* All these calls simply pass the arguments through to the right
+	 * routines. */
 	case LHCALL_SEND_DMA:
 		send_dma(lg, regs->edx, regs->ebx);
 		break;
@@ -86,10 +112,13 @@ static void do_hcall(struct lguest *lg, struct lguest_regs *regs)
 	case LHCALL_SET_CLOCKEVENT:
 		guest_set_clockevent(lg, regs->edx);
 		break;
+
 	case LHCALL_TS:
+		/* This sets the TS flag, as we saw used in run_guest(). */
 		lg->ts = regs->edx;
 		break;
 	case LHCALL_HALT:
+		/* Similarly, this sets the halted flag for run_guest(). */
 		lg->halted = 1;
 		break;
 	default:
@@ -97,25 +126,42 @@ static void do_hcall(struct lguest *lg, struct lguest_regs *regs)
 	}
 }
 
-/* We always do queued calls before actual hypercall. */
+/* Asynchronous hypercalls are easy: we just look in the array in the Guest's
+ * "struct lguest_data" and see if there are any new ones 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). */
 static void do_async_hcalls(struct lguest *lg)
 {
 	unsigned int i;
 	u8 st[LHCALL_RING_SIZE];
 
+	/* For simplicity, we copy the entire call status array in at once. */
 	if (copy_from_user(&st, &lg->lguest_data->hcall_status, sizeof(st)))
 		return;
 
+
+	/* We process "struct lguest_data"s hcalls[] ring once. */
 	for (i = 0; i < ARRAY_SIZE(st); i++) {
 		struct lguest_regs regs;
+		/* 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. */
 		unsigned int n = lg->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. */
 		if (++lg->next_hcall == LHCALL_RING_SIZE)
 			lg->next_hcall = 0;
 
+		/* We copy the hypercall arguments into a fake register
+		 * structure.  This makes life simple for do_hcall(). */
 		if (get_user(regs.eax, &lg->lguest_data->hcalls[n].eax)
 		    || get_user(regs.edx, &lg->lguest_data->hcalls[n].edx)
 		    || get_user(regs.ecx, &lg->lguest_data->hcalls[n].ecx)
@@ -124,74 +170,126 @@ static void do_async_hcalls(struct lguest *lg)
 			break;
 		}
 
+		/* Do the hypercall, same as a normal one. */
 		do_hcall(lg, &regs);
+
+		/* Mark the hypercall done. */
 		if (put_user(0xFF, &lg->lguest_data->hcall_status[n])) {
 			kill_guest(lg, "Writing result for async hypercall");
 			break;
 		}
 
+ 		/* Stop doing hypercalls if we've just done a DMA to the
+		 * Launcher: it needs to service this first. */
 		if (lg->dma_is_pending)
 			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: */
 static void initialize(struct lguest *lg)
 {
 	u32 tsc_speed;
 
+	/* You can't do anything until you're initialized.  The Guest knows the
+	 * rules, so we're unforgiving here. */
 	if (lg->regs->eax != LHCALL_LGUEST_INIT) {
 		kill_guest(lg, "hypercall %li before LGUEST_INIT",
 			   lg->regs->eax);
 		return;
 	}
 
-	/* We only tell the guest to use the TSC if it's reliable. */
+	/* 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. */
 	if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) && !check_tsc_unstable())
 		tsc_speed = tsc_khz;
 	else
 		tsc_speed = 0;
 
+	/* The pointer to the Guest's "struct lguest_data" is the only
+	 * argument. */
 	lg->lguest_data = (struct lguest_data __user *)lg->regs->edx;
-	/* We check here so we can simply copy_to_user/from_user */
+	/* If we check the address they gave is OK now, we can simply
+	 * copy_to_user/from_user from now on rather than using lgread/lgwrite.
+	 * I put this in to show that I'm not immune to writing stupid
+	 * optimizations. */
 	if (!lguest_address_ok(lg, lg->regs->edx, sizeof(*lg->lguest_data))) {
 		kill_guest(lg, "bad guest page %p", lg->lguest_data);
 		return;
 	}
+	/* The Guest tells us where we're not to deliver interrupts by putting
+	 * the range of addresses into "struct lguest_data". */
 	if (get_user(lg->noirq_start, &lg->lguest_data->noirq_start)
 	    || get_user(lg->noirq_end, &lg->lguest_data->noirq_end)
-	    /* We reserve the top pgd entry. */
+	    /* We tell the Guest that it can't use the top 4MB of virtual
+	     * addresses used by the Switcher. */
 	    || put_user(4U*1024*1024, &lg->lguest_data->reserve_mem)
 	    || put_user(tsc_speed, &lg->lguest_data->tsc_khz)
+	    /* We also give the Guest a unique id, as used in lguest_net.c. */
 	    || put_user(lg->guestid, &lg->lguest_data->guestid))
 		kill_guest(lg, "bad guest page %p", lg->lguest_data);
 
-	/* 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 Guest might be referring to
-	 * the old (read-only) page. */
+	/* 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 Guest might be referring to the old (read-only)
+	 * page. */
 	guest_pagetable_clear_all(lg);
 }
+/* Now we've examined the hypercall code; our Guest can make requests.  There
+ * is one other way we can do things for the Guest, as we see in
+ * emulate_insn(). */
 
-/* Even if we go out to userspace and come back, we don't want to do
- * the hypercall again. */
+/*H:110 Tricky point: we mark the hypercall as "done" once we've done it.
+ * Normally we don't need to do this: the Guest will run again and update the
+ * trap number before we come back around the run_guest() loop to
+ * do_hypercalls().
+ *
+ * However, if we are signalled or the Guest sends DMA to the Launcher, that
+ * loop will exit without running the Guest.  When it comes back it would try
+ * to re-run the hypercall. */
 static void clear_hcall(struct lguest *lg)
 {
 	lg->regs->trapnum = 255;
 }
 
+/*H:100
+ * Hypercalls
+ *
+ * Remember from the Guest, hypercalls come in two flavors: normal and
+ * asynchronous.  This file handles both of types.
+ */
 void do_hypercalls(struct lguest *lg)
 {
+	/* Not initialized yet? */
 	if (unlikely(!lg->lguest_data)) {
+		/* Did the Guest make a hypercall?  We might have come back for
+		 * some other reason (an interrupt, a different trap). */
 		if (lg->regs->trapnum == LGUEST_TRAP_ENTRY) {
+			/* Set up the "struct lguest_data" */
 			initialize(lg);
+			/* The hypercall is done. */
 			clear_hcall(lg);
 		}
 		return;
 	}
 
+	/* The Guest has initialized.
+	 *
+	 * Look in the hypercall ring for the async hypercalls: */
 	do_async_hcalls(lg);
+
+	/* If we stopped reading the hypercall ring because the Guest did a
+	 * SEND_DMA to the Launcher, we want to return now.  Otherwise if the
+	 * Guest asked us to do a hypercall, we do it. */
 	if (!lg->dma_is_pending && lg->regs->trapnum == LGUEST_TRAP_ENTRY) {
 		do_hcall(lg, lg->regs);
+		/* The hypercall is done. */
 		clear_hcall(lg);
 	}
 }
diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c
index b2647974e1a..3d983032264 100644
--- a/drivers/lguest/interrupts_and_traps.c
+++ b/drivers/lguest/interrupts_and_traps.c
@@ -14,100 +14,147 @@
 #include <linux/uaccess.h>
 #include "lg.h"
 
+/* The address of the interrupt handler is split into two bits: */
 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. */
 static int idt_type(u32 lo, u32 hi)
 {
 	return (hi >> 8) & 0xF;
 }
 
+/* An IDT entry can't be used unless the "present" bit is set. */
 static int 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. */
 static void push_guest_stack(struct lguest *lg, unsigned long *gstack, u32 val)
 {
+	/* Stack grows upwards: move stack then write value. */
 	*gstack -= 4;
 	lgwrite_u32(lg, *gstack, val);
 }
 
+/*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.
+ *
+ * "lo" and "hi" are the two parts of the Interrupt Descriptor Table for this
+ * interrupt or trap.  It's split into two parts for traditional reasons: gcc
+ * on i386 used to be frightened by 64 bit numbers.
+ *
+ * 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). */
 static void set_guest_interrupt(struct lguest *lg, u32 lo, u32 hi, int has_err)
 {
 	unsigned long gstack;
 	u32 eflags, ss, irq_enable;
 
-	/* If they want a ring change, we use new stack and push old ss/esp */
+	/* 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. */
 	if ((lg->regs->ss&0x3) != GUEST_PL) {
+		/* The Guest told us their kernel stack with the SET_STACK
+		 * hypercall: both the virtual address and the segment */
 		gstack = guest_pa(lg, lg->esp1);
 		ss = lg->ss1;
+		/* 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. */
 		push_guest_stack(lg, &gstack, lg->regs->ss);
 		push_guest_stack(lg, &gstack, lg->regs->esp);
 	} else {
+		/* We're staying on the same Guest (kernel) stack. */
 		gstack = guest_pa(lg, lg->regs->esp);
 		ss = lg->regs->ss;
 	}
 
-	/* We use IF bit in eflags to indicate whether irqs were enabled
-	   (it's always 1, since irqs are enabled when guest is running). */
+	/* 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: the Guest copies
+	 * it back in "lguest_iret". */
 	eflags = lg->regs->eflags;
 	if (get_user(irq_enable, &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
+	 * "eflags" word, the old code segment, and the old instruction
+	 * pointer. */
 	push_guest_stack(lg, &gstack, eflags);
 	push_guest_stack(lg, &gstack, lg->regs->cs);
 	push_guest_stack(lg, &gstack, lg->regs->eip);
 
+	/* For the six traps which supply an error code, we push that, too. */
 	if (has_err)
 		push_guest_stack(lg, &gstack, lg->regs->errcode);
 
-	/* Change the real stack so switcher returns to trap handler */
+	/* Now we've pushed all the old state, we change the stack, the code
+	 * segment and the address to execute. */
 	lg->regs->ss = ss;
 	lg->regs->esp = gstack + lg->page_offset;
 	lg->regs->cs = (__KERNEL_CS|GUEST_PL);
 	lg->regs->eip = idt_address(lo, hi);
 
-	/* Disable interrupts for an interrupt gate. */
+	/* 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, &lg->lguest_data->irq_enabled))
 			kill_guest(lg, "Disabling interrupts");
 }
 
+/*H:200
+ * Virtual Interrupts.
+ *
+ * maybe_do_interrupt() gets called before every entry to the Guest, to see if
+ * we should divert the Guest to running an interrupt handler. */
 void maybe_do_interrupt(struct lguest *lg)
 {