cr3 OOS optimisation breaks 32-bit GNU/kFreeBSD guest

Commit Message

Marcelo Tosatti March 20, 2009, 11:14 p.m. UTC

On Mon, Feb 23, 2009 at 01:33:05AM +0100, Aurelien Jarno wrote:
> Hi,
> 
> Since kvm-81, I have noticed that GNU/kFreeBSD 32-bit guest are crashing
> under high load (during a compilation for example) with the following 
> error message:
> 
> | Fatal trap 12: page fault while in kernel mode
> | fault virtual address   = 0x4
> | fault code              = supervisor read, page not present
> | instruction pointer     = 0x20:0xc0a4fc00
> | stack pointer           = 0x28:0xe66d7a70
> | frame pointer           = 0x28:0xe66d7a80
> | code segment            = base 0x0, limit 0xfffff, type 0x1b
> |                         = DPL 0, pres 1, def32 1, gran 1
> | processor eflags        = interrupt enabled, resume, IOPL = 0
> | current process         = 24037 (bash)
> | trap number             = 12
> | panic: page fault
> | Uptime: 4m7s
> | Cannot dump. No dump device defined.
> | Automatic reboot in 15 seconds - press a key on the console to abort

Aurelien,

Can you try this patch please.

-------

KVM: MMU: do not allow unsync global pages to become unreachable

Section 5.1 of the Intel TLB doc:

"• INVLPG. This instruction takes a single operand, which is a linear
address. The instruction invalidates any TLB entries for the page number
of that linear address including those for global pages (see Section
7). INVLPG also invalidates all entries in all paging-structure caches
regardless of the linear addresses to which they correspond."

Section 7:

"The following items detail the invalidation of global TLB entries:

• An execution of INVLPG invalidates any TLB entries for the page
number of the linear address that is its operand, even if the entries
are global."

If an unsync global page becomes unreachable via the shadow tree, which
can happen if one its parent pages is zapped, invlpg will fail to
invalidate translations for gvas contained in such unreachable pages.

So invalidate all unsync global entries when zapping a page. Another
possibility would be to cover global pages in the unsync bitmaps, but
that would increase overhead for mmu_sync_roots.


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Aurelien Jarno March 21, 2009, 8:51 a.m. UTC | #1

On Fri, Mar 20, 2009 at 08:14:05PM -0300, Marcelo Tosatti wrote:
> On Mon, Feb 23, 2009 at 01:33:05AM +0100, Aurelien Jarno wrote:
> > Hi,
> > 
> > Since kvm-81, I have noticed that GNU/kFreeBSD 32-bit guest are crashing
> > under high load (during a compilation for example) with the following 
> > error message:
> > 
> > | Fatal trap 12: page fault while in kernel mode
> > | fault virtual address   = 0x4
> > | fault code              = supervisor read, page not present
> > | instruction pointer     = 0x20:0xc0a4fc00
> > | stack pointer           = 0x28:0xe66d7a70
> > | frame pointer           = 0x28:0xe66d7a80
> > | code segment            = base 0x0, limit 0xfffff, type 0x1b
> > |                         = DPL 0, pres 1, def32 1, gran 1
> > | processor eflags        = interrupt enabled, resume, IOPL = 0
> > | current process         = 24037 (bash)
> > | trap number             = 12
> > | panic: page fault
> > | Uptime: 4m7s
> > | Cannot dump. No dump device defined.
> > | Automatic reboot in 15 seconds - press a key on the console to abort
> 
> Aurelien,
> 
> Can you try this patch please.

I have just tried it, and it works. Thanks a lot for your work!

> -------
> 
> KVM: MMU: do not allow unsync global pages to become unreachable
> 
> Section 5.1 of the Intel TLB doc:
> 
> "• INVLPG. This instruction takes a single operand, which is a linear
> address. The instruction invalidates any TLB entries for the page number
> of that linear address including those for global pages (see Section
> 7). INVLPG also invalidates all entries in all paging-structure caches
> regardless of the linear addresses to which they correspond."
> 
> Section 7:
> 
> "The following items detail the invalidation of global TLB entries:
> 
> • An execution of INVLPG invalidates any TLB entries for the page
> number of the linear address that is its operand, even if the entries
> are global."
> 
> If an unsync global page becomes unreachable via the shadow tree, which
> can happen if one its parent pages is zapped, invlpg will fail to
> invalidate translations for gvas contained in such unreachable pages.
> 
> So invalidate all unsync global entries when zapping a page. Another
> possibility would be to cover global pages in the unsync bitmaps, but
> that would increase overhead for mmu_sync_roots.
> 
> Index: kvm/arch/x86/kvm/mmu.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/mmu.c
> +++ kvm/arch/x86/kvm/mmu.c
> @@ -1362,6 +1362,16 @@ static void kvm_mmu_unlink_parents(struc
>  	}
>  }
>  
> +static void mmu_invalidate_unsync_global(struct kvm *kvm)
> +{
> +	struct kvm_mmu_page *sp, *n;
> +
> +	list_for_each_entry_safe(sp, n, &kvm->arch.oos_global_pages, oos_link) {
> +		kvm_mmu_page_unlink_children(kvm, sp);
> +		kvm_unlink_unsync_page(kvm, sp);
> +	}
> +}
> +
>  static int mmu_zap_unsync_children(struct kvm *kvm,
>  				   struct kvm_mmu_page *parent)
>  {
> @@ -1384,6 +1394,8 @@ static int mmu_zap_unsync_children(struc
>  		kvm_mmu_pages_init(parent, &parents, &pages);
>  	}
>  
> +	mmu_invalidate_unsync_global(kvm);
> +
>  	return zapped;
>  }
>  
> 
>

Avi Kivity March 22, 2009, 9:35 a.m. UTC | #2

Marcelo Tosatti wrote:
> On Mon, Feb 23, 2009 at 01:33:05AM +0100, Aurelien Jarno wrote:
>   
>> Hi,
>>
>> Since kvm-81, I have noticed that GNU/kFreeBSD 32-bit guest are crashing
>> under high load (during a compilation for example) with the following 
>> error message:
>>
>> | Fatal trap 12: page fault while in kernel mode
>> | fault virtual address   = 0x4
>> | fault code              = supervisor read, page not present
>> | instruction pointer     = 0x20:0xc0a4fc00
>> | stack pointer           = 0x28:0xe66d7a70
>> | frame pointer           = 0x28:0xe66d7a80
>> | code segment            = base 0x0, limit 0xfffff, type 0x1b
>> |                         = DPL 0, pres 1, def32 1, gran 1
>> | processor eflags        = interrupt enabled, resume, IOPL = 0
>> | current process         = 24037 (bash)
>> | trap number             = 12
>> | panic: page fault
>> | Uptime: 4m7s
>> | Cannot dump. No dump device defined.
>> | Automatic reboot in 15 seconds - press a key on the console to abort
>>     
>
> Aurelien,
>
> Can you try this patch please.
>
> -------
>
> KVM: MMU: do not allow unsync global pages to become unreachable
>
> Section 5.1 of the Intel TLB doc:
>
> "• INVLPG. This instruction takes a single operand, which is a linear
> address. The instruction invalidates any TLB entries for the page number
> of that linear address including those for global pages (see Section
> 7). INVLPG also invalidates all entries in all paging-structure caches
> regardless of the linear addresses to which they correspond."
>
> Section 7:
>
> "The following items detail the invalidation of global TLB entries:
>
> • An execution of INVLPG invalidates any TLB entries for the page
> number of the linear address that is its operand, even if the entries
> are global."
>
> If an unsync global page becomes unreachable via the shadow tree, which
> can happen if one its parent pages is zapped, invlpg will fail to
> invalidate translations for gvas contained in such unreachable pages.
>
> So invalidate all unsync global entries when zapping a page. Another
> possibility would be to cover global pages in the unsync bitmaps, but
> that would increase overhead for mmu_sync_roots.
>
> Index: kvm/arch/x86/kvm/mmu.c
> ===================================================================
> --- kvm.orig/arch/x86/kvm/mmu.c
> +++ kvm/arch/x86/kvm/mmu.c
> @@ -1362,6 +1362,16 @@ static void kvm_mmu_unlink_parents(struc
>  	}
>  }
>  
> +static void mmu_invalidate_unsync_global(struct kvm *kvm)
> +{
> +	struct kvm_mmu_page *sp, *n;
> +
> +	list_for_each_entry_safe(sp, n, &kvm->arch.oos_global_pages, oos_link) {
> +		kvm_mmu_page_unlink_children(kvm, sp);
> +		kvm_unlink_unsync_page(kvm, sp);
> +	}
> +}
> +
>  static int mmu_zap_unsync_children(struct kvm *kvm,
>  				   struct kvm_mmu_page *parent)
>  {
> @@ -1384,6 +1394,8 @@ static int mmu_zap_unsync_children(struc
>  		kvm_mmu_pages_init(parent, &parents, &pages);
>  	}
>  
> +	mmu_invalidate_unsync_global(kvm);
> +
>  	return zapped;
>  }
>  
>   

Good catch, indeed.  But is it sufficient?  We could unlink a page 
through other means, for example by the guest zapping a page directory 
entry.  Maybe it's best to resync when relinking a global page?

Patch

Index: kvm/arch/x86/kvm/mmu.c
===================================================================
--- kvm.orig/arch/x86/kvm/mmu.c
+++ kvm/arch/x86/kvm/mmu.c
@@ -1362,6 +1362,16 @@  static void kvm_mmu_unlink_parents(struc
 	}
 }
 
+static void mmu_invalidate_unsync_global(struct kvm *kvm)
+{
+	struct kvm_mmu_page *sp, *n;
+
+	list_for_each_entry_safe(sp, n, &kvm->arch.oos_global_pages, oos_link) {
+		kvm_mmu_page_unlink_children(kvm, sp);
+		kvm_unlink_unsync_page(kvm, sp);
+	}
+}
+
 static int mmu_zap_unsync_children(struct kvm *kvm,
 				   struct kvm_mmu_page *parent)
 {
@@ -1384,6 +1394,8 @@  static int mmu_zap_unsync_children(struc
 		kvm_mmu_pages_init(parent, &parents, &pages);
 	}
 
+	mmu_invalidate_unsync_global(kvm);
+
 	return zapped;
 }