[PATCH unit-tests] Add async page fault test

[PATCH unit-tests] Add async page fault test

[Gleb Natapov]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/config-x86-common.mak b/config-x86-common.mak
index c8fbda7..6976f78 100644
--- a/config-x86-common.mak
+++ b/config-x86-common.mak
@@ -34,7 +34,7 @@ tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
                $(TEST_DIR)/realmode.flat $(TEST_DIR)/msr.flat \
                $(TEST_DIR)/hypercall.flat $(TEST_DIR)/sieve.flat \
                $(TEST_DIR)/kvmclock_test.flat  $(TEST_DIR)/eventinj.flat \
-               $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat
+               $(TEST_DIR)/s3.flat $(TEST_DIR)/pmu.flat $(TEST_DIR)/asyncpf.flat
 
 ifdef API
 tests-common += api/api-sample
@@ -90,6 +90,8 @@ $(TEST_DIR)/s3.elf: $(cstart.o) $(TEST_DIR)/s3.o
 
 $(TEST_DIR)/pmu.elf: $(cstart.o) $(TEST_DIR)/pmu.o
 
+$(TEST_DIR)/asyncpf.elf: $(cstart.o) $(TEST_DIR)/asyncpf.o
+
 arch_clean:
 	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
 	$(TEST_DIR)/.*.d $(TEST_DIR)/lib/.*.d $(TEST_DIR)/lib/*.o
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index c7e1afb..f700e9c 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -302,4 +302,9 @@ static inline void invlpg(void *va)
 {
 	asm volatile("invlpg (%0)" ::"r" (va) : "memory");
 }
+
+static inline void halt(void)
+{
+	asm volatile("hlt");
+}
 #endif
diff --git a/lib/x86/vm.c b/lib/x86/vm.c
index 550ec9b..b030934 100644
--- a/lib/x86/vm.c
+++ b/lib/x86/vm.c
@@ -219,6 +219,11 @@ void *vmalloc(unsigned long size)
     return mem;
 }
 
+unsigned long virt_to_phys_cr3(void *mem)
+{
+    return (get_pte(phys_to_virt(read_cr3()), mem) & PTE_ADDR) + ((ulong)mem & (PAGE_SIZE - 1));
+}
+
 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);
 
 void install_pte(unsigned long *cr3,
                         int pte_level,
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
new file mode 100644
index 0000000..f02f983
--- /dev/null
+++ b/x86/asyncpf.c
@@ -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.
+ */
+#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 <stdint.h>
+
+#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;
+bool fail;
+
+static inline uint32_t get_apf_reason(void)
+{
+	uint32_t r = apf_reason;
+	apf_reason = 0;
+	return r;
+}
+
+static void pf_isr(struct ex_regs *r)
+{
+	void* virt = (void*)((ulong)(buf+i) & ~4095ul);
+
+	switch (get_apf_reason()) {
+		case 0:
+			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());
+			printf("Got not present #PF token %x virt addr %p phys addr %p\n", read_cr2(), virt, phys);
+			while(phys) {
+				irq_enable();
+				halt();
+				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;
+	}
+}
+
+#define MEM 1ull*1024*1024*1024
+
+int main(int ac, char **av)
+{
+	int loop = 2;
+
+	setup_vm();
+	setup_idt();
+	setup_gdt();
+	printf("install handler\n");
+	handle_exception(14, pf_isr);
+	apf_reason = 0;
+	printf("enable async pf\n");
+	wrmsr(MSR_KVM_ASYNC_PF_EN, virt_to_phys((void*)&apf_reason) |
+			KVM_ASYNC_PF_SEND_ALWAYS | KVM_ASYNC_PF_ENABLED);
+	printf("alloc memory\n");
+	buf = vmalloc(MEM);
+	irq_enable();
+	while(loop--) {
+		printf("start loop\n");
+		/* access a lot of memory to make host swap it out */
+		for (i=0; i < MEM; i+=4096)
+			buf[i] = 1;
+		printf("end loop\n");
+	}
+	irq_disable();
+
+	printf("%s\n", fail ? "FAIL" : "PASS");
+	return fail;
+}
--
			Gleb.

Re: [PATCH unit-tests] Add async page fault test

[Avi Kivity]

On 05/08/2012 02:24 PM, Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov <gleb@redhat.com>

Please describe the regression you're testing for.  We could even link
it to the fix with the commit hash.


>  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.

> @@ -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.

Alterative ways of doing this:
- file-backed memory using FUSE to control paging
- add madvise(MADV_DONTNEED) support to testdev, and have the guest
trigger page-in itself.

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 <stdint.h>
> +
> +#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:

> +			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?

> +			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.

> +				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

Re: [PATCH unit-tests] Add async page fault test

[Gleb Natapov]

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 <gleb@redhat.com>
> 
> 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 <stdint.h>
> > +
> > +#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.

Re: [PATCH unit-tests] Add async page fault test

[Avi Kivity]

On 05/09/2012 11:41 AM, Gleb Natapov wrote:
> > 
> > >  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.

It's certainly not enterprise quality yet.  But let's not add more problems.

> > 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.

Right, but it will be have to be reloaded from disk (it has to be
file-backed for this to work).  If it's dirty, sync it first.

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

Re: [PATCH unit-tests] Add async page fault test

[Gleb Natapov]

On Wed, May 09, 2012 at 11:52:41AM +0300, Avi Kivity wrote:
> On 05/09/2012 11:41 AM, Gleb Natapov wrote:
> > > 
> > > >  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.
> 
> It's certainly not enterprise quality yet.  But let's not add more problems.
> 
Okay.

> > > 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.
> 
> Right, but it will be have to be reloaded from disk (it has to be
> file-backed for this to work).  If it's dirty, sync it first.
> 
Hmm, yes if it is file backed it may work. Setting up qemu to use file
backed memory is one more complication while running the test though.
I haven't checked by I am not sure that MADV_DONTNEED will drop page
immediately though. It probably puts it on some list to be freed later.
Hmm actually looking at the comments it seems like this is what happens:

/*
 * Application no longer needs these pages.  If the pages are dirty,
 * it's OK to just throw them away.  The app will be more careful about
 * data it wants to keep.  Be sure to free swap resources too.  The
 * zap_page_range call sets things up for shrink_active_list to actually
 * free
 * these pages later if no one else has touched them in the meantime,
 * although we could add these pages to a global reuse list for
 * shrink_active_list to pick up before reclaiming other pages.
 */

--
			Gleb.

Re: [PATCH unit-tests] Add async page fault test

[Takuya Yoshikawa]

On Wed, 9 May 2012 11:59:17 +0300
Gleb Natapov <gleb@redhat.com> wrote:

> Hmm, yes if it is file backed it may work. Setting up qemu to use file
> backed memory is one more complication while running the test though.
> I haven't checked by I am not sure that MADV_DONTNEED will drop page
> immediately though. It probably puts it on some list to be freed later.
> Hmm actually looking at the comments it seems like this is what happens:
> 
> /*
>  * Application no longer needs these pages.  If the pages are dirty,
>  * it's OK to just throw them away.  The app will be more careful about
>  * data it wants to keep.  Be sure to free swap resources too.  The
>  * zap_page_range call sets things up for shrink_active_list to actually
>  * free
>  * these pages later if no one else has touched them in the meantime,
>  * although we could add these pages to a global reuse list for
>  * shrink_active_list to pick up before reclaiming other pages.
>  */

zap_page_range() actually frees these pages, no?

Virtio balloon seems to rely on this.

Thanks,
	Takuya

Re: [PATCH unit-tests] Add async page fault test

[Avi Kivity]

On 05/09/2012 04:18 PM, Takuya Yoshikawa wrote:
> On Wed, 9 May 2012 11:59:17 +0300
> Gleb Natapov <gleb@redhat.com> wrote:
>
> > Hmm, yes if it is file backed it may work. Setting up qemu to use file
> > backed memory is one more complication while running the test though.
> > I haven't checked by I am not sure that MADV_DONTNEED will drop page
> > immediately though. It probably puts it on some list to be freed later.
> > Hmm actually looking at the comments it seems like this is what happens:
> > 
> > /*
> >  * Application no longer needs these pages.  If the pages are dirty,
> >  * it's OK to just throw them away.  The app will be more careful about
> >  * data it wants to keep.  Be sure to free swap resources too.  The
> >  * zap_page_range call sets things up for shrink_active_list to actually
> >  * free
> >  * these pages later if no one else has touched them in the meantime,
> >  * although we could add these pages to a global reuse list for
> >  * shrink_active_list to pick up before reclaiming other pages.
> >  */
>
> zap_page_range() actually frees these pages, no?
>
> Virtio balloon seems to rely on this.
>

The pages are removed from the user address space.  But if they're not
anonymous, the pages still live in the page cache.

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

Re: [PATCH unit-tests] Add async page fault test

[Takuya Yoshikawa]

On Wed, 09 May 2012 16:20:23 +0300
Avi Kivity <avi@redhat.com> wrote:

> > zap_page_range() actually frees these pages, no?
> >
> > Virtio balloon seems to rely on this.
> >
> 
> The pages are removed from the user address space.  But if they're not
> anonymous, the pages still live in the page cache.

Ah, about non-anonymous/file-backed case, I see.

Thanks,
	Takuya