lguest: PAE fixes

1) j wasn't initialized in setup_pagetables, so they weren't set up for me causing immediate guest crashes. 2) gpte_addr should not re-read the pmd from the Guest. Especially not BUG_ON() based on the value. If we ever supported SMP guests, they could trigger that. And the Launcher could also trigger it (tho currently root-only). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lguest: PAE fixes
1) j wasn't initialized in setup_pagetables, so they weren't set up for me causing immediate guest crashes. 2) gpte_addr should not re-read the pmd from the Guest. Especially not BUG_ON() based on the value. If we ever supported SMP guests, they could trigger that. And the Launcher could also trigger it (tho currently root-only). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rusty Russell
1 parent acdd0b6292
Showing 1 changed file with 21 additions and 17 deletions Side-by-side Diff
drivers/lguest/page_tables.c
@@ -154,26 +154,25 @@
 	BUG_ON(!(pgd_flags(gpgd) & _PAGE_PRESENT));
 	return gpage + pmd_index(vaddr) * sizeof(pmd_t);
 }
-#endif
  
 static unsigned long gpte_addr(struct lg_cpu *cpu,
-				pgd_t gpgd, unsigned long vaddr)
+			       pmd_t gpmd, unsigned long vaddr)
 {
-#ifdef CONFIG_X86_PAE
-	pmd_t gpmd;
-#endif
-	unsigned long gpage;
+	unsigned long gpage = pmd_pfn(gpmd) << PAGE_SHIFT;
  
-	BUG_ON(!(pgd_flags(gpgd) & _PAGE_PRESENT));
-#ifdef CONFIG_X86_PAE
-	gpmd = lgread(cpu, gpmd_addr(gpgd, vaddr), pmd_t);
-	gpage = pmd_pfn(gpmd) << PAGE_SHIFT;
 	BUG_ON(!(pmd_flags(gpmd) & _PAGE_PRESENT));
+	return gpage + pte_index(vaddr) * sizeof(pte_t);
+}
 #else
-	gpage = pgd_pfn(gpgd) << PAGE_SHIFT;
-#endif
+static unsigned long gpte_addr(struct lg_cpu *cpu,
+				pgd_t gpgd, unsigned long vaddr)
+{
+	unsigned long gpage = pgd_pfn(gpgd) << PAGE_SHIFT;
+
+	BUG_ON(!(pgd_flags(gpgd) & _PAGE_PRESENT));
 	return gpage + pte_index(vaddr) * sizeof(pte_t);
 }
+#endif
 /*:*/
  
 /*M:014 get_pfn is slow: we could probably try to grab batches of pages here as
  
  
@@ -339,10 +338,15 @@
 		 * number in the shadow PMD is the page we just allocated. */
 		native_set_pmd(spmd, __pmd(__pa(ptepage) | pmd_flags(gpmd)));
 	}
-#endif
+
 	/* OK, now we look at the lower level in the Guest page table: keep its
 	 * address, because we might update it later. */
+	gpte_ptr = gpte_addr(cpu, gpmd, vaddr);
+#else
+	/* OK, now we look at the lower level in the Guest page table: keep its
+	 * address, because we might update it later. */
 	gpte_ptr = gpte_addr(cpu, gpgd, vaddr);
+#endif
 	gpte = lgread(cpu, gpte_ptr, pte_t);
  
 	/* If this page isn't in the Guest page tables, we can't page it in. */
@@ -522,7 +526,6 @@
 {
 	pgd_t gpgd;
 	pte_t gpte;
-
 #ifdef CONFIG_X86_PAE
 	pmd_t gpmd;
 #endif
  
  
@@ -534,13 +537,14 @@
 		return -1UL;
 	}
  
-	gpte = lgread(cpu, gpte_addr(cpu, gpgd, vaddr), pte_t);
 #ifdef CONFIG_X86_PAE
 	gpmd = lgread(cpu, gpmd_addr(gpgd, vaddr), pmd_t);
 	if (!(pmd_flags(gpmd) & _PAGE_PRESENT))
 		kill_guest(cpu, "Bad address %#lx", vaddr);
-#endif
+	gpte = lgread(cpu, gpte_addr(cpu, gpmd, vaddr), pte_t);
+#else
 	gpte = lgread(cpu, gpte_addr(cpu, gpgd, vaddr), pte_t);
+#endif
 	if (!(pte_flags(gpte) & _PAGE_PRESENT))
 		kill_guest(cpu, "Bad address %#lx", vaddr);
  
@@ -847,7 +851,7 @@
 	/* The top level points to the linear page table pages above.
 	 * We setup the identity and linear mappings here. */
 #ifdef CONFIG_X86_PAE
-	for (i = 0, j; i < mapped_pages && j < PTRS_PER_PMD;
+	for (i = j = 0; i < mapped_pages && j < PTRS_PER_PMD;
 	     i += PTRS_PER_PTE, j++) {
 		native_set_pmd(&pmd, __pmd(((unsigned long)(linear + i)
 		- mem_base) | _PAGE_PRESENT | _PAGE_RW | _PAGE_USER));
...	...	@@ -154,26 +154,25 @@
154	154	BUG_ON(!(pgd_flags(gpgd) & _PAGE_PRESENT));
155	155	return gpage + pmd_index(vaddr) * sizeof(pmd_t);
156	156	}
157		-#endif
158	157
159	158	static unsigned long gpte_addr(struct lg_cpu *cpu,
160		- pgd_t gpgd, unsigned long vaddr)
	159	+ pmd_t gpmd, unsigned long vaddr)
161	160	{
162		-#ifdef CONFIG_X86_PAE
163		- pmd_t gpmd;
164		-#endif
165		- unsigned long gpage;
	161	+ unsigned long gpage = pmd_pfn(gpmd) << PAGE_SHIFT;
166	162
167		- BUG_ON(!(pgd_flags(gpgd) & _PAGE_PRESENT));
168		-#ifdef CONFIG_X86_PAE
169		- gpmd = lgread(cpu, gpmd_addr(gpgd, vaddr), pmd_t);
170		- gpage = pmd_pfn(gpmd) << PAGE_SHIFT;
171	163	BUG_ON(!(pmd_flags(gpmd) & _PAGE_PRESENT));
	164	+ return gpage + pte_index(vaddr) * sizeof(pte_t);
	165	+}
172	166	#else
173		- gpage = pgd_pfn(gpgd) << PAGE_SHIFT;
174		-#endif
	167	+static unsigned long gpte_addr(struct lg_cpu *cpu,
	168	+ pgd_t gpgd, unsigned long vaddr)
	169	+{
	170	+ unsigned long gpage = pgd_pfn(gpgd) << PAGE_SHIFT;
	171	+
	172	+ BUG_ON(!(pgd_flags(gpgd) & _PAGE_PRESENT));
175	173	return gpage + pte_index(vaddr) * sizeof(pte_t);
176	174	}
	175	+#endif
177	176	/:/
178	177
179	178	/*M:014 get_pfn is slow: we could probably try to grab batches of pages here as
180	179
181	180
...	...	@@ -339,10 +338,15 @@
339	338	* number in the shadow PMD is the page we just allocated. */
340	339	native_set_pmd(spmd, __pmd(__pa(ptepage) \| pmd_flags(gpmd)));
341	340	}
342		-#endif
	341	+
343	342	/* OK, now we look at the lower level in the Guest page table: keep its
344	343	* address, because we might update it later. */
	344	+ gpte_ptr = gpte_addr(cpu, gpmd, vaddr);
	345	+#else
	346	+ /* OK, now we look at the lower level in the Guest page table: keep its
	347	+ * address, because we might update it later. */
345	348	gpte_ptr = gpte_addr(cpu, gpgd, vaddr);
	349	+#endif
346	350	gpte = lgread(cpu, gpte_ptr, pte_t);
347	351
348	352	/* If this page isn't in the Guest page tables, we can't page it in. */
...	...	@@ -522,7 +526,6 @@
522	526	{
523	527	pgd_t gpgd;
524	528	pte_t gpte;
525		-
526	529	#ifdef CONFIG_X86_PAE
527	530	pmd_t gpmd;
528	531	#endif
529	532
530	533
...	...	@@ -534,13 +537,14 @@
534	537	return -1UL;
535	538	}
536	539
537		- gpte = lgread(cpu, gpte_addr(cpu, gpgd, vaddr), pte_t);
538	540	#ifdef CONFIG_X86_PAE
539	541	gpmd = lgread(cpu, gpmd_addr(gpgd, vaddr), pmd_t);
540	542	if (!(pmd_flags(gpmd) & _PAGE_PRESENT))
541	543	kill_guest(cpu, "Bad address %#lx", vaddr);
542		-#endif
	544	+ gpte = lgread(cpu, gpte_addr(cpu, gpmd, vaddr), pte_t);
	545	+#else
543	546	gpte = lgread(cpu, gpte_addr(cpu, gpgd, vaddr), pte_t);
	547	+#endif
544	548	if (!(pte_flags(gpte) & _PAGE_PRESENT))
545	549	kill_guest(cpu, "Bad address %#lx", vaddr);
546	550
...	...	@@ -847,7 +851,7 @@
847	851	/* The top level points to the linear page table pages above.
848	852	* We setup the identity and linear mappings here. */
849	853	#ifdef CONFIG_X86_PAE
850		- for (i = 0, j; i < mapped_pages && j < PTRS_PER_PMD;
	854	+ for (i = j = 0; i < mapped_pages && j < PTRS_PER_PMD;
851	855	i += PTRS_PER_PTE, j++) {
852	856	native_set_pmd(&pmd, __pmd(((unsigned long)(linear + i)
853	857	- mem_base) \| _PAGE_PRESENT \| _PAGE_RW \| _PAGE_USER));