swapping with MMU Notifiers V2

swapping with MMU Notifiers V2

[Andrea Arcangeli]

Hello,

I'm testing KVM swapping on top of Christoph's latest patch
series. However the host is hanging hard for me. Could others test it?
I changed test-hardware, kernel version and kvm kernel version at the
same time, so it might not be an issue with MMU Notifiers V2 but
something else with my new test-setup (previously I was developing and
testing on my workstation which was by far not ideal).

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 4086080..c527d7d 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -18,6 +18,7 @@ config KVM
 	tristate "Kernel-based Virtual Machine (KVM) support"
 	depends on ARCH_SUPPORTS_KVM && EXPERIMENTAL
 	select PREEMPT_NOTIFIERS
+	select MMU_NOTIFIER
 	select ANON_INODES
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 635e70c..80ebc19 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -524,6 +524,110 @@ static void rmap_write_protect(struct kvm *kvm, u64 gfn)
 		kvm_flush_remote_tlbs(kvm);
 }
 
+static void kvm_unmap_spte(struct kvm *kvm, u64 *spte)
+{
+	struct page *page = pfn_to_page((*spte & PT64_BASE_ADDR_MASK) >> PAGE_SHIFT);
+	get_page(page);
+	rmap_remove(kvm, spte);
+	set_shadow_pte(spte, shadow_trap_nonpresent_pte);
+	kvm_flush_remote_tlbs(kvm);
+	__free_page(page);
+}
+
+static void kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
+{
+	u64 *spte, *curr_spte;
+
+	spte = rmap_next(kvm, rmapp, NULL);
+	while (spte) {
+		BUG_ON(!(*spte & PT_PRESENT_MASK));
+		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx\n", spte, *spte);
+		curr_spte = spte;
+		spte = rmap_next(kvm, rmapp, spte);
+		kvm_unmap_spte(kvm, curr_spte);
+	}
+}
+
+void kvm_unmap_hva(struct kvm *kvm, unsigned long hva)
+{
+	int i;
+
+	/*
+	 * If mmap_sem isn't taken, we can look the memslots with only
+	 * the mmu_lock by skipping over the slots with userspace_addr == 0.
+	 */
+	spin_lock(&kvm->mmu_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		struct kvm_memory_slot *memslot = &kvm->memslots[i];
+		unsigned long start = memslot->userspace_addr;
+		unsigned long end;
+
+		/* mmu_lock protects userspace_addr */
+		if (!start)
+			continue;
+
+		end = start + (memslot->npages << PAGE_SHIFT);
+		if (hva >= start && hva < end) {
+			gfn_t gfn_offset = (hva - start) >> PAGE_SHIFT;
+			kvm_unmap_rmapp(kvm, &memslot->rmap[gfn_offset]);
+		}
+	}
+	spin_unlock(&kvm->mmu_lock);
+}
+
+static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp)
+{
+	u64 *spte;
+	int young = 0;
+
+	spte = rmap_next(kvm, rmapp, NULL);
+	while (spte) {
+		int _young;
+		u64 _spte = *spte;
+		BUG_ON(!(_spte & PT_PRESENT_MASK));
+		_young = _spte & PT_ACCESSED_MASK;
+		if (_young) {
+			young = !!_young;
+			set_shadow_pte(spte, _spte & ~PT_ACCESSED_MASK);
+		}
+		spte = rmap_next(kvm, rmapp, spte);
+	}
+	return young;
+}
+
+int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+{
+	int i;
+	int young = 0;
+
+	/*
+	 * If mmap_sem isn't taken, we can look the memslots with only
+	 * the mmu_lock by skipping over the slots with userspace_addr == 0.
+	 */
+	spin_lock(&kvm->mmu_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		struct kvm_memory_slot *memslot = &kvm->memslots[i];
+		unsigned long start = memslot->userspace_addr;
+		unsigned long end;
+
+		/* mmu_lock protects userspace_addr */
+		if (!start)
+			continue;
+
+		end = start + (memslot->npages << PAGE_SHIFT);
+		if (hva >= start && hva < end) {
+			gfn_t gfn_offset = (hva - start) >> PAGE_SHIFT;
+			young |= kvm_age_rmapp(kvm, &memslot->rmap[gfn_offset]);
+		}
+	}
+	spin_unlock(&kvm->mmu_lock);
+
+	if (young)
+		kvm_flush_remote_tlbs(kvm);
+
+	return young;
+}
+
 #ifdef MMU_DEBUG
 static int is_empty_shadow_page(u64 *spt)
 {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8f94a0b..8954836 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3167,6 +3167,44 @@ void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 	free_page((unsigned long)vcpu->arch.pio_data);
 }
 
+static inline struct kvm *mmu_notifier_to_kvm(struct mmu_notifier *mn)
+{
+	return container_of(mn, struct kvm, mmu_notifier);
+}
+
+void kvm_mmu_notifier_invalidate_page(struct mmu_notifier *mn,
+				      struct mm_struct *mm,
+				      unsigned long address)
+{
+	struct kvm *kvm = mmu_notifier_to_kvm(mn);
+	BUG_ON(mm != kvm->mm);
+	kvm_unmap_hva(kvm, address);
+}
+
+void kvm_mmu_notifier_invalidate_range(struct mmu_notifier *mn,
+				       struct mm_struct *mm,
+				       unsigned long start, unsigned long end,
+				       int lock)
+{
+	for (; start < end; start += PAGE_SIZE)
+		kvm_mmu_notifier_invalidate_page(mn, mm, start);
+}
+
+int kvm_mmu_notifier_age_page(struct mmu_notifier *mn,
+			      struct mm_struct *mm,
+			      unsigned long address)
+{
+	struct kvm *kvm = mmu_notifier_to_kvm(mn);
+	BUG_ON(mm != kvm->mm);
+	return kvm_age_hva(kvm, address);
+}
+
+static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
+	.invalidate_range	= kvm_mmu_notifier_invalidate_range,
+	.invalidate_page	= kvm_mmu_notifier_invalidate_page,
+	.age_page		= kvm_mmu_notifier_age_page,
+};
+
 struct  kvm *kvm_arch_create_vm(void)
 {
 	struct kvm *kvm = kzalloc(sizeof(struct kvm), GFP_KERNEL);
@@ -3175,6 +3213,7 @@ struct  kvm *kvm_arch_create_vm(void)
 		return ERR_PTR(-ENOMEM);
 
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
+	kvm->mmu_notifier.ops = &kvm_mmu_notifier_ops;
 
 	return kvm;
 }
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 67ae307..14733f2 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -404,6 +404,8 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu);
 int kvm_mmu_setup(struct kvm_vcpu *vcpu);
 void kvm_mmu_set_nonpresent_ptes(u64 trap_pte, u64 notrap_pte);
 
+void kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
+int kvm_age_hva(struct kvm *kvm, unsigned long hva);
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
 void kvm_mmu_zap_all(struct kvm *kvm);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ea4764b..9349160 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -15,6 +15,7 @@
 #include <linux/sched.h>
 #include <linux/mm.h>
 #include <linux/preempt.h>
+#include <linux/mmu_notifier.h>
 #include <asm/signal.h>
 
 #include <linux/kvm.h>
@@ -118,6 +119,7 @@ struct kvm {
 	struct kvm_io_bus pio_bus;
 	struct kvm_vm_stat stat;
 	struct kvm_arch arch;
+	struct mmu_notifier mmu_notifier;
 };
 
 /* The guest did something we don't support. */
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8fc12dc..bb4747c 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -165,6 +165,7 @@ static struct kvm *kvm_create_vm(void)
 
 	kvm->mm = current->mm;
 	atomic_inc(&kvm->mm->mm_count);
+	mmu_notifier_register(&kvm->mmu_notifier, kvm->mm);
 	spin_lock_init(&kvm->mmu_lock);
 	kvm_io_bus_init(&kvm->pio_bus);
 	mutex_init(&kvm->lock);
@@ -1265,7 +1266,11 @@ static int kvm_resume(struct sys_device *dev)
 }
 
 static struct sysdev_class kvm_sysdev_class = {
+#ifdef set_kset_name
 	set_kset_name("kvm"),
+#else
+	.name = "kvm",
+#endif
 	.suspend = kvm_suspend,
 	.resume = kvm_resume,
 };



You also need the locking patch:

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8a90403..35a2ee0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3219,14 +3249,20 @@ int kvm_arch_set_memory_region(struct kvm *kvm,
 	 */
 	if (!user_alloc) {
 		if (npages && !old.rmap) {
-			memslot->userspace_addr = do_mmap(NULL, 0,
-						     npages * PAGE_SIZE,
-						     PROT_READ | PROT_WRITE,
-						     MAP_SHARED | MAP_ANONYMOUS,
-						     0);
-
-			if (IS_ERR((void *)memslot->userspace_addr))
-				return PTR_ERR((void *)memslot->userspace_addr);
+			unsigned long userspace_addr;
+			
+			userspace_addr = do_mmap(NULL, 0,
+						 npages * PAGE_SIZE,
+						 PROT_READ | PROT_WRITE,
+						 MAP_SHARED | MAP_ANONYMOUS,
+						 0);
+			if (IS_ERR((void *)userspace_addr))
+				return PTR_ERR((void *)userspace_addr);
+
+			/* set userspace_addr atomically for kvm_hva_to_rmapp */
+			spin_lock(&kvm->mmu_lock);
+			memslot->userspace_addr = userspace_addr;
+			spin_unlock(&kvm->mmu_lock);
 		} else {
 			if (!old.user_alloc && old.rmap) {
 				int ret;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4295623..a67e38f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -298,7 +299,15 @@ int __kvm_set_memory_region(struct kvm *kvm,
 		memset(new.rmap, 0, npages * sizeof(*new.rmap));
 
 		new.user_alloc = user_alloc;
-		new.userspace_addr = mem->userspace_addr;
+		/*
+		 * hva_to_rmmap() serialzies with the mmu_lock and to be
+		 * safe it has to ignore memslots with !user_alloc &&
+		 * !userspace_addr.
+		 */
+		if (user_alloc)
+			new.userspace_addr = mem->userspace_addr;
+		else
+			new.userspace_addr = 0;
 	}
 
 	/* Allocate page dirty bitmap if needed */
@@ -311,14 +320,18 @@ int __kvm_set_memory_region(struct kvm *kvm,
 		memset(new.dirty_bitmap, 0, dirty_bytes);
 	}
 
+	spin_lock(&kvm->mmu_lock);
 	if (mem->slot >= kvm->nmemslots)
 		kvm->nmemslots = mem->slot + 1;
 
 	*memslot = new;
+	spin_unlock(&kvm->mmu_lock);
 
 	r = kvm_arch_set_memory_region(kvm, mem, old, user_alloc);
 	if (r) {
+		spin_lock(&kvm->mmu_lock);
 		*memslot = old;
+		spin_unlock(&kvm->mmu_lock);
 		goto out_free;
 	}



This is the updated compatibility code:

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/kernel/external-module-compat.h b/kernel/external-module-compat.h
index 67b9cc4..6d2be6a 100644
--- a/kernel/external-module-compat.h
+++ b/kernel/external-module-compat.h
@@ -17,6 +17,45 @@
 #include <linux/hrtimer.h>
 #include <asm/bitops.h>
 
+#ifndef CONFIG_MMU_NOTIFIER
+struct mmu_notifier;
+
+struct mmu_notifier_ops {
+	/*
+	 * Note: The mmu_notifier structure must be released with
+	 * call_rcu() since other processors are only guaranteed to
+	 * see the changes after a quiescent period.
+	 */
+	void (*release)(struct mmu_notifier *mn,
+			struct mm_struct *mm);
+
+	int (*age_page)(struct mmu_notifier *mn,
+			struct mm_struct *mm,
+			unsigned long address);
+
+	void (*invalidate_page)(struct mmu_notifier *mn,
+				struct mm_struct *mm,
+				unsigned long address);
+
+	/* Dummy needed because the mmu_notifier() macro requires it */
+	void (*invalidate_all)(struct mmu_notifier *mn, struct mm_struct *mm,
+				int dummy);
+
+	/*
+	 * lock indicates that the function is called under spinlock.
+	 */
+	void (*invalidate_range)(struct mmu_notifier *mn,
+				 struct mm_struct *mm,
+				 unsigned long start, unsigned long end,
+				 int lock);
+};
+
+struct mmu_notifier {
+	const struct mmu_notifier_ops *ops;
+};
+#define mmu_notifier_register(mn, mm) do {} while(0)
+#endif
+
 /*
  * 2.6.16 does not have GFP_NOWAIT
  */
 


This is the patch to apply on top of V2 fix the aging:

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/mm/rmap.c b/mm/rmap.c
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -285,10 +285,8 @@ static int page_referenced_one(struct pa
 	if (!pte)
 		goto out;
 
-	if (ptep_clear_flush_young(vma, address, pte))
-		referenced++;
-
-	if (mmu_notifier_age_page(mm, address))
+	if (ptep_clear_flush_young(vma, address, pte) |
+	    mmu_notifier_age_page(mm, address))
 		referenced++;
 
 	/* Pretend the page is referenced if the task has the
@@ -684,7 +682,7 @@ static int try_to_unmap_one(struct page 
 	 * skipped over this mm) then we should reactivate it.
 	 */
 	if (!migration && ((vma->vm_flags & VM_LOCKED) ||
-			(ptep_clear_flush_young(vma, address, pte) ||
+			(ptep_clear_flush_young(vma, address, pte) |
 				mmu_notifier_age_page(mm, address)))) {
 		ret = SWAP_FAIL;
 		goto out_unmap;
@@ -818,10 +816,8 @@ static void try_to_unmap_cluster(unsigne
 		page = vm_normal_page(vma, address, *pte);
 		BUG_ON(!page || PageAnon(page));
 
-		if (ptep_clear_flush_young(vma, address, pte))
-			continue;
-
-		if (mmu_notifier_age_page(mm, address))
+		if (ptep_clear_flush_young(vma, address, pte) | 
+		    mmu_notifier_age_page(mm, address))
 			continue;
 
 		/* Nuke the page table entry. */

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Izik Eidus]

Andrea Arcangeli wrote:
> Hello,
>
> I'm testing KVM swapping on top of Christoph's latest patch
> series. However the host is hanging hard for me. Could others test it?
>   

i will ask alexey to run it

> I changed test-hardware, kernel version and kvm kernel version at the
> same time, so it might not be an issue with MMU Notifiers V2 but
> something else with my new test-setup (previously I was developing and
> testing on my workstation which was by far not ideal).
>   


-- 
woof.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Andrea Arcangeli wrote:
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ea4764b..9349160 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -15,6 +15,7 @@
>  #include <linux/sched.h>
>  #include <linux/mm.h>
>  #include <linux/preempt.h>
> +#include <linux/mmu_notifier.h>
>  #include <asm/signal.h>
> 
>  #include <linux/kvm.h>
> @@ -118,6 +119,7 @@ struct kvm {
>  	struct kvm_io_bus pio_bus;
>  	struct kvm_vm_stat stat;
>  	struct kvm_arch arch;
> +	struct mmu_notifier mmu_notifier;
>  };
> 
>  /* The guest did something we don't support. */
This should not be in struct kvm, it should go to x86's kvm_arch. This 
is x86 specific, we don't need a notifier since the core-vm will just 
page out our guest memory like regular userspace mem.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 8fc12dc..bb4747c 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -165,6 +165,7 @@ static struct kvm *kvm_create_vm(void)
> 
>  	kvm->mm = current->mm;
>  	atomic_inc(&kvm->mm->mm_count);
> +	mmu_notifier_register(&kvm->mmu_notifier, kvm->mm);
>  	spin_lock_init(&kvm->mmu_lock);
>  	kvm_io_bus_init(&kvm->pio_bus);
>  	mutex_init(&kvm->lock);
to kvm_arch_create_vm please

> @@ -1265,7 +1266,11 @@ static int kvm_resume(struct sys_device *dev)
>  }
> 
>  static struct sysdev_class kvm_sysdev_class = {
> +#ifdef set_kset_name
>  	set_kset_name("kvm"),
> +#else
> +	.name = "kvm",
> +#endif
>  	.suspend = kvm_suspend,
>  	.resume = kvm_resume,
>  };

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 4295623..a67e38f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -298,7 +299,15 @@ int __kvm_set_memory_region(struct kvm *kvm,
>  		memset(new.rmap, 0, npages * sizeof(*new.rmap));
> 
>  		new.user_alloc = user_alloc;
> -		new.userspace_addr = mem->userspace_addr;
> +		/*
> +		 * hva_to_rmmap() serialzies with the mmu_lock and to be
> +		 * safe it has to ignore memslots with !user_alloc &&
> +		 * !userspace_addr.
> +		 */
> +		if (user_alloc)
> +			new.userspace_addr = mem->userspace_addr;
> +		else
> +			new.userspace_addr = 0;
>  	}
> 
>  	/* Allocate page dirty bitmap if needed */
> @@ -311,14 +320,18 @@ int __kvm_set_memory_region(struct kvm *kvm,
>  		memset(new.dirty_bitmap, 0, dirty_bytes);
>  	}
> 
> +	spin_lock(&kvm->mmu_lock);
>  	if (mem->slot >= kvm->nmemslots)
>  		kvm->nmemslots = mem->slot + 1;
> 
>  	*memslot = new;
> +	spin_unlock(&kvm->mmu_lock);
> 
>  	r = kvm_arch_set_memory_region(kvm, mem, old, user_alloc);
>  	if (r) {
> +		spin_lock(&kvm->mmu_lock);
>  		*memslot = old;
> +		spin_unlock(&kvm->mmu_lock);
>  		goto out_free;
>  	}
> 
> 
> 
This needs to go to arch too.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Carsten Otte wrote:
>>  #include <linux/kvm.h>
>> @@ -118,6 +119,7 @@ struct kvm {
>>  	struct kvm_io_bus pio_bus;
>>  	struct kvm_vm_stat stat;
>>  	struct kvm_arch arch;
>> +	struct mmu_notifier mmu_notifier;
>>  };
>>
>>  /* The guest did something we don't support. */
>>     
> This should not be in struct kvm, it should go to x86's kvm_arch. This 
> is x86 specific, we don't need a notifier since the core-vm will just 
> page out our guest memory like regular userspace mem.
>
>   

Every arch except s390 needs it.  An ugly #ifndef 
CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Avi Kivity wrote:
> Every arch except s390 needs it.  An ugly #ifndef 
> CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.
Yea I guess you've got a point there. The struct should be in struct 
kvm, but the call to initialize/destroy it should go out to arch_init.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Avi Kivity wrote:
> Every arch except s390 needs it.  An ugly #ifndef 
> CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.
BTW, from reading AMDs spec I don't expect NPT to need this vehicle 
for swapping either. They can just let core-vm page out guest pages 
and will receive a proper page fault in the host. Jörg can you confirm 
that?


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

Didn't realize s390 doesn't need those at all. Do you think
mmu_notifier.h should also go in asm/mmu_notifier? We can always move
them there later after merging with some compat code if needed.

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 4086080..c527d7d 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -18,6 +18,7 @@ config KVM
 	tristate "Kernel-based Virtual Machine (KVM) support"
 	depends on ARCH_SUPPORTS_KVM && EXPERIMENTAL
 	select PREEMPT_NOTIFIERS
+	select MMU_NOTIFIER
 	select ANON_INODES
 	---help---
 	  Support hosting fully virtualized guest machines using hardware
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 635e70c..80ebc19 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -524,6 +524,110 @@ static void rmap_write_protect(struct kvm *kvm, u64 gfn)
 		kvm_flush_remote_tlbs(kvm);
 }
 
+static void kvm_unmap_spte(struct kvm *kvm, u64 *spte)
+{
+	struct page *page = pfn_to_page((*spte & PT64_BASE_ADDR_MASK) >> PAGE_SHIFT);
+	get_page(page);
+	rmap_remove(kvm, spte);
+	set_shadow_pte(spte, shadow_trap_nonpresent_pte);
+	kvm_flush_remote_tlbs(kvm);
+	__free_page(page);
+}
+
+static void kvm_unmap_rmapp(struct kvm *kvm, unsigned long *rmapp)
+{
+	u64 *spte, *curr_spte;
+
+	spte = rmap_next(kvm, rmapp, NULL);
+	while (spte) {
+		BUG_ON(!(*spte & PT_PRESENT_MASK));
+		rmap_printk("kvm_rmap_unmap_hva: spte %p %llx\n", spte, *spte);
+		curr_spte = spte;
+		spte = rmap_next(kvm, rmapp, spte);
+		kvm_unmap_spte(kvm, curr_spte);
+	}
+}
+
+void kvm_unmap_hva(struct kvm *kvm, unsigned long hva)
+{
+	int i;
+
+	/*
+	 * If mmap_sem isn't taken, we can look the memslots with only
+	 * the mmu_lock by skipping over the slots with userspace_addr == 0.
+	 */
+	spin_lock(&kvm->mmu_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		struct kvm_memory_slot *memslot = &kvm->memslots[i];
+		unsigned long start = memslot->userspace_addr;
+		unsigned long end;
+
+		/* mmu_lock protects userspace_addr */
+		if (!start)
+			continue;
+
+		end = start + (memslot->npages << PAGE_SHIFT);
+		if (hva >= start && hva < end) {
+			gfn_t gfn_offset = (hva - start) >> PAGE_SHIFT;
+			kvm_unmap_rmapp(kvm, &memslot->rmap[gfn_offset]);
+		}
+	}
+	spin_unlock(&kvm->mmu_lock);
+}
+
+static int kvm_age_rmapp(struct kvm *kvm, unsigned long *rmapp)
+{
+	u64 *spte;
+	int young = 0;
+
+	spte = rmap_next(kvm, rmapp, NULL);
+	while (spte) {
+		int _young;
+		u64 _spte = *spte;
+		BUG_ON(!(_spte & PT_PRESENT_MASK));
+		_young = _spte & PT_ACCESSED_MASK;
+		if (_young) {
+			young = !!_young;
+			set_shadow_pte(spte, _spte & ~PT_ACCESSED_MASK);
+		}
+		spte = rmap_next(kvm, rmapp, spte);
+	}
+	return young;
+}
+
+int kvm_age_hva(struct kvm *kvm, unsigned long hva)
+{
+	int i;
+	int young = 0;
+
+	/*
+	 * If mmap_sem isn't taken, we can look the memslots with only
+	 * the mmu_lock by skipping over the slots with userspace_addr == 0.
+	 */
+	spin_lock(&kvm->mmu_lock);
+	for (i = 0; i < kvm->nmemslots; i++) {
+		struct kvm_memory_slot *memslot = &kvm->memslots[i];
+		unsigned long start = memslot->userspace_addr;
+		unsigned long end;
+
+		/* mmu_lock protects userspace_addr */
+		if (!start)
+			continue;
+
+		end = start + (memslot->npages << PAGE_SHIFT);
+		if (hva >= start && hva < end) {
+			gfn_t gfn_offset = (hva - start) >> PAGE_SHIFT;
+			young |= kvm_age_rmapp(kvm, &memslot->rmap[gfn_offset]);
+		}
+	}
+	spin_unlock(&kvm->mmu_lock);
+
+	if (young)
+		kvm_flush_remote_tlbs(kvm);
+
+	return young;
+}
+
 #ifdef MMU_DEBUG
 static int is_empty_shadow_page(u64 *spt)
 {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8f94a0b..f556af6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3167,6 +3167,46 @@ void kvm_arch_vcpu_uninit(struct kvm_vcpu *vcpu)
 	free_page((unsigned long)vcpu->arch.pio_data);
 }
 
+static inline struct kvm *mmu_notifier_to_kvm(struct mmu_notifier *mn)
+{
+	struct kvm_arch *kvm_arch;
+	kvm_arch = container_of(mn, struct kvm_arch, mmu_notifier);
+	return container_of(kvm_arch, struct kvm, arch);
+}
+
+void kvm_mmu_notifier_invalidate_page(struct mmu_notifier *mn,
+				      struct mm_struct *mm,
+				      unsigned long address)
+{
+	struct kvm *kvm = mmu_notifier_to_kvm(mn);
+	BUG_ON(mm != kvm->mm);
+	kvm_unmap_hva(kvm, address);
+}
+
+void kvm_mmu_notifier_invalidate_range(struct mmu_notifier *mn,
+				       struct mm_struct *mm,
+				       unsigned long start, unsigned long end,
+				       int lock)
+{
+	for (; start < end; start += PAGE_SIZE)
+		kvm_mmu_notifier_invalidate_page(mn, mm, start);
+}
+
+int kvm_mmu_notifier_age_page(struct mmu_notifier *mn,
+			      struct mm_struct *mm,
+			      unsigned long address)
+{
+	struct kvm *kvm = mmu_notifier_to_kvm(mn);
+	BUG_ON(mm != kvm->mm);
+	return kvm_age_hva(kvm, address);
+}
+
+static const struct mmu_notifier_ops kvm_mmu_notifier_ops = {
+	.invalidate_range	= kvm_mmu_notifier_invalidate_range,
+	.invalidate_page	= kvm_mmu_notifier_invalidate_page,
+	.age_page		= kvm_mmu_notifier_age_page,
+};
+
 struct  kvm *kvm_arch_create_vm(void)
 {
 	struct kvm *kvm = kzalloc(sizeof(struct kvm), GFP_KERNEL);
@@ -3176,6 +3216,9 @@ struct  kvm *kvm_arch_create_vm(void)
 
 	INIT_LIST_HEAD(&kvm->arch.active_mmu_pages);
 
+	kvm->arch.mmu_notifier.ops = &kvm_mmu_notifier_ops;
+	mmu_notifier_register(&kvm->arch.mmu_notifier, current->mm);
+
 	return kvm;
 }
 
diff --git a/include/asm-x86/kvm_host.h b/include/asm-x86/kvm_host.h
index 67ae307..72ba267 100644
--- a/include/asm-x86/kvm_host.h
+++ b/include/asm-x86/kvm_host.h
@@ -13,6 +13,7 @@
 
 #include <linux/types.h>
 #include <linux/mm.h>
+#include <linux/mmu_notifier.h>
 
 #include <linux/kvm.h>
 #include <linux/kvm_para.h>
@@ -287,6 +288,8 @@ struct kvm_arch{
 	int round_robin_prev_vcpu;
 	unsigned int tss_addr;
 	struct page *apic_access_page;
+
+	struct mmu_notifier mmu_notifier;
 };
 
 struct kvm_vm_stat {
@@ -404,6 +407,8 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu);
 int kvm_mmu_setup(struct kvm_vcpu *vcpu);
 void kvm_mmu_set_nonpresent_ptes(u64 trap_pte, u64 notrap_pte);
 
+void kvm_unmap_hva(struct kvm *kvm, unsigned long hva);
+int kvm_age_hva(struct kvm *kvm, unsigned long hva);
 int kvm_mmu_reset_context(struct kvm_vcpu *vcpu);
 void kvm_mmu_slot_remove_write_access(struct kvm *kvm, int slot);
 void kvm_mmu_zap_all(struct kvm *kvm);




> > @@ -1265,7 +1266,11 @@ static int kvm_resume(struct sys_device *dev)
> >  }
> > 
> >  static struct sysdev_class kvm_sysdev_class = {
> > +#ifdef set_kset_name
> >  	set_kset_name("kvm"),
> > +#else
> > +	.name = "kvm",
> > +#endif
> >  	.suspend = kvm_suspend,
> >  	.resume = kvm_resume,
> >  };
> 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index 4295623..a67e38f 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -298,7 +299,15 @@ int __kvm_set_memory_region(struct kvm *kvm,
> >  		memset(new.rmap, 0, npages * sizeof(*new.rmap));
> > 
> >  		new.user_alloc = user_alloc;
> > -		new.userspace_addr = mem->userspace_addr;
> > +		/*
> > +		 * hva_to_rmmap() serialzies with the mmu_lock and to be
> > +		 * safe it has to ignore memslots with !user_alloc &&
> > +		 * !userspace_addr.
> > +		 */
> > +		if (user_alloc)
> > +			new.userspace_addr = mem->userspace_addr;
> > +		else
> > +			new.userspace_addr = 0;
> >  	}
> > 
> >  	/* Allocate page dirty bitmap if needed */
> > @@ -311,14 +320,18 @@ int __kvm_set_memory_region(struct kvm *kvm,
> >  		memset(new.dirty_bitmap, 0, dirty_bytes);
> >  	}
> > 
> > +	spin_lock(&kvm->mmu_lock);
> >  	if (mem->slot >= kvm->nmemslots)
> >  		kvm->nmemslots = mem->slot + 1;
> > 
> >  	*memslot = new;
> > +	spin_unlock(&kvm->mmu_lock);
> > 
> >  	r = kvm_arch_set_memory_region(kvm, mem, old, user_alloc);
> >  	if (r) {
> > +		spin_lock(&kvm->mmu_lock);
> >  		*memslot = old;
> > +		spin_unlock(&kvm->mmu_lock);
> >  		goto out_free;
> >  	}
> > 
> > 
> > 
> This needs to go to arch too.

The mmu_lock isn't a s390 thing, so I doubt we should have different
locking rules for each arch. memslots are common code, and the locking
primitives required to access them should be common too.

Either the data structure is common and their locking primitives are
common too, or the data structure should be per-arch. I realize you
don't currently need to browse the memslots with only the mmu_lock,
but it doesn't sound good to have different locking rules in each arch
for common data structures. The performance impact of the additional
mmu_lock there, should be zero.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Tue, Jan 29, 2008 at 06:24:12PM +0200, Avi Kivity wrote:
> Carsten Otte wrote:
>>>  #include <linux/kvm.h>
>>> @@ -118,6 +119,7 @@ struct kvm {
>>>  	struct kvm_io_bus pio_bus;
>>>  	struct kvm_vm_stat stat;
>>>  	struct kvm_arch arch;
>>> +	struct mmu_notifier mmu_notifier;
>>>  };
>>>
>>>  /* The guest did something we don't support. */
>>>     
>> This should not be in struct kvm, it should go to x86's kvm_arch. This is 
>> x86 specific, we don't need a notifier since the core-vm will just page 
>> out our guest memory like regular userspace mem.
>>
>>   
>
> Every arch except s390 needs it.  An ugly #ifndef 
> CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.

Well I already moved that bit to x86, at least that had a good reason
for being moved there, it's really invisible code to s390. The memslot
are all but invisible to s390 instead, and so the locking rules of the
memslots should be common as long as memslots remains a common-code
data structure too IMHO.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>> Every arch except s390 needs it.  An ugly #ifndef 
>> CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.
> BTW, from reading AMDs spec I don't expect NPT to need this vehicle 
> for swapping either. They can just let core-vm page out guest pages 
> and will receive a proper page fault in the host. Jörg can you confirm 
> that?
>

No, that doesn't work:

- even though npt can use the same pagetable for guest and host, that 
isn't workable for kvm as npt doesn't have an offset/size thing.  so kvm 
uses a separate pagetable for guest and host.
- npt doesn't have a dual-tagged tlb, where a host tlb invalidate also 
invalidates all guest tlbs that point to the same page

Try again in 25 years, maybe x86 will reach where s390 is now by that 
timeframe.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Andrea Arcangeli wrote:
> Didn't realize s390 doesn't need those at all. Do you think
> mmu_notifier.h should also go in asm/mmu_notifier? We can always move
> them there later after merging with some compat code if needed.
>   

s390 is the odd bird, not x86, so I'd like a solution that doesn't 
involve duplication for x86, ppc, and ia64.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Andrea Arcangeli wrote:
> Didn't realize s390 doesn't need those at all. Do you think
> mmu_notifier.h should also go in asm/mmu_notifier? We can always move
> them there later after merging with some compat code if needed.
No I think mmu_notifier.h is fine in include/linux. I just think kvm 
should only _use_ it on archs that do require assistence.

On s390, we use the same page table to translate host.user -> 
host.phys and guest.phys -> host.phys. Using storage keys, the host 
memory management takes into account dirty and reference operations 
done by the guest when doing it's swapping descitions. The host does 
invalidate a page table entry by using a magic "invalidate page table 
entry" instruction. Running virtual cpus are guaranteed not to rely on 
tlb data once the page table entry was invalidated by that 
instruction. Maybe we should just fix other hardware ;-).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Andrea Arcangeli wrote:
> Well I already moved that bit to x86, at least that had a good reason
> for being moved there, it's really invisible code to s390. The memslot
> are all but invisible to s390 instead, and so the locking rules of the
> memslots should be common as long as memslots remains a common-code
> data structure too IMHO.
That makes sense to me.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Tue, Jan 29, 2008 at 06:17:51PM +0100, Carsten Otte wrote:
> Andrea Arcangeli wrote:
>> Well I already moved that bit to x86, at least that had a good reason
>> for being moved there, it's really invisible code to s390. The memslot
>> are all but invisible to s390 instead, and so the locking rules of the
>> memslots should be common as long as memslots remains a common-code
>> data structure too IMHO.
> That makes sense to me.

Ok thanks!

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Tue, Jan 29, 2008 at 07:04:59PM +0200, Avi Kivity wrote:
> Andrea Arcangeli wrote:
>> Didn't realize s390 doesn't need those at all. Do you think
>> mmu_notifier.h should also go in asm/mmu_notifier? We can always move
>> them there later after merging with some compat code if needed.
>>   
>
> s390 is the odd bird, not x86, so I'd like a solution that doesn't involve 

;)

> duplication for x86, ppc, and ia64.

There is a few lines of duplication yes, the bulk of the code was
already only in x86.c and it has to stay there. It's only the few
lines of registration we're talking about it, so I don't think the
#ifdef would save enough.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Andrea Arcangeli wrote:
>
>> duplication for x86, ppc, and ia64.
>>     
>
> There is a few lines of duplication yes, the bulk of the code was
> already only in x86.c and it has to stay there. It's only the few
> lines of registration we're talking about it, so I don't think the
> #ifdef would save enough.
>   

Okay; fine by me.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Tue, Jan 29, 2008 at 05:35:34PM +0100, Carsten Otte wrote:
> Avi Kivity wrote:
> > Every arch except s390 needs it.  An ugly #ifndef 
> > CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.
> BTW, from reading AMDs spec I don't expect NPT to need this vehicle 

By your conclusion I suppose you thought NPT maps guest physical to
host virtual. If it was the case the cpu would to walk three layer of
pagetables (each layer is an arrow): guest virtual -> guest physical
-> host virtual -> host physical. Instead it's just guest virtual ->
guest physical -> host physical. Or even less for shadow: guest
virtual -> host physical, which is why shadow should be faster for
number crunching still (and definitely slower for dbms).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Andrea Arcangeli wrote:
> On Tue, Jan 29, 2008 at 05:35:34PM +0100, Carsten Otte wrote:
>   
>> Avi Kivity wrote:
>>     
>>> Every arch except s390 needs it.  An ugly #ifndef 
>>> CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.
>>>       
>> BTW, from reading AMDs spec I don't expect NPT to need this vehicle 
>>     
>
> By your conclusion I suppose you thought NPT maps guest physical to
> host virtual. If it was the case the cpu would to walk three layer of
> pagetables (each layer is an arrow): guest virtual -> guest physical
> -> host virtual -> host physical. Instead it's just guest virtual ->
> guest physical -> host physical. Or even less for shadow: guest
> virtual -> host physical, which is why shadow should be faster for
> number crunching still (and definitely slower for dbms).
>   

If a hypervisor mandates (host virtual) == (guest physical), it would 
work.  x86 still misses the dual-tagged tlb, so mmu notifiers are needed 
regardless.  With s390, they have an additional offset parameter, so 
(host virtual) == (guest physical) + offset, so qemu can coexist with 
the guest, and dual tagged tlb so that a host invalidate also evicts 
guest tlb entries.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Joerg Roedel]

On Tue, Jan 29, 2008 at 07:02:52PM +0200, Avi Kivity wrote:
> Carsten Otte wrote:
> >Avi Kivity wrote:
> >>Every arch except s390 needs it.  An ugly #ifndef CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating 
> >>the code.
> >BTW, from reading AMDs spec I don't expect NPT to need this vehicle for swapping either. They can just let 
> >core-vm page out guest pages and will receive a proper page fault in the host. Jörg can you confirm that?
> >
> 
> No, that doesn't work:
> 
> - even though npt can use the same pagetable for guest and host, that isn't workable for kvm as npt doesn't 
> have an offset/size thing.  so kvm uses a separate pagetable for guest and host.

Right. We can't reuse page tables from the Linux MM for Nested Paging.

> - npt doesn't have a dual-tagged tlb, where a host tlb invalidate also invalidates all guest tlbs that point 
> to the same page

Since we have our own page table for Nested Paging this is also true.

Joerg

-- 
           |           AMD Saxony Limited Liability Company & Co. KG
 Operating |         Wilschdorfer Landstr. 101, 01109 Dresden, Germany
 System    |                  Register Court Dresden: HRA 4896
 Research  |              General Partner authorized to represent:
 Center    |             AMD Saxony LLC (Wilmington, Delaware, US)
           | General Manager of AMD Saxony LLC: Dr. Hans-R. Deppe, Thomas McCoy



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: swapping with MMU Notifiers V2

[Joerg Roedel]

On Tue, Jan 29, 2008 at 05:35:34PM +0100, Carsten Otte wrote:
> Avi Kivity wrote:
> > Every arch except s390 needs it.  An ugly #ifndef 
> > CONFIG_KVM_HARDWARE_TLB_SYNC is preferred to duplicating the code.
> BTW, from reading AMDs spec I don't expect NPT to need this vehicle 
> for swapping either. They can just let core-vm page out guest pages 
> and will receive a proper page fault in the host. Jörg can you confirm 
> that?

Since NPT uses the host page table format it is in theory possible to
add the pagetable to the Linux MM rmap. In this case it would not be
necessary to use MMU notifiers. But I think this would complicate the
NPT support code significantly.

Joerg

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Tue, Jan 29, 2008 at 08:05:20PM +0200, Avi Kivity wrote:
> If a hypervisor mandates (host virtual) == (guest physical), it would work. 
>  x86 still misses the dual-tagged tlb, so mmu notifiers are needed 
> regardless.  With s390, they have an additional offset parameter, so (host 

Yep. NPT is certainly better given two levels are bad enough. The mmu
notifiers aren't really a performance problem unlike the three levels
would be (the bigger cost is likely the ipi for the tlb flushes on a
smp guest on smp host, and they cost nothing on up guest).

> virtual) == (guest physical) + offset, so qemu can coexist with the guest, 
> and dual tagged tlb so that a host invalidate also evicts guest tlb 
> entries.

Thanks!

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Tue, Jan 29, 2008 at 07:19:18PM +0100, Joerg Roedel wrote:
> Since NPT uses the host page table format it is in theory possible to
> add the pagetable to the Linux MM rmap. In this case it would not be
> necessary to use MMU notifiers. But I think this would complicate the
> NPT support code significantly.

The Linux rmap isn't like Christoph's secondary-rmap, nor similar to
the KVM rmap. The difference is that the linux rmap requires zero ram
in rmap structures, for each new page allocated by a linux page
fault. While KVM rmap requires a few bytes for each new page allocated
with get_user_pages and mapped/cached in some spte. So we can't
represent NTP pagetables in linux rmap and the current mmu notifier
model is quite optimal for it.

What instead may be possible with NTP given the radix tree format, is
to make a KVM rmap implementation for NPT similar to the one in the
linux VM, to avoid losing 64bit of ram for each new NPT pagetable
allocated and mapped, so the mmu notifier may be able to reverse from
host virtual to NPT pagetable without having to use any metadata but
by just walking the NPT tree for the VM. I'm not sure if it's feasible
though.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Joerg Roedel wrote:
> Since NPT uses the host page table format it is in theory possible to
> add the pagetable to the Linux MM rmap. In this case it would not be
> necessary to use MMU notifiers. But I think this would complicate the
> NPT support code significantly.
I was hoping for a "nearest neighbour" to s390 in kvm. You really 
should consider going that way in ends up way simpler :-). Maybe you 
should look over the s390 kvm port when we post it in a few weeks from 
now ;-).

Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Andrea Arcangeli wrote:
> By your conclusion I suppose you thought NPT maps guest physical to
> host virtual. If it was the case the cpu would to walk three layer of
> pagetables (each layer is an arrow): guest virtual -> guest physical
> -> host virtual -> host physical. Instead it's just guest virtual ->
> guest physical -> host physical. Or even less for shadow: guest
> virtual -> host physical, which is why shadow should be faster for
> number crunching still (and definitely slower for dbms).
No, I was assuming guest virtual -> guest physical -> host physical. 
We do the same. We just re-use the very same page table that maps gp 
to hp to map host userland to hp. The only tricky part for AMD64 is to 
hide host userland parts from the guest that are not supposed to be 
visible to the guest-os as far as I see.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Wed, Jan 30, 2008 at 12:26:39PM +0100, Carsten Otte wrote:
> Andrea Arcangeli wrote:
>> By your conclusion I suppose you thought NPT maps guest physical to
>> host virtual. If it was the case the cpu would to walk three layer of
>> pagetables (each layer is an arrow): guest virtual -> guest physical
>> -> host virtual -> host physical. Instead it's just guest virtual ->
>> guest physical -> host physical. Or even less for shadow: guest
>> virtual -> host physical, which is why shadow should be faster for
>> number crunching still (and definitely slower for dbms).
> No, I was assuming guest virtual -> guest physical -> host physical. We do 
> the same. We just re-use the very same page table that maps gp to hp to map 
> host userland to hp. The only tricky part for AMD64 is to hide host 

Oh I see! So when linux pte is cleared, the NPT equivalent is
implicitly and atomically cleared too. That really requires _identical_
semantics and formats for both pagetables.

> userland parts from the guest that are not supposed to be visible to the 
> guest-os as far as I see.

That problem is quite easily fixable by only sharing a 3rd level
pagetable hand having the NPT-pgd filled with only that shared 3rd
level linux pagetable. The guest ram can be allocated with MAP_FIXED
at a proper 512G aligned address. That has the cons to reduce the max
VM (_virtual_ adding more swap won't help) size to 512G. For the 64bit
kernel that may not be really a problem, for 32bit it's likely a
blocker instead. What worries me more are the details of the pte and
tlbs, I don't know the NPT format well enough yet to comment, but I
think when Avi just said we can't flush both tlbs in a single op
through tlb tagging like s390, will still require mmu notifiers. Still
if we can share the ptes it can save quite some memory for large VM,
so even if the mmu notifiers only have to deal with tlb flush + unpin
(without having to clear the NPT themself, anymore, because it was
already cleared by ptep_clear_flush) that may be still interesting to
consider. Which pdf were you reading?

Thanks!

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Joerg Roedel]

On Wed, Jan 30, 2008 at 10:49:10AM +0100, Carsten Otte wrote:
> Joerg Roedel wrote:
> >Since NPT uses the host page table format it is in theory possible to
> >add the pagetable to the Linux MM rmap. In this case it would not be
> >necessary to use MMU notifiers. But I think this would complicate the
> >NPT support code significantly.
> I was hoping for a "nearest neighbour" to s390 in kvm. You really 
> should consider going that way in ends up way simpler :-). Maybe you 
> should look over the s390 kvm port when we post it in a few weeks from 
> now ;-).

Yes, I will definitly have a look at the port. I am curious how paging
works on s390 :-)

Joerg

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Andrea Arcangeli wrote:
> Oh I see! So when linux pte is cleared, the NPT equivalent is
> implicitly and atomically cleared too. That really requires _identical_
> semantics and formats for both pagetables.
Bingo. We have that on s390, and it seems workable on npt too.

> That problem is quite easily fixable by only sharing a 3rd level
> pagetable hand having the NPT-pgd filled with only that shared 3rd
> level linux pagetable. The guest ram can be allocated with MAP_FIXED
> at a proper 512G aligned address. That has the cons to reduce the max
> VM (_virtual_ adding more swap won't help) size to 512G. For the 64bit
> kernel that may not be really a problem, for 32bit it's likely a
> blocker instead. What worries me more are the details of the pte and
> tlbs, I don't know the NPT format well enough yet to comment, but I
> think when Avi just said we can't flush both tlbs in a single op
> through tlb tagging like s390, will still require mmu notifiers. Still
> if we can share the ptes it can save quite some memory for large VM,
> so even if the mmu notifiers only have to deal with tlb flush + unpin
> (without having to clear the NPT themself, anymore, because it was
> already cleared by ptep_clear_flush) that may be still interesting to
> consider. 
We have similar restrictions than you're naming here. Our guest may 
start at a (userspace-) page boundary, and has a fixed 1:1 mapping to 
userspace for a given length. We do that by just having one memory 
slot which has to start at virtual address zero in kvm. I have way to 
few knowledge about x86, but to me both the native page table layout 
and the nested one look very similar. I believe AMD has done a pretty 
good job designing that.

> Which pdf were you reading?
I believe it was 
http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24593.pdf


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Carsten Otte wrote:
> We have similar restrictions than you're naming here. Our guest may 
> start at a (userspace-) page boundary, and has a fixed 1:1 mapping to 
> userspace for a given length. We do that by just having one memory 
> slot which has to start at virtual address zero in kvm.

I thought you're using an offset to put the guest somewhere else?  
Putting the guest at offset zero means that a qemu null pointer deref 
causes a memory corruption instead of an abort.

> I have way to few knowledge about x86, but to me both the native page 
> table layout and the nested one look very similar. I believe AMD has 
> done a pretty good job designing that.

The page table formats are identical.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Carsten Otte]

Avi Kivity wrote:
> Carsten Otte wrote:
>> We have similar restrictions than you're naming here. Our guest may 
>> start at a (userspace-) page boundary, and has a fixed 1:1 mapping to 
>> userspace for a given length. We do that by just having one memory 
>> slot which has to start at virtual address zero in kvm.
> 
> I thought you're using an offset to put the guest somewhere else?  
> Putting the guest at offset zero means that a qemu null pointer deref 
> causes a memory corruption instead of an abort.
At a page boundary in userspace (non-zero), that matches zero in guest 
physical. 1:1+n. Sorry for causing the confusion, my explanation was 
diffuse.

>> I have way to few knowledge about x86, but to me both the native page 
>> table layout and the nested one look very similar. I believe AMD has 
>> done a pretty good job designing that.
> 
> The page table formats are identical.
Looks like Andrea should start hacking then ;-).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

Ok, I think I found one first deadlock source during swapping with the
mmu notifiers and it's a KVM bug. I got a deadlock inversion between
PT lock and mmu_lock because of this bug. With PREEMPT=n it's not
enough to spin_lock(mmu_lock) to disable preempt and in turn the page
fault will go through and it'll take the PT lock _after_ the mmu_lock
which is a forbidden ordering (the VM often calls the mmu notifier
invalidate page with the PT lock held). For example FNAME(fetch) calls
kvm_read_guest_atomic with the mmu_lock held (taken by
FNAME(page_fault)). Not sure why I didn't trigger yet on my
workstation but only on the test system. This seem not enough to get
V2 stable yet (but I think it was enough to get my old codebase stable
on the test system). I'll now try to rollback kvm source to my last
stable status and to apply this fix and run it on V2/V3.

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 8fc12dc..48d9a11 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -553,7 +565,9 @@ int kvm_read_guest_atomic(struct kvm *kvm, gpa_t gpa, void *data,
 	addr = gfn_to_hva(kvm, gfn);
 	if (kvm_is_error_hva(addr))
 		return -EFAULT;
+	pagefault_disable();
 	r = __copy_from_user_inatomic(data, (void __user *)addr + offset, len);
+	pagefault_enable();
 	if (r)
 		return -EFAULT;
 	return 0;

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Andrea Arcangeli wrote:
> Ok, I think I found one first deadlock source during swapping with the
> mmu notifiers and it's a KVM bug. I got a deadlock inversion between
> PT lock and mmu_lock because of this bug. With PREEMPT=n it's not
> enough to spin_lock(mmu_lock) to disable preempt and in turn the page
> fault will go through and it'll take the PT lock _after_ the mmu_lock
> which is a forbidden ordering (the VM often calls the mmu notifier
> invalidate page with the PT lock held). For example FNAME(fetch) calls
> kvm_read_guest_atomic with the mmu_lock held (taken by
> FNAME(page_fault)). Not sure why I didn't trigger yet on my
> workstation but only on the test system. This seem not enough to get
> V2 stable yet (but I think it was enough to get my old codebase stable
> on the test system). I'll now try to rollback kvm source to my last
> stable status and to apply this fix and run it on V2/V3.
>
> Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 8fc12dc..48d9a11 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -553,7 +565,9 @@ int kvm_read_guest_atomic(struct kvm *kvm, gpa_t gpa, void *data,
>  	addr = gfn_to_hva(kvm, gfn);
>  	if (kvm_is_error_hva(addr))
>  		return -EFAULT;
> +	pagefault_disable();
>  	r = __copy_from_user_inatomic(data, (void __user *)addr + offset, len);
> +	pagefault_enable();
>  	if (r)
>  		return -EFAULT;
>  	return 0;
>   

This is surprising.  pagefault_disable() is really a preempt_disable(), 
and kvm_read_guest_atomic() should only be called from atomic contexts 
(with preemption already disabled), no?


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Thu, Jan 31, 2008 at 08:50:01AM +0200, Avi Kivity wrote:
> This is surprising.  pagefault_disable() is really a preempt_disable(), and 
> kvm_read_guest_atomic() should only be called from atomic contexts (with 
> preemption already disabled), no?

_spin_lock calls preempt_disable() and that's the result with PREEMPT=n:

#define preempt_disable()	do { } while (0)

pagefault_disable always calls into inc_preempt_count() which is why
kmap_atomic uses it instead of preempt_disable().

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Andrea Arcangeli wrote:
> On Thu, Jan 31, 2008 at 08:50:01AM +0200, Avi Kivity wrote:
>   
>> This is surprising.  pagefault_disable() is really a preempt_disable(), and 
>> kvm_read_guest_atomic() should only be called from atomic contexts (with 
>> preemption already disabled), no?
>>     
>
> _spin_lock calls preempt_disable() and that's the result with PREEMPT=n:
>
> #define preempt_disable()	do { } while (0)
>
> pagefault_disable always calls into inc_preempt_count() which is why
> kmap_atomic uses it instead of preempt_disable().
>   

I see.  Will merge that patch, thanks.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Thu, Jan 31, 2008 at 12:34:37PM +0200, Avi Kivity wrote:
> I see.  Will merge that patch, thanks.

Thanks. BTW, with this fix I finally got KVM swapping 100% stable
on my test system.

However I had to rollback everything: I'm using my last mmu notifier
patch (not Christoph's ones), my mmu locking patch to browse memslots
with only kvm->mmu_lock, my last kvm patch against my last mmu
notifier patch (only difference is that mmu notifier registers in
common code instead of x86), the pagefault_disable to avoid the
deadlock lock inversion, kvm.git at tag
3d12af8d03b98644df72c9ac59416d37ea000303 and kvm-userland at tag
2439b8e5ab4e733d7793b54315098567ce5deff3. Host kernel was rolled back
from 2.6.24 to 2.6.23-something too.

The last bit I had to rollback was kvm-userland, reconfigure, make
clean, though it'd be surprising if kvm-userland updates are leading
to a host crash, but they may be in the kernel/* directory (I'm using
the external module).

It might also be something stale in the buildsystem (perhaps a distcc
of ccache glitch?), I also cleared 1G of ccache just to be sure in
case it was a ccache glitch, after "make clean". Or I may not have run
make clean after changing --kerneldir or after git pull, thinking the
buildsystem would figure out the change itself (it should have in
theory ;).

Anyway because things are rock solid again now in both my workstation
and in the test system, I will be able to find quickly if something's
really wrong in kvm/kvm-userland or if it was a buildsystem issue of
some sort and I'll move to the latest code again for all pieces.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Thu, Jan 31, 2008 at 01:58:42PM +0100, Andrea Arcangeli wrote:
> It might also be something stale in the buildsystem (perhaps a distcc
> of ccache glitch?), I also cleared 1G of ccache just to be sure in

My build problem might have been related to the fact the
kvm-userland/kernel/include directory isn't used by the kernel build
system while building the external module. I did the kvm modifications
to the kvm.git, synching from kvm.git to kvm-userland.git and then
building the external module against a third tree that happen to have
the kvm_* headers too but slightly different. I thought the
kvm-userland/kernel/include directory was used but apparently it isn't
anymore because the way it gets priority is a bit fragile.

This seems to fix things but it's certainly not more robust, it just
happens to build and run fine so far. I hope there's a better way to
make it work ;)

Signed-off-by: Andrea Arcangeli <andrea-atKUWr5tajBWk0Htik3J/w@public.gmane.org>

diff --git a/kernel/external-module-compat.h b/kernel/external-module-compat.h
index 052d561..96f61d8 100644
--- a/kernel/external-module-compat.h
+++ b/kernel/external-module-compat.h
@@ -10,8 +10,62 @@
 #include <linux/compiler.h>
 #include <linux/version.h>
 #include <linux/string.h>
-#include "include/linux/kvm.h"
+
+#ifndef CONFIG_PREEMPT_NOTIFIERS
+#include <linux/preempt.h>
+#define CONFIG_PREEMPT_NOTIFIERS
+#define CONFIG_PREEMPT_NOTIFIERS_COMPAT
+
+struct preempt_notifier;
+
+struct preempt_ops {
+	void (*sched_in)(struct preempt_notifier *notifier, int cpu);
+	void (*sched_out)(struct preempt_notifier *notifier,
+			  struct task_struct *next);
+};
+
+struct preempt_notifier {
+	struct list_head link;
+	struct task_struct *tsk;
+	struct preempt_ops *ops;
+};
+
+void preempt_notifier_register(struct preempt_notifier *notifier);
+void preempt_notifier_unregister(struct preempt_notifier *notifier);
+
+static inline void preempt_notifier_init(struct preempt_notifier *notifier,
+				     struct preempt_ops *ops)
+{
+	notifier->ops = ops;
+}
+
+void start_special_insn(void);
+void end_special_insn(void);
+void in_special_section(void);
+void special_reload_dr7(void);
+
+void preempt_notifier_sys_init(void);
+void preempt_notifier_sys_exit(void);
+
+#else
+
+static inline void start_special_insn(void) {}
+static inline void end_special_insn(void) {}
+static inline void in_special_section(void) {}
+static inline void special_reload_dr7(void) {}
+
+static inline void preempt_notifier_sys_init(void) {}
+static inline void preempt_notifier_sys_exit(void) {}
+
+#endif
+
+#include "include/asm/kvm_para.h"
+#include "include/asm/kvm.h"
+#include "include/linux/kvm_types.h"
 #include "include/linux/kvm_para.h"
+#include "include/linux/kvm.h"
+#include "include/asm/kvm_host.h"
+#include "include/linux/kvm_host.h"
 #include <linux/cpu.h>
 #include <asm/processor.h>
 #include <linux/hrtimer.h>
@@ -347,53 +401,6 @@ static inline unsigned long long __kvm_cmpxchg64(volatile void *ptr,
 
 #endif
 
-#ifndef CONFIG_PREEMPT_NOTIFIERS
-#define CONFIG_PREEMPT_NOTIFIERS
-#define CONFIG_PREEMPT_NOTIFIERS_COMPAT
-
-struct preempt_notifier;
-
-struct preempt_ops {
-	void (*sched_in)(struct preempt_notifier *notifier, int cpu);
-	void (*sched_out)(struct preempt_notifier *notifier,
-			  struct task_struct *next);
-};
-
-struct preempt_notifier {
-	struct list_head link;
-	struct task_struct *tsk;
-	struct preempt_ops *ops;
-};
-
-void preempt_notifier_register(struct preempt_notifier *notifier);
-void preempt_notifier_unregister(struct preempt_notifier *notifier);
-
-static inline void preempt_notifier_init(struct preempt_notifier *notifier,
-				     struct preempt_ops *ops)
-{
-	notifier->ops = ops;
-}
-
-void start_special_insn(void);
-void end_special_insn(void);
-void in_special_section(void);
-void special_reload_dr7(void);
-
-void preempt_notifier_sys_init(void);
-void preempt_notifier_sys_exit(void);
-
-#else
-
-static inline void start_special_insn(void) {}
-static inline void end_special_insn(void) {}
-static inline void in_special_section(void) {}
-static inline void special_reload_dr7(void) {}
-
-static inline void preempt_notifier_sys_init(void) {}
-static inline void preempt_notifier_sys_exit(void) {}
-
-#endif
-
 /* HRTIMER_MODE_ABS started life with a different name */
 #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,21)
 #define HRTIMER_MODE_ABS HRTIMER_ABS


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Avi Kivity]

Andrea Arcangeli wrote:
> On Thu, Jan 31, 2008 at 01:58:42PM +0100, Andrea Arcangeli wrote:
>   
>> It might also be something stale in the buildsystem (perhaps a distcc
>> of ccache glitch?), I also cleared 1G of ccache just to be sure in
>>     
>
> My build problem might have been related to the fact the
> kvm-userland/kernel/include directory isn't used by the kernel build
> system while building the external module. I did the kvm modifications
> to the kvm.git, synching from kvm.git to kvm-userland.git and then
> building the external module against a third tree that happen to have
> the kvm_* headers too but slightly different. I thought the
> kvm-userland/kernel/include directory was used but apparently it isn't
> anymore because the way it gets priority is a bit fragile.
>   

It is used; for example it supplies headers that don't exist on older 
kernels.  The problem is likely more subtle.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Re: swapping with MMU Notifiers V2

[Andrea Arcangeli]

On Mon, Feb 11, 2008 at 10:20:37AM +0200, Avi Kivity wrote:
> Andrea Arcangeli wrote:
>> On Thu, Jan 31, 2008 at 01:58:42PM +0100, Andrea Arcangeli wrote:
>>   
>>> It might also be something stale in the buildsystem (perhaps a distcc
>>> of ccache glitch?), I also cleared 1G of ccache just to be sure in
>>>     
>>
>> My build problem might have been related to the fact the
>> kvm-userland/kernel/include directory isn't used by the kernel build
>> system while building the external module. I did the kvm modifications
>> to the kvm.git, synching from kvm.git to kvm-userland.git and then
>> building the external module against a third tree that happen to have
>> the kvm_* headers too but slightly different. I thought the
>> kvm-userland/kernel/include directory was used but apparently it isn't
>> anymore because the way it gets priority is a bit fragile.
>>   
>
> It is used; for example it supplies headers that don't exist on older 
> kernels.  The problem is likely more subtle.

See gcc -E for the incorrect include behavior:

# 13
  "/home/andrea/kernel/kvm-userspace/kernel/external-module-compat.h"
  2
# 1 "/home/andrea/kernel/kvm-userspace/kernel/include/linux/kvm.h" 1
# 11 "/home/andrea/kernel/kvm-userspace/kernel/include/linux/kvm.h"

kvm.h correctly comes from $LINUX sync. But then the first instance of
kvm_host.h comes from $KERNELDIR (because it exists now, it wasn't a
problem with older mainline kernels because kvm_host.h didn't exist
yet there)

# 16 "/home/andrea/kernel/kvm-userspace/kernel/svm.c"
# 1 "include/linux/kvm_host.h" 1
# 10 "include/linux/kvm_host.h"

Any modification to linux/kvm_host.h wouldn't be picked up without my
fix. This isn't enough to solve my troubles with last kvm-userland but
at least the right include files are picked now so there are hopefully
no more subtle build troubles with my patch applied.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/