lguest: documentation VII: FIXMEs
authorRusty Russell <rusty@rustcorp.com.au>
Thu, 26 Jul 2007 17:41:05 +0000 (10:41 -0700)
committerLinus Torvalds <torvalds@woody.linux-foundation.org>
Thu, 26 Jul 2007 18:35:17 +0000 (11:35 -0700)
Documentation: The FIXMEs

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>
Documentation/lguest/lguest.c
drivers/char/hvc_lguest.c
drivers/lguest/interrupts_and_traps.c
drivers/lguest/io.c
drivers/lguest/lguest.c
drivers/lguest/lguest_asm.S
drivers/lguest/page_tables.c
drivers/lguest/segments.c
drivers/net/lguest_net.c

index d7e26f02595923c13c7b962bde33f5d704252976..f7918401a0072c9b08bf81bb07434b66a96f6085 100644 (file)
@@ -1510,3 +1510,15 @@ int main(int argc, char *argv[])
        /* Finally, run the Guest.  This doesn't return. */
        run_guest(lguest_fd, &device_list);
 }
+/*:*/
+
+/*M:999
+ * Mastery is done: you now know everything I do.
+ *
+ * But surely you have seen code, features and bugs in your wanderings which
+ * you now yearn to attack?  That is the real game, and I look forward to you
+ * patching and forking lguest into the Your-Name-Here-visor.
+ *
+ * Farewell, and good coding!
+ * Rusty Russell.
+ */
index 1de8967cce0690a4f71aed21986adc5fceb8c15f..feeccbaec4382c55c1583fcad49cf73264a407ce 100644 (file)
@@ -13,6 +13,9 @@
  * functions.
  :*/
 
+/*M:002 The console can be flooded: while the Guest is processing input the
+ * Host can send more.  Buffering in the Host could alleviate this, but it is a
+ * difficult problem in general. :*/
 /* Copyright (C) 2006 Rusty Russell, IBM Corporation
  *
  * This program is free software; you can redistribute it and/or modify
index 3d9830322646a0b02f647b07e1955725882e305a..bd0091bf79ec485edacedcdaf47194e429a35cb3 100644 (file)
@@ -231,6 +231,20 @@ static int direct_trap(const struct lguest *lg,
         * go direct, of course 8) */
        return idt_type(trap->a, trap->b) == 0xF;
 }
+/*:*/
+
+/*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. */
+
+/*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 :*/
 
 /*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
index da288128e44f8df314ecbc4954064b7ab029edd5..ea68613b43f6282fb922d19a5af30b0e2073b870 100644 (file)
@@ -553,6 +553,16 @@ void release_all_dma(struct lguest *lg)
        up_read(&lg->mm->mmap_sem);
 }
 
+/*M:007 We only return a single DMA buffer to the Launcher, but it would be
+ * more efficient to return a pointer to the entire array of DMA buffers, which
+ * it can cache and choose one whenever it wants.
+ *
+ * Currently the Launcher uses a write to /dev/lguest, and the return value is
+ * the address of the DMA structure with the interrupt number placed in
+ * dma->used_len.  If we wanted to return the entire array, we need to return
+ * the address, array size and interrupt number: this seems to require an
+ * ioctl(). :*/
+
 /*L:320 This routine looks for a DMA buffer registered by the Guest on the
  * given key (using the BIND_DMA hypercall). */
 unsigned long get_dma_buffer(struct lguest *lg,
index 7e7e9fb3aefda092b792de2871993dd59ec2587f..6dfe568523a2d0cf0e319fd83bd818886f7fd1cd 100644 (file)
@@ -250,6 +250,14 @@ static void irq_enable(void)
 {
        lguest_data.irq_enabled = X86_EFLAGS_IF;
 }
+/*:*/
+/*M:003 Note that we don't check for outstanding interrupts when we re-enable
+ * them (or when we unmask an interrupt).  This seems to work for the moment,
+ * since interrupts are rare and we'll just get the interrupt on the next timer
+ * tick, but when we turn on CONFIG_NO_HZ, we should revisit this.  One way
+ * would be to put the "irq_enabled" field in a page by itself, and have the
+ * Host write-protect it when an interrupt comes in when irqs are disabled.
+ * There will then be a page fault as soon as interrupts are re-enabled. :*/
 
 /*G:034
  * The Interrupt Descriptor Table (IDT).
index 3126ae923cc0c7108e353163fa660322f8983c7d..f182c6a36209c4c35b5f0c1b5bba3e5bf1b57f0c 100644 (file)
@@ -39,6 +39,20 @@ LGUEST_PATCH(pushf, movl lguest_data+LGUEST_DATA_irq_enabled, %eax)
 .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).
+ *
+ * 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! :*/
+
 /*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*!
  *
index cd047e81cd636f0afaddd5536e5a0d7ec68221fa..b7a924ace68426d85167adb6121f7e39364e9be4 100644 (file)
 #include <asm/tlbflush.h>
 #include "lg.h"
 
+/*M:008 We hold reference to pages, which prevents them from being swapped.
+ * It'd be nice to have a callback in the "struct mm_struct" when Linux wants
+ * to swap out.  If we had this, and a shrinker callback to trim PTE pages, we
+ * could probably consider launching Guests as non-root. :*/
+
 /*H:300
  * The Page Table Code
  *
index 4d4e5a4586f96aad51720458c51a1d928df6252d..f675a41a80da57cdc11d5d4825d2c7ce84a9254f 100644 (file)
@@ -94,6 +94,10 @@ static void check_segment_use(struct lguest *lg, unsigned int desc)
            || lg->regs->ss / 8 == desc)
                kill_guest(lg, "Removed live GDT entry %u", desc);
 }
+/*:*/
+/*M:009 We wouldn't need to check for removal of in-use segments if we handled
+ * faults in the Switcher.  However, it's probably not a worthwhile
+ * optimization. :*/
 
 /*H:610 Once the GDT has been changed, we look through the changed entries and
  * see if they're OK.  If not, we'll call kill_guest() and the Guest will never
index 20df6a848923d4656bf30cd901d1e412c4d43038..cab57911a80e041e1f7b536324f1c56de6a44c19 100644 (file)
 #define MAX_LANS               4
 #define NUM_SKBS               8
 
+/*M:011 Network code master Jeff Garzik points out numerous shortcomings in
+ * this driver if it aspires to greatness.
+ *
+ * Firstly, it doesn't use "NAPI": the networking's New API, and is poorer for
+ * it.  As he says "NAPI means system-wide load leveling, across multiple
+ * network interfaces.  Lack of NAPI can mean competition at higher loads."
+ *
+ * He also points out that we don't implement set_mac_address, so users cannot
+ * change the devices hardware address.  When I asked why one would want to:
+ * "Bonding, and situations where you /do/ want the MAC address to "leak" out
+ * of the host onto the wider net."
+ *
+ * Finally, he would like module unloading: "It is not unrealistic to think of
+ * [un|re|]loading the net support module in an lguest guest.  And, adding
+ * module support makes the programmer more responsible, because they now have
+ * to learn to clean up after themselves.  Any driver that cannot clean up
+ * after itself is an incomplete driver in my book."
+ :*/
+
 /*D:530 The "struct lguestnet_info" contains all the information we need to
  * know about the network device. */
 struct lguestnet_info