Skip to contentSkip to conversation

Lguest32, use guest page tables to find paddr for emulated instructions

PATCH In review Steven Rostedt 2 participants 3 messages Apr 4, 2007 – Apr 5, 2007 virtio core

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Lguest32, use guest page tables to find paddr for emulated instructions

[Bug that was found by my previous patch]

This patch allows things like modules, which don't have a direct __pa(EIP) mapping to do emulated instructions.

Sure, the emulated instruction probably should be a paravirt_op, but this patch lets you at least boot a kernel that has modules needing emulated instructions.

drivers/lguest/core.c +6 -3
Index: linux-2.6.21-rc5-mm2/drivers/lguest/core.c
===================================================================
@@ -160,11 +160,14 @@ static int emulate_insn(struct lguest *l
 {
 	u8 insn;
 	unsigned int insnlen = 0, in = 0, shift = 0;
-	unsigned long physaddr = guest_pa(lg, lg->regs->eip);
+	unsigned long physaddr = lguest_find_guest_paddr(lg, lg->regs->eip);
 
-	/* This only works for addresses in linear mapping... */
-	if (lg->regs->eip < lg->page_offset)
+	/* FIXME: Handle physaddr's that crosses pages (modules are in VM) */
+
+	/* did we actually find the physaddr? */
+	if (physaddr == (unsigned long)-1UL)
 		return 0;
+
 	lgread(lg, &insn, physaddr, 1);
 
 	/* Operand size prefix means it's actually for ax. */
drivers/lguest/lg.h +1 -0
Index: linux-2.6.21-rc5-mm2/drivers/lguest/lg.h
===================================================================
@@ -218,6 +218,7 @@ void guest_set_pte(struct lguest *lg, un
 void map_hypervisor_in_guest(struct lguest *lg, struct lguest_pages *pages);
 int demand_page(struct lguest *info, unsigned long cr2, int write);
 void pin_page(struct lguest *lg, unsigned long vaddr);
+unsigned long lguest_find_guest_paddr(struct lguest *lg, unsigned long vaddr);
 
 /* lguest_user.c: */
 int lguest_device_init(void);
drivers/lguest/page_tables.c +19 -0
Index: linux-2.6.21-rc5-mm2/drivers/lguest/page_tables.c
===================================================================
@@ -105,6 +105,25 @@ static spte_t gpte_to_spte(struct lguest
 	return spte;
 }
 
+unsigned long lguest_find_guest_paddr(struct lguest *lg, unsigned long vaddr)
+{
+	gpgd_t gpgd;
+	gpte_t gpte;
+	unsigned long gpte_ptr;
+
+	gpgd = mkgpgd(lgread_u32(lg, gpgd_addr(lg, vaddr)));
+	if (!(gpgd.flags & _PAGE_PRESENT))
+		return -1;
+
+	gpte_ptr = gpte_addr(lg, gpgd, vaddr);
+	gpte = mkgpte(lgread_u32(lg, gpte_ptr));
+
+	if (!(gpte.flags & _PAGE_PRESENT))
+		return -1;
+
+	return (gpte.pfn << PAGE_SHIFT) | (vaddr & (PAGE_SIZE-1));
+}
+
 /* FIXME: We hold reference to pages, which prevents them from being
    swapped.  It'd be nice to have a callback when Linux wants to swap out. */
 
#1 lore raw
Quoted 8 lines · Steven Rostedt

[Bug that was found by my previous patch]

This patch allows things like modules, which don't have a direct __pa(EIP) mapping to do emulated instructions.

Sure, the emulated instruction probably should be a paravirt_op, but this patch lets you at least boot a kernel that has modules needing emulated instructions.

Yeah, I haven't tried loading random modules but I can imagine this does happen (what module was it, BTW?)

I used to have a function just like this, but managed to get rid of it.

Hmm, perhaps we should have an "int lgread_virt_byte(u8 *)" which does the pgtable walk and read all in one? It won't be efficient, but it'll be more correct and maybe even fewer lines 8)

Thanks for the patch!
Rusty.

#2 lore raw
Quoted 4 lines · Rusty Russell

On Wed, 2007-04-04 at 15:07 -0400, Steven Rostedt wrote:

Yeah, I haven't tried loading random modules but I can imagine this does happen (what module was it, BTW?)

I have no idea of which module it crashed on. I didn't investigate that too much. I could simply send a trap to guest when __pa(addr) != lguest_find_guest_paddr(addr) and see which module it crashed on.

My block device I used was basically a copy of a RHEL5 system. I only modified the inittab and fstab to get it working. So on startup and doing the udev init was when it crashed.

Quoted 7 lines

I used to have a function just like this, but managed to get rid of it.

Hmm, perhaps we should have an "int lgread_virt_byte(u8 *)" which does the pgtable walk and read all in one? It won't be efficient, but it'll be more correct and maybe even fewer lines 8)

I forgot that you have a goal to keep lguest small :)

Perhaps we can fork, and have lguest and lguest-lite.

-- Steve