[Lguest] Writing to a single entry at the GDT

[Rusty Russell]

On Tue, 7 Sep 2010 08:15:45 pm Eviatar Khen wrote:
> Hi All,
> I'm hacking lguest of the 2.6.34 kernel version (But it seems that for
> this matter there is no difference with 35).
> In [~/lguest/boot.c], at the function "lguest_write_gdt_entry". The
> documentation says:"we do the lazy thing: alter our GDT, then tell the
> Host to reload the entire thing.  This operation is so rare that this
> naive implementation is reasonable."
> 
> But we actually don't reload all the table because we are using
> LOAD_GDT_ENTRY hypercall. 

Yep, that changed, and I missed this point in the documentation.

> My question is, if we have a hypercall that changes only one entry, why
> we still need the LHCALL_LOAD_TLS hypercall?

It's still more efficient: both because it's a single hypercall and
because the host only copies those changed entries into the GDT when
we switch between guests (see CHANGED_GDT vs CHANGED_GDT_TLS).

Thanks!
Rusty.
PS. Here's the patch I've queued for Linus:

lguest: update comments to reflect LHCALL_LOAD_GDT_ENTRY.

We used to have a hypercall which reloaded the entire GDT, then we
switched to one which loaded a single entry (to match the IDT code).

Some comments were not updated, so fix them.

Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
Reported by: Eviatar Khen <eviatarkhen at gmail.com>

diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -324,9 +324,8 @@ static void lguest_load_gdt(const struct
 }
 
 /*
- * For a single GDT entry which changes, we do the lazy thing: alter our GDT,
- * then tell the Host to reload the entire thing.  This operation is so rare
- * that this naive implementation is reasonable.
+ * For a single GDT entry which changes, we simply change our copy and
+ * then tell the host about it.
  */
 static void lguest_write_gdt_entry(struct desc_struct *dt, int entrynum,
 				   const void *desc, int type)
@@ -338,9 +337,13 @@ static void lguest_write_gdt_entry(struc
 }
 
 /*
- * OK, I lied.  There are three "thread local storage" GDT entries which change
+ * There are three "thread local storage" GDT entries which change
  * on every context switch (these three entries are how glibc implements
- * __thread variables).  So we have a hypercall specifically for this case.
+ * __thread variables).  As an optimization, we have a hypercall
+ * specifically for this case.
+ *
+ * Wouldn't it be nicer to have a general LOAD_GDT_ENTRIES hypercall
+ * which took a range of entries?
  */
 static void lguest_load_tls(struct thread_struct *t, unsigned int cpu)
 {