Re: cr3 OOS optimisation breaks 32-bit GNU/kFreeBSD guest

[Avi Kivity <avi@redhat.com>]

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?


-- 
error compiling committee.c: too many arguments to function