From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH unit-tests] Add async page fault test Date: Wed, 9 May 2012 11:41:19 +0300 Message-ID: <20120509084119.GP15960@redhat.com> References: <20120508112446.GB8988@redhat.com> <4FAA2AD7.7050109@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, mtosatti@redhat.com To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:24501 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754330Ab2EIIlU (ORCPT ); Wed, 9 May 2012 04:41:20 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q498fK0f018344 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 9 May 2012 04:41:20 -0400 Content-Disposition: inline In-Reply-To: <4FAA2AD7.7050109@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, May 09, 2012 at 11:29:11AM +0300, Avi Kivity wrote: > On 05/08/2012 02:24 PM, Gleb Natapov wrote: > > Signed-off-by: Gleb Natapov > > Please describe the regression you're testing for. We could even link > it to the fix with the commit hash. > The test does not really tests for a regression, because there wasn't one. It test that prefault does not generates spurious #PFs. Will write that. > > > void vfree(void *mem) > > { > > unsigned long size = ((unsigned long *)mem)[-1]; > > diff --git a/lib/x86/vm.h b/lib/x86/vm.h > > index 71ab4a8..ff4842f 100644 > > --- a/lib/x86/vm.h > > +++ b/lib/x86/vm.h > > @@ -22,6 +22,7 @@ void vfree(void *mem); > > void *vmap(unsigned long long phys, unsigned long size); > > void *alloc_vpage(void); > > void *alloc_vpages(ulong nr); > > +unsigned long virt_to_phys_cr3(void *mem); > > uint64_t. virt_to_phys() also unsigned long. And get_pte() that virt_to_phys_cr3() uses also. I guess the code is not ready for more then 2^32 memory in 32bit VM. > > > @@ -0,0 +1,98 @@ > > +/* > > + * Async PF test. For the test to actually do anywathing it ineeds to be started > > + * in memory cgroup with 512M of memory and with more then 1G memory provided > > + * to the guest. > > + */ > > Please include instructions or a script on how to do that. > OK. > Alterative ways of doing this: > - file-backed memory using FUSE to control paging Not sure how that can be done. > - add madvise(MADV_DONTNEED) support to testdev, and have the guest > trigger page-in itself. MADV_DONTNEED will drop page, not swap it out. > > I'm not asking to change this test, just providing ideas for the future > in case fine-grained control is needed. It also doesn't thrash the disk. > > > +#include "x86/msr.h" > > +#include "x86/processor.h" > > +#include "x86/apic-defs.h" > > +#include "x86/apic.h" > > +#include "x86/desc.h" > > +#include "x86/isr.h" > > +#include "x86/vm.h" > > + > > +#include "libcflat.h" > > +#include > > + > > +#define KVM_PV_REASON_PAGE_NOT_PRESENT 1 > > +#define KVM_PV_REASON_PAGE_READY 2 > > + > > +#define MSR_KVM_ASYNC_PF_EN 0x4b564d02 > > + > > +#define KVM_ASYNC_PF_ENABLED (1 << 0) > > +#define KVM_ASYNC_PF_SEND_ALWAYS (1 << 1) > > + > > +volatile uint32_t apf_reason __attribute__((aligned(64))); > > +char *buf; > > +volatile uint64_t i; > > +volatile unsigned long phys; > > uint64_t. > > > + > > +static void pf_isr(struct ex_regs *r) > > +{ > > + void* virt = (void*)((ulong)(buf+i) & ~4095ul); > > + > > + switch (get_apf_reason()) { > > + case 0: > > default: I'd rather make deafult: fail the test. It shouldn't happen. > > > + printf("unexpected #PF at %p\n", read_cr2()); > > + fail = true; > > + break; > > + case KVM_PV_REASON_PAGE_NOT_PRESENT: > > + phys = virt_to_phys_cr3(virt); > > + install_pte(phys_to_virt(read_cr3()), 1, virt, phys, 0); > > + write_cr3(read_cr3()); > > What's the point of these? > Shouldn't we reload page tables after changing them? > > + printf("Got not present #PF token %x virt addr %p phys addr %p\n", read_cr2(), virt, phys); > > + while(phys) { > > + irq_enable(); > > + halt(); > > Racy... you need safe_halt() here. The code generated is sti; hlt; cli. By I as well may add safe_halt() to do sti; hlt explicit. > > > + irq_disable(); > > + } > > + break; > > + case KVM_PV_REASON_PAGE_READY: > > + printf("Got present #PF token %x\n", read_cr2()); > > + if ((uint32_t)read_cr2() == ~0) > > + break; > > + install_pte(phys_to_virt(read_cr3()), 1, virt, phys | PTE_PRESENT | PTE_WRITE, 0); > > + write_cr3(read_cr3()); > > + phys = 0; > > + break; > > + } > > +} > > + > > > > -- > error compiling committee.c: too many arguments to function -- Gleb.