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

Avi Kivity
2009-03-22T09:35:14Z

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