RFC/patch portability: split kvm_vm_ioctl

RFC/patch portability: split kvm_vm_ioctl

[Carsten Otte]

This patch splits kvm_vm_ioctl into archtecture independent parts, and
x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 

Common ioctls for all architectures are:
KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG

I'd really like to see more commonalities, but all others did not fit
our needs. I would love to keep KVM_GET_DIRTY_LOG common, so that the
ingenious migration code does not need to care too much about different
architectures.

x86 specific ioctls are:
KVM_SET_MEMORY_REGION, KVM_SET_USER_MEMORY_REGION,
KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP

While the pic/apic related functions are obviously x86 specific, some
other ioctls seem to be common at a first glance.
KVM_SET_(USER)_MEMORY_REGION for example. We've got a total different
address layout on s390: we cannot support multiple slots, and a user
memory range always equals the guest physical memory [guest_phys + vm
specific offset = host user address]. We don't have nor need dedicated
vmas for the guest memory, we just use what the memory managment has in
stock. This is true, because we reuse the page table for user and guest
mode.
Looks to me like the s390 might have a lot in common with a future AMD
nested page table implementation. If AMD choose to reuse the page table
too, we might share the same ioctl to set up guest addressing with them.

signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
reviewed-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
reviewed-by: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
Index: kvm/drivers/kvm/kvm.h
===================================================================
--- kvm.orig/drivers/kvm/kvm.h	2007-10-12 13:38:59.000000000 +0200
+++ kvm/drivers/kvm/kvm.h	2007-10-12 14:22:40.000000000 +0200
@@ -661,6 +661,9 @@
 			unsigned int ioctl, unsigned long arg);
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg);
+long kvm_arch_vm_ioctl(struct file *filp,
+		        unsigned int ioctl, unsigned long arg);
+void kvm_arch_destroy_vm(struct kvm *kvm);
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
 
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-10-12 13:38:59.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-10-12 13:57:30.000000000 +0200
@@ -40,7 +40,6 @@
 #include <linux/anon_inodes.h>
 #include <linux/profile.h>
 #include <linux/kvm_para.h>
-#include <linux/pagemap.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
@@ -319,61 +318,6 @@
 	return kvm;
 }
 
-static void kvm_free_userspace_physmem(struct kvm_memory_slot *free)
-{
-	int i;
-
-	for (i = 0; i < free->npages; ++i) {
-		if (free->phys_mem[i]) {
-			if (!PageReserved(free->phys_mem[i]))
-				SetPageDirty(free->phys_mem[i]);
-			page_cache_release(free->phys_mem[i]);
-		}
-	}
-}
-
-static void kvm_free_kernel_physmem(struct kvm_memory_slot *free)
-{
-	int i;
-
-	for (i = 0; i < free->npages; ++i)
-		if (free->phys_mem[i])
-			__free_page(free->phys_mem[i]);
-}
-
-/*
- * Free any memory in @free but not in @dont.
- */
-static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
-				  struct kvm_memory_slot *dont)
-{
-	if (!dont || free->phys_mem != dont->phys_mem)
-		if (free->phys_mem) {
-			if (free->user_alloc)
-				kvm_free_userspace_physmem(free);
-			else
-				kvm_free_kernel_physmem(free);
-			vfree(free->phys_mem);
-		}
-	if (!dont || free->rmap != dont->rmap)
-		vfree(free->rmap);
-
-	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
-		vfree(free->dirty_bitmap);
-
-	free->phys_mem = NULL;
-	free->npages = 0;
-	free->dirty_bitmap = NULL;
-}
-
-static void kvm_free_physmem(struct kvm *kvm)
-{
-	int i;
-
-	for (i = 0; i < kvm->nmemslots; ++i)
-		kvm_free_physmem_slot(&kvm->memslots[i], NULL);
-}
-
 static void free_pio_guest_pages(struct kvm_vcpu *vcpu)
 {
 	int i;
@@ -421,7 +365,7 @@
 	kfree(kvm->vpic);
 	kfree(kvm->vioapic);
 	kvm_free_vcpus(kvm);
-	kvm_free_physmem(kvm);
+	kvm_arch_destroy_vm(kvm);
 	kfree(kvm);
 }
 
@@ -686,183 +630,6 @@
 EXPORT_SYMBOL_GPL(fx_init);
 
 /*
- * Allocate some memory and give it an address in the guest physical address
- * space.
- *
- * Discontiguous memory is allowed, mostly for framebuffers.
- */
-static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
-					  struct
-					  kvm_userspace_memory_region *mem,
-					  int user_alloc)
-{
-	int r;
-	gfn_t base_gfn;
-	unsigned long npages;
-	unsigned long i;
-	struct kvm_memory_slot *memslot;
-	struct kvm_memory_slot old, new;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (mem->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (mem->slot >= KVM_MEMORY_SLOTS)
-		goto out;
-	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
-		goto out;
-
-	memslot = &kvm->memslots[mem->slot];
-	base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
-	npages = mem->memory_size >> PAGE_SHIFT;
-
-	if (!npages)
-		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
-
-	mutex_lock(&kvm->lock);
-
-	new = old = *memslot;
-
-	new.base_gfn = base_gfn;
-	new.npages = npages;
-	new.flags = mem->flags;
-
-	/* Disallow changing a memory slot's size. */
-	r = -EINVAL;
-	if (npages && old.npages && npages != old.npages)
-		goto out_unlock;
-
-	/* Check for overlaps */
-	r = -EEXIST;
-	for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
-		struct kvm_memory_slot *s = &kvm->memslots[i];
-
-		if (s == memslot)
-			continue;
-		if (!((base_gfn + npages <= s->base_gfn) ||
-		      (base_gfn >= s->base_gfn + s->npages)))
-			goto out_unlock;
-	}
-
-	/* Deallocate if slot is being removed */
-	if (!npages)
-		new.phys_mem = NULL;
-
-	/* Free page dirty bitmap if unneeded */
-	if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
-		new.dirty_bitmap = NULL;
-
-	r = -ENOMEM;
-
-	/* Allocate if a slot is being created */
-	if (npages && !new.phys_mem) {
-		new.phys_mem = vmalloc(npages * sizeof(struct page *));
-
-		if (!new.phys_mem)
-			goto out_unlock;
-
-		new.rmap = vmalloc(npages * sizeof(struct page *));
-
-		if (!new.rmap)
-			goto out_unlock;
-
-		memset(new.phys_mem, 0, npages * sizeof(struct page *));
-		memset(new.rmap, 0, npages * sizeof(*new.rmap));
-		if (user_alloc) {
-			unsigned long pages_num;
-
-			new.user_alloc = 1;
-			down_read(&current->mm->mmap_sem);
-
-			pages_num = get_user_pages(current, current->mm,
-						   mem->userspace_addr,
-						   npages, 1, 0, new.phys_mem,
-						   NULL);
-
-			up_read(&current->mm->mmap_sem);
-			if (pages_num != npages)
-				goto out_unlock;
-		} else {
-			for (i = 0; i < npages; ++i) {
-				new.phys_mem[i] = alloc_page(GFP_HIGHUSER
-							     | __GFP_ZERO);
-				if (!new.phys_mem[i])
-					goto out_unlock;
-			}
-		}
-	}
-
-	/* Allocate page dirty bitmap if needed */
-	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
-		unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
-
-		new.dirty_bitmap = vmalloc(dirty_bytes);
-		if (!new.dirty_bitmap)
-			goto out_unlock;
-		memset(new.dirty_bitmap, 0, dirty_bytes);
-	}
-
-	if (mem->slot >= kvm->nmemslots)
-		kvm->nmemslots = mem->slot + 1;
-
-	if (!kvm->n_requested_mmu_pages) {
-		unsigned int n_pages;
-
-		if (npages) {
-			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
-						 n_pages);
-		} else {
-			unsigned int nr_mmu_pages;
-
-			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
-			nr_mmu_pages = max(nr_mmu_pages,
-				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
-			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
-		}
-	}
-
-	*memslot = new;
-
-	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
-	kvm_flush_remote_tlbs(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	kvm_free_physmem_slot(&old, &new);
-	return 0;
-
-out_unlock:
-	mutex_unlock(&kvm->lock);
-	kvm_free_physmem_slot(&new, &old);
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
-					  u32 kvm_nr_mmu_pages)
-{
-	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
-		return -EINVAL;
-
-	mutex_lock(&kvm->lock);
-
-	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
-	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
-
-	mutex_unlock(&kvm->lock);
-	return 0;
-}
-
-static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
-{
-	return kvm->n_alloc_mmu_pages;
-}
-
-/*
  * Get (and clear) the dirty memory log for a memory slot.
  */
 static int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
@@ -907,111 +674,6 @@
 	return r;
 }
 
-/*
- * Set a new alias region.  Aliases map a portion of physical memory into
- * another portion.  This is useful for memory windows, for example the PC
- * VGA region.
- */
-static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
-					 struct kvm_memory_alias *alias)
-{
-	int r, n;
-	struct kvm_mem_alias *p;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (alias->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->slot >= KVM_ALIAS_SLOTS)
-		goto out;
-	if (alias->guest_phys_addr + alias->memory_size
-	    < alias->guest_phys_addr)
-		goto out;
-	if (alias->target_phys_addr + alias->memory_size
-	    < alias->target_phys_addr)
-		goto out;
-
-	mutex_lock(&kvm->lock);
-
-	p = &kvm->aliases[alias->slot];
-	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
-	p->npages = alias->memory_size >> PAGE_SHIFT;
-	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
-
-	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
-		if (kvm->aliases[n - 1].npages)
-			break;
-	kvm->naliases = n;
-
-	kvm_mmu_zap_all(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	return 0;
-
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[0],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[1],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(&chip->chip.ioapic,
-			ioapic_irqchip(kvm),
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	return r;
-}
-
-static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&pic_irqchip(kvm)->pics[0],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&pic_irqchip(kvm)->pics[1],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(ioapic_irqchip(kvm),
-			&chip->chip.ioapic,
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	kvm_pic_update_irq(pic_irqchip(kvm));
-	return r;
-}
-
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn)
 {
 	int i;
@@ -2923,7 +2585,7 @@
 {
 	struct kvm *kvm = filp->private_data;
 	void __user *argp = (void __user *)arg;
-	int r = -EINVAL;
+	int r;
 
 	switch (ioctl) {
 	case KVM_CREATE_VCPU:
@@ -2931,43 +2593,6 @@
 		if (r < 0)
 			goto out;
 		break;
-	case KVM_SET_MEMORY_REGION: {
-		struct kvm_memory_region kvm_mem;
-		struct kvm_userspace_memory_region kvm_userspace_mem;
-
-		r = -EFAULT;
-		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
-			goto out;
-		kvm_userspace_mem.slot = kvm_mem.slot;
-		kvm_userspace_mem.flags = kvm_mem.flags;
-		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
-		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_SET_USER_MEMORY_REGION: {
-		struct kvm_userspace_memory_region kvm_userspace_mem;
-
-		r = -EFAULT;
-		if (copy_from_user(&kvm_userspace_mem, argp,
-						sizeof kvm_userspace_mem))
-			goto out;
-
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 1);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_SET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
-		if (r)
-			goto out;
-		break;
-	case KVM_GET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
-		break;
 	case KVM_GET_DIRTY_LOG: {
 		struct kvm_dirty_log log;
 
@@ -2979,87 +2604,8 @@
 			goto out;
 		break;
 	}
-	case KVM_SET_MEMORY_ALIAS: {
-		struct kvm_memory_alias alias;
-
-		r = -EFAULT;
-		if (copy_from_user(&alias, argp, sizeof alias))
-			goto out;
-		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_CREATE_IRQCHIP:
-		r = -ENOMEM;
-		kvm->vpic = kvm_create_pic(kvm);
-		if (kvm->vpic) {
-			r = kvm_ioapic_init(kvm);
-			if (r) {
-				kfree(kvm->vpic);
-				kvm->vpic = NULL;
-				goto out;
-			}
-		} else
-			goto out;
-		break;
-	case KVM_IRQ_LINE: {
-		struct kvm_irq_level irq_event;
-
-		r = -EFAULT;
-		if (copy_from_user(&irq_event, argp, sizeof irq_event))
-			goto out;
-		if (irqchip_in_kernel(kvm)) {
-			mutex_lock(&kvm->lock);
-			if (irq_event.irq < 16)
-				kvm_pic_set_irq(pic_irqchip(kvm),
-					irq_event.irq,
-					irq_event.level);
-			kvm_ioapic_set_irq(kvm->vioapic,
-					irq_event.irq,
-					irq_event.level);
-			mutex_unlock(&kvm->lock);
-			r = 0;
-		}
-		break;
-	}
-	case KVM_GET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(argp, &chip, sizeof chip))
-			goto out;
-		r = 0;
-		break;
-	}
-	case KVM_SET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = 0;
-		break;
-	}
 	default:
-		;
+		return kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
Index: kvm/drivers/kvm/x86.c
===================================================================
--- kvm.orig/drivers/kvm/x86.c	2007-10-12 13:38:59.000000000 +0200
+++ kvm/drivers/kvm/x86.c	2007-10-12 13:57:28.000000000 +0200
@@ -21,6 +21,7 @@
 #include <linux/kvm.h>
 #include <linux/fs.h>
 #include <linux/vmalloc.h>
+#include <linux/pagemap.h>
 
 #include <asm/uaccess.h>
 
@@ -300,6 +301,477 @@
 	return r;
 }
 
+static void kvm_free_userspace_physmem(struct kvm_memory_slot *free)
+{
+	int i;
+
+	for (i = 0; i < free->npages; ++i) {
+		if (free->phys_mem[i]) {
+			if (!PageReserved(free->phys_mem[i]))
+				SetPageDirty(free->phys_mem[i]);
+			page_cache_release(free->phys_mem[i]);
+		}
+	}
+}
+
+static void kvm_free_kernel_physmem(struct kvm_memory_slot *free)
+{
+	int i;
+
+	for (i = 0; i < free->npages; ++i)
+		if (free->phys_mem[i])
+			__free_page(free->phys_mem[i]);
+}
+
+/*
+ * Free any memory in @free but not in @dont.
+ */
+static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
+				  struct kvm_memory_slot *dont)
+{
+	if (!dont || free->phys_mem != dont->phys_mem)
+		if (free->phys_mem) {
+			if (free->user_alloc)
+				kvm_free_userspace_physmem(free);
+			else
+				kvm_free_kernel_physmem(free);
+			vfree(free->phys_mem);
+		}
+	if (!dont || free->rmap != dont->rmap)
+		vfree(free->rmap);
+
+	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
+		vfree(free->dirty_bitmap);
+
+	free->phys_mem = NULL;
+	free->npages = 0;
+	free->dirty_bitmap = NULL;
+}
+
+static void kvm_free_physmem(struct kvm *kvm)
+{
+	int i;
+
+	for (i = 0; i < kvm->nmemslots; ++i)
+		kvm_free_physmem_slot(&kvm->memslots[i], NULL);
+}
+
+/*
+ * Allocate some memory and give it an address in the guest physical address
+ * space.
+ *
+ * Discontiguous memory is allowed, mostly for framebuffers.
+ */
+static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
+					  struct
+					  kvm_userspace_memory_region *mem,
+					  int user_alloc)
+{
+	int r;
+	gfn_t base_gfn;
+	unsigned long npages;
+	unsigned long i;
+	struct kvm_memory_slot *memslot;
+	struct kvm_memory_slot old, new;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (mem->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (mem->slot >= KVM_MEMORY_SLOTS)
+		goto out;
+	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
+		goto out;
+
+	memslot = &kvm->memslots[mem->slot];
+	base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
+	npages = mem->memory_size >> PAGE_SHIFT;
+
+	if (!npages)
+		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
+
+	mutex_lock(&kvm->lock);
+
+	new = old = *memslot;
+
+	new.base_gfn = base_gfn;
+	new.npages = npages;
+	new.flags = mem->flags;
+
+	/* Disallow changing a memory slot's size. */
+	r = -EINVAL;
+	if (npages && old.npages && npages != old.npages)
+		goto out_unlock;
+
+	/* Check for overlaps */
+	r = -EEXIST;
+	for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
+		struct kvm_memory_slot *s = &kvm->memslots[i];
+
+		if (s == memslot)
+			continue;
+		if (!((base_gfn + npages <= s->base_gfn) ||
+		      (base_gfn >= s->base_gfn + s->npages)))
+			goto out_unlock;
+	}
+
+	/* Deallocate if slot is being removed */
+	if (!npages)
+		new.phys_mem = NULL;
+
+	/* Free page dirty bitmap if unneeded */
+	if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
+		new.dirty_bitmap = NULL;
+
+	r = -ENOMEM;
+
+	/* Allocate if a slot is being created */
+	if (npages && !new.phys_mem) {
+		new.phys_mem = vmalloc(npages * sizeof(struct page *));
+
+		if (!new.phys_mem)
+			goto out_unlock;
+
+		new.rmap = vmalloc(npages * sizeof(struct page *));
+
+		if (!new.rmap)
+			goto out_unlock;
+
+		memset(new.phys_mem, 0, npages * sizeof(struct page *));
+		memset(new.rmap, 0, npages * sizeof(*new.rmap));
+		if (user_alloc) {
+			unsigned long pages_num;
+
+			new.user_alloc = 1;
+			down_read(&current->mm->mmap_sem);
+
+			pages_num = get_user_pages(current, current->mm,
+						   mem->userspace_addr,
+						   npages, 1, 0, new.phys_mem,
+						   NULL);
+
+			up_read(&current->mm->mmap_sem);
+			if (pages_num != npages)
+				goto out_unlock;
+		} else {
+			for (i = 0; i < npages; ++i) {
+				new.phys_mem[i] = alloc_page(GFP_HIGHUSER
+							     | __GFP_ZERO);
+				if (!new.phys_mem[i])
+					goto out_unlock;
+			}
+		}
+	}
+
+	/* Allocate page dirty bitmap if needed */
+	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
+		unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
+
+		new.dirty_bitmap = vmalloc(dirty_bytes);
+		if (!new.dirty_bitmap)
+			goto out_unlock;
+		memset(new.dirty_bitmap, 0, dirty_bytes);
+	}
+
+	if (mem->slot >= kvm->nmemslots)
+		kvm->nmemslots = mem->slot + 1;
+
+	if (!kvm->n_requested_mmu_pages) {
+		unsigned int n_pages;
+
+		if (npages) {
+			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
+			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
+						 n_pages);
+		} else {
+			unsigned int nr_mmu_pages;
+
+			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
+			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
+			nr_mmu_pages = max(nr_mmu_pages,
+				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
+			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
+		}
+	}
+
+	*memslot = new;
+
+	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
+	kvm_flush_remote_tlbs(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	kvm_free_physmem_slot(&old, &new);
+	return 0;
+
+out_unlock:
+	mutex_unlock(&kvm->lock);
+	kvm_free_physmem_slot(&new, &old);
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
+					  u32 kvm_nr_mmu_pages)
+{
+	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
+		return -EINVAL;
+
+	mutex_lock(&kvm->lock);
+
+	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
+	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
+
+	mutex_unlock(&kvm->lock);
+	return 0;
+}
+
+static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
+{
+	return kvm->n_alloc_mmu_pages;
+}
+
+/*
+ * Set a new alias region.  Aliases map a portion of physical memory into
+ * another portion.  This is useful for memory windows, for example the PC
+ * VGA region.
+ */
+static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
+					 struct kvm_memory_alias *alias)
+{
+	int r, n;
+	struct kvm_mem_alias *p;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (alias->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->slot >= KVM_ALIAS_SLOTS)
+		goto out;
+	if (alias->guest_phys_addr + alias->memory_size
+	    < alias->guest_phys_addr)
+		goto out;
+	if (alias->target_phys_addr + alias->memory_size
+	    < alias->target_phys_addr)
+		goto out;
+
+	mutex_lock(&kvm->lock);
+
+	p = &kvm->aliases[alias->slot];
+	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
+	p->npages = alias->memory_size >> PAGE_SHIFT;
+	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
+
+	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
+		if (kvm->aliases[n - 1].npages)
+			break;
+	kvm->naliases = n;
+
+	kvm_mmu_zap_all(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	return 0;
+
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[0],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[1],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(&chip->chip.ioapic,
+			ioapic_irqchip(kvm),
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	return r;
+}
+
+static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&pic_irqchip(kvm)->pics[0],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&pic_irqchip(kvm)->pics[1],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(ioapic_irqchip(kvm),
+			&chip->chip.ioapic,
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	kvm_pic_update_irq(pic_irqchip(kvm));
+	return r;
+}
+
+long kvm_arch_vm_ioctl(struct file *filp,
+		       unsigned int ioctl, unsigned long arg)
+{
+	struct kvm *kvm = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	int r = -EINVAL;
+
+	switch(ioctl) {
+	case KVM_SET_MEMORY_REGION: {
+		struct kvm_memory_region kvm_mem;
+		struct kvm_userspace_memory_region kvm_userspace_mem;
+
+		r = -EFAULT;
+		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
+			goto out;
+		kvm_userspace_mem.slot = kvm_mem.slot;
+		kvm_userspace_mem.flags = kvm_mem.flags;
+		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
+		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
+		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_USER_MEMORY_REGION: {
+		struct kvm_userspace_memory_region kvm_userspace_mem;
+
+		r = -EFAULT;
+		if (copy_from_user(&kvm_userspace_mem, argp,
+						sizeof kvm_userspace_mem))
+			goto out;
+
+		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 1);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
+		if (r)
+			goto out;
+		break;
+	case KVM_GET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
+		break;
+	case KVM_SET_MEMORY_ALIAS: {
+		struct kvm_memory_alias alias;
+
+		r = -EFAULT;
+		if (copy_from_user(&alias, argp, sizeof alias))
+			goto out;
+		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_CREATE_IRQCHIP:
+		r = -ENOMEM;
+		kvm->vpic = kvm_create_pic(kvm);
+		if (kvm->vpic) {
+			r = kvm_ioapic_init(kvm);
+			if (r) {
+				kfree(kvm->vpic);
+				kvm->vpic = NULL;
+				goto out;
+			}
+		} else
+			goto out;
+		break;
+	case KVM_IRQ_LINE: {
+		struct kvm_irq_level irq_event;
+
+		r = -EFAULT;
+		if (copy_from_user(&irq_event, argp, sizeof irq_event))
+			goto out;
+		if (irqchip_in_kernel(kvm)) {
+			mutex_lock(&kvm->lock);
+			if (irq_event.irq < 16)
+				kvm_pic_set_irq(pic_irqchip(kvm),
+					irq_event.irq,
+					irq_event.level);
+			kvm_ioapic_set_irq(kvm->vioapic,
+					irq_event.irq,
+					irq_event.level);
+			mutex_unlock(&kvm->lock);
+			r = 0;
+		}
+		break;
+	}
+	case KVM_GET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(argp, &chip, sizeof chip))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_SET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = 0;
+		break;
+	}
+	}
+out:
+	return r;
+}
+
+void kvm_arch_destroy_vm(struct kvm *kvm)
+{
+	kvm_free_physmem(kvm);
+}
+
 static __init void kvm_init_msr_list(void)
 {
 	u32 dummy[2];



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Arnd Bergmann]

On Friday 12 October 2007, Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
> 

I assume the contents are ok, since you're just moving code around, but
please

> signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> reviewed-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> reviewed-by: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

write this 'Signed-off-by' and 'Reviewed-by' (capital letters), and
include a diffstat for any patch that doesn't fit on a few pages
of mail client screen space.

	Arnd <><

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Carsten Otte]

Am Freitag, den 12.10.2007, 15:37 +0200 schrieb Arnd Bergmann:
> I assume the contents are ok, since you're just moving code around, but
> please
> write this 'Signed-off-by' and 'Reviewed-by' (capital letters), and
> include a diffstat for any patch that doesn't fit on a few pages
> of mail client screen space.
The intend of an rfc is in general to review a patch, not to pick on
formalities.

Signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 kvm.h      |    3 
 kvm_main.c |  460 -----------------------------------------------------------
 x86.c      |  472 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 478 insertions(+), 457 deletions(-)
Index: kvm/drivers/kvm/kvm.h
===================================================================
--- kvm.orig/drivers/kvm/kvm.h	2007-10-12 13:38:59.000000000 +0200
+++ kvm/drivers/kvm/kvm.h	2007-10-12 14:22:40.000000000 +0200
@@ -661,6 +661,9 @@
 			unsigned int ioctl, unsigned long arg);
 long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg);
+long kvm_arch_vm_ioctl(struct file *filp,
+		        unsigned int ioctl, unsigned long arg);
+void kvm_arch_destroy_vm(struct kvm *kvm);
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
 
Index: kvm/drivers/kvm/kvm_main.c
===================================================================
--- kvm.orig/drivers/kvm/kvm_main.c	2007-10-12 13:38:59.000000000 +0200
+++ kvm/drivers/kvm/kvm_main.c	2007-10-12 13:57:30.000000000 +0200
@@ -40,7 +40,6 @@
 #include <linux/anon_inodes.h>
 #include <linux/profile.h>
 #include <linux/kvm_para.h>
-#include <linux/pagemap.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
@@ -319,61 +318,6 @@
 	return kvm;
 }
 
-static void kvm_free_userspace_physmem(struct kvm_memory_slot *free)
-{
-	int i;
-
-	for (i = 0; i < free->npages; ++i) {
-		if (free->phys_mem[i]) {
-			if (!PageReserved(free->phys_mem[i]))
-				SetPageDirty(free->phys_mem[i]);
-			page_cache_release(free->phys_mem[i]);
-		}
-	}
-}
-
-static void kvm_free_kernel_physmem(struct kvm_memory_slot *free)
-{
-	int i;
-
-	for (i = 0; i < free->npages; ++i)
-		if (free->phys_mem[i])
-			__free_page(free->phys_mem[i]);
-}
-
-/*
- * Free any memory in @free but not in @dont.
- */
-static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
-				  struct kvm_memory_slot *dont)
-{
-	if (!dont || free->phys_mem != dont->phys_mem)
-		if (free->phys_mem) {
-			if (free->user_alloc)
-				kvm_free_userspace_physmem(free);
-			else
-				kvm_free_kernel_physmem(free);
-			vfree(free->phys_mem);
-		}
-	if (!dont || free->rmap != dont->rmap)
-		vfree(free->rmap);
-
-	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
-		vfree(free->dirty_bitmap);
-
-	free->phys_mem = NULL;
-	free->npages = 0;
-	free->dirty_bitmap = NULL;
-}
-
-static void kvm_free_physmem(struct kvm *kvm)
-{
-	int i;
-
-	for (i = 0; i < kvm->nmemslots; ++i)
-		kvm_free_physmem_slot(&kvm->memslots[i], NULL);
-}
-
 static void free_pio_guest_pages(struct kvm_vcpu *vcpu)
 {
 	int i;
@@ -421,7 +365,7 @@
 	kfree(kvm->vpic);
 	kfree(kvm->vioapic);
 	kvm_free_vcpus(kvm);
-	kvm_free_physmem(kvm);
+	kvm_arch_destroy_vm(kvm);
 	kfree(kvm);
 }
 
@@ -686,183 +630,6 @@
 EXPORT_SYMBOL_GPL(fx_init);
 
 /*
- * Allocate some memory and give it an address in the guest physical address
- * space.
- *
- * Discontiguous memory is allowed, mostly for framebuffers.
- */
-static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
-					  struct
-					  kvm_userspace_memory_region *mem,
-					  int user_alloc)
-{
-	int r;
-	gfn_t base_gfn;
-	unsigned long npages;
-	unsigned long i;
-	struct kvm_memory_slot *memslot;
-	struct kvm_memory_slot old, new;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (mem->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (mem->slot >= KVM_MEMORY_SLOTS)
-		goto out;
-	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
-		goto out;
-
-	memslot = &kvm->memslots[mem->slot];
-	base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
-	npages = mem->memory_size >> PAGE_SHIFT;
-
-	if (!npages)
-		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
-
-	mutex_lock(&kvm->lock);
-
-	new = old = *memslot;
-
-	new.base_gfn = base_gfn;
-	new.npages = npages;
-	new.flags = mem->flags;
-
-	/* Disallow changing a memory slot's size. */
-	r = -EINVAL;
-	if (npages && old.npages && npages != old.npages)
-		goto out_unlock;
-
-	/* Check for overlaps */
-	r = -EEXIST;
-	for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
-		struct kvm_memory_slot *s = &kvm->memslots[i];
-
-		if (s == memslot)
-			continue;
-		if (!((base_gfn + npages <= s->base_gfn) ||
-		      (base_gfn >= s->base_gfn + s->npages)))
-			goto out_unlock;
-	}
-
-	/* Deallocate if slot is being removed */
-	if (!npages)
-		new.phys_mem = NULL;
-
-	/* Free page dirty bitmap if unneeded */
-	if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
-		new.dirty_bitmap = NULL;
-
-	r = -ENOMEM;
-
-	/* Allocate if a slot is being created */
-	if (npages && !new.phys_mem) {
-		new.phys_mem = vmalloc(npages * sizeof(struct page *));
-
-		if (!new.phys_mem)
-			goto out_unlock;
-
-		new.rmap = vmalloc(npages * sizeof(struct page *));
-
-		if (!new.rmap)
-			goto out_unlock;
-
-		memset(new.phys_mem, 0, npages * sizeof(struct page *));
-		memset(new.rmap, 0, npages * sizeof(*new.rmap));
-		if (user_alloc) {
-			unsigned long pages_num;
-
-			new.user_alloc = 1;
-			down_read(&current->mm->mmap_sem);
-
-			pages_num = get_user_pages(current, current->mm,
-						   mem->userspace_addr,
-						   npages, 1, 0, new.phys_mem,
-						   NULL);
-
-			up_read(&current->mm->mmap_sem);
-			if (pages_num != npages)
-				goto out_unlock;
-		} else {
-			for (i = 0; i < npages; ++i) {
-				new.phys_mem[i] = alloc_page(GFP_HIGHUSER
-							     | __GFP_ZERO);
-				if (!new.phys_mem[i])
-					goto out_unlock;
-			}
-		}
-	}
-
-	/* Allocate page dirty bitmap if needed */
-	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
-		unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
-
-		new.dirty_bitmap = vmalloc(dirty_bytes);
-		if (!new.dirty_bitmap)
-			goto out_unlock;
-		memset(new.dirty_bitmap, 0, dirty_bytes);
-	}
-
-	if (mem->slot >= kvm->nmemslots)
-		kvm->nmemslots = mem->slot + 1;
-
-	if (!kvm->n_requested_mmu_pages) {
-		unsigned int n_pages;
-
-		if (npages) {
-			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
-						 n_pages);
-		} else {
-			unsigned int nr_mmu_pages;
-
-			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
-			nr_mmu_pages = max(nr_mmu_pages,
-				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
-			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
-		}
-	}
-
-	*memslot = new;
-
-	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
-	kvm_flush_remote_tlbs(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	kvm_free_physmem_slot(&old, &new);
-	return 0;
-
-out_unlock:
-	mutex_unlock(&kvm->lock);
-	kvm_free_physmem_slot(&new, &old);
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
-					  u32 kvm_nr_mmu_pages)
-{
-	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
-		return -EINVAL;
-
-	mutex_lock(&kvm->lock);
-
-	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
-	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
-
-	mutex_unlock(&kvm->lock);
-	return 0;
-}
-
-static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
-{
-	return kvm->n_alloc_mmu_pages;
-}
-
-/*
  * Get (and clear) the dirty memory log for a memory slot.
  */
 static int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
@@ -907,111 +674,6 @@
 	return r;
 }
 
-/*
- * Set a new alias region.  Aliases map a portion of physical memory into
- * another portion.  This is useful for memory windows, for example the PC
- * VGA region.
- */
-static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
-					 struct kvm_memory_alias *alias)
-{
-	int r, n;
-	struct kvm_mem_alias *p;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (alias->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->slot >= KVM_ALIAS_SLOTS)
-		goto out;
-	if (alias->guest_phys_addr + alias->memory_size
-	    < alias->guest_phys_addr)
-		goto out;
-	if (alias->target_phys_addr + alias->memory_size
-	    < alias->target_phys_addr)
-		goto out;
-
-	mutex_lock(&kvm->lock);
-
-	p = &kvm->aliases[alias->slot];
-	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
-	p->npages = alias->memory_size >> PAGE_SHIFT;
-	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
-
-	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
-		if (kvm->aliases[n - 1].npages)
-			break;
-	kvm->naliases = n;
-
-	kvm_mmu_zap_all(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	return 0;
-
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[0],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[1],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(&chip->chip.ioapic,
-			ioapic_irqchip(kvm),
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	return r;
-}
-
-static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&pic_irqchip(kvm)->pics[0],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&pic_irqchip(kvm)->pics[1],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(ioapic_irqchip(kvm),
-			&chip->chip.ioapic,
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	kvm_pic_update_irq(pic_irqchip(kvm));
-	return r;
-}
-
 gfn_t unalias_gfn(struct kvm *kvm, gfn_t gfn)
 {
 	int i;
@@ -2923,7 +2585,7 @@
 {
 	struct kvm *kvm = filp->private_data;
 	void __user *argp = (void __user *)arg;
-	int r = -EINVAL;
+	int r;
 
 	switch (ioctl) {
 	case KVM_CREATE_VCPU:
@@ -2931,43 +2593,6 @@
 		if (r < 0)
 			goto out;
 		break;
-	case KVM_SET_MEMORY_REGION: {
-		struct kvm_memory_region kvm_mem;
-		struct kvm_userspace_memory_region kvm_userspace_mem;
-
-		r = -EFAULT;
-		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
-			goto out;
-		kvm_userspace_mem.slot = kvm_mem.slot;
-		kvm_userspace_mem.flags = kvm_mem.flags;
-		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
-		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_SET_USER_MEMORY_REGION: {
-		struct kvm_userspace_memory_region kvm_userspace_mem;
-
-		r = -EFAULT;
-		if (copy_from_user(&kvm_userspace_mem, argp,
-						sizeof kvm_userspace_mem))
-			goto out;
-
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 1);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_SET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
-		if (r)
-			goto out;
-		break;
-	case KVM_GET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
-		break;
 	case KVM_GET_DIRTY_LOG: {
 		struct kvm_dirty_log log;
 
@@ -2979,87 +2604,8 @@
 			goto out;
 		break;
 	}
-	case KVM_SET_MEMORY_ALIAS: {
-		struct kvm_memory_alias alias;
-
-		r = -EFAULT;
-		if (copy_from_user(&alias, argp, sizeof alias))
-			goto out;
-		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_CREATE_IRQCHIP:
-		r = -ENOMEM;
-		kvm->vpic = kvm_create_pic(kvm);
-		if (kvm->vpic) {
-			r = kvm_ioapic_init(kvm);
-			if (r) {
-				kfree(kvm->vpic);
-				kvm->vpic = NULL;
-				goto out;
-			}
-		} else
-			goto out;
-		break;
-	case KVM_IRQ_LINE: {
-		struct kvm_irq_level irq_event;
-
-		r = -EFAULT;
-		if (copy_from_user(&irq_event, argp, sizeof irq_event))
-			goto out;
-		if (irqchip_in_kernel(kvm)) {
-			mutex_lock(&kvm->lock);
-			if (irq_event.irq < 16)
-				kvm_pic_set_irq(pic_irqchip(kvm),
-					irq_event.irq,
-					irq_event.level);
-			kvm_ioapic_set_irq(kvm->vioapic,
-					irq_event.irq,
-					irq_event.level);
-			mutex_unlock(&kvm->lock);
-			r = 0;
-		}
-		break;
-	}
-	case KVM_GET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(argp, &chip, sizeof chip))
-			goto out;
-		r = 0;
-		break;
-	}
-	case KVM_SET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = 0;
-		break;
-	}
 	default:
-		;
+		return kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
Index: kvm/drivers/kvm/x86.c
===================================================================
--- kvm.orig/drivers/kvm/x86.c	2007-10-12 13:38:59.000000000 +0200
+++ kvm/drivers/kvm/x86.c	2007-10-12 13:57:28.000000000 +0200
@@ -21,6 +21,7 @@
 #include <linux/kvm.h>
 #include <linux/fs.h>
 #include <linux/vmalloc.h>
+#include <linux/pagemap.h>
 
 #include <asm/uaccess.h>
 
@@ -300,6 +301,477 @@
 	return r;
 }
 
+static void kvm_free_userspace_physmem(struct kvm_memory_slot *free)
+{
+	int i;
+
+	for (i = 0; i < free->npages; ++i) {
+		if (free->phys_mem[i]) {
+			if (!PageReserved(free->phys_mem[i]))
+				SetPageDirty(free->phys_mem[i]);
+			page_cache_release(free->phys_mem[i]);
+		}
+	}
+}
+
+static void kvm_free_kernel_physmem(struct kvm_memory_slot *free)
+{
+	int i;
+
+	for (i = 0; i < free->npages; ++i)
+		if (free->phys_mem[i])
+			__free_page(free->phys_mem[i]);
+}
+
+/*
+ * Free any memory in @free but not in @dont.
+ */
+static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
+				  struct kvm_memory_slot *dont)
+{
+	if (!dont || free->phys_mem != dont->phys_mem)
+		if (free->phys_mem) {
+			if (free->user_alloc)
+				kvm_free_userspace_physmem(free);
+			else
+				kvm_free_kernel_physmem(free);
+			vfree(free->phys_mem);
+		}
+	if (!dont || free->rmap != dont->rmap)
+		vfree(free->rmap);
+
+	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
+		vfree(free->dirty_bitmap);
+
+	free->phys_mem = NULL;
+	free->npages = 0;
+	free->dirty_bitmap = NULL;
+}
+
+static void kvm_free_physmem(struct kvm *kvm)
+{
+	int i;
+
+	for (i = 0; i < kvm->nmemslots; ++i)
+		kvm_free_physmem_slot(&kvm->memslots[i], NULL);
+}
+
+/*
+ * Allocate some memory and give it an address in the guest physical address
+ * space.
+ *
+ * Discontiguous memory is allowed, mostly for framebuffers.
+ */
+static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
+					  struct
+					  kvm_userspace_memory_region *mem,
+					  int user_alloc)
+{
+	int r;
+	gfn_t base_gfn;
+	unsigned long npages;
+	unsigned long i;
+	struct kvm_memory_slot *memslot;
+	struct kvm_memory_slot old, new;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (mem->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (mem->slot >= KVM_MEMORY_SLOTS)
+		goto out;
+	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
+		goto out;
+
+	memslot = &kvm->memslots[mem->slot];
+	base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
+	npages = mem->memory_size >> PAGE_SHIFT;
+
+	if (!npages)
+		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
+
+	mutex_lock(&kvm->lock);
+
+	new = old = *memslot;
+
+	new.base_gfn = base_gfn;
+	new.npages = npages;
+	new.flags = mem->flags;
+
+	/* Disallow changing a memory slot's size. */
+	r = -EINVAL;
+	if (npages && old.npages && npages != old.npages)
+		goto out_unlock;
+
+	/* Check for overlaps */
+	r = -EEXIST;
+	for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
+		struct kvm_memory_slot *s = &kvm->memslots[i];
+
+		if (s == memslot)
+			continue;
+		if (!((base_gfn + npages <= s->base_gfn) ||
+		      (base_gfn >= s->base_gfn + s->npages)))
+			goto out_unlock;
+	}
+
+	/* Deallocate if slot is being removed */
+	if (!npages)
+		new.phys_mem = NULL;
+
+	/* Free page dirty bitmap if unneeded */
+	if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
+		new.dirty_bitmap = NULL;
+
+	r = -ENOMEM;
+
+	/* Allocate if a slot is being created */
+	if (npages && !new.phys_mem) {
+		new.phys_mem = vmalloc(npages * sizeof(struct page *));
+
+		if (!new.phys_mem)
+			goto out_unlock;
+
+		new.rmap = vmalloc(npages * sizeof(struct page *));
+
+		if (!new.rmap)
+			goto out_unlock;
+
+		memset(new.phys_mem, 0, npages * sizeof(struct page *));
+		memset(new.rmap, 0, npages * sizeof(*new.rmap));
+		if (user_alloc) {
+			unsigned long pages_num;
+
+			new.user_alloc = 1;
+			down_read(&current->mm->mmap_sem);
+
+			pages_num = get_user_pages(current, current->mm,
+						   mem->userspace_addr,
+						   npages, 1, 0, new.phys_mem,
+						   NULL);
+
+			up_read(&current->mm->mmap_sem);
+			if (pages_num != npages)
+				goto out_unlock;
+		} else {
+			for (i = 0; i < npages; ++i) {
+				new.phys_mem[i] = alloc_page(GFP_HIGHUSER
+							     | __GFP_ZERO);
+				if (!new.phys_mem[i])
+					goto out_unlock;
+			}
+		}
+	}
+
+	/* Allocate page dirty bitmap if needed */
+	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
+		unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
+
+		new.dirty_bitmap = vmalloc(dirty_bytes);
+		if (!new.dirty_bitmap)
+			goto out_unlock;
+		memset(new.dirty_bitmap, 0, dirty_bytes);
+	}
+
+	if (mem->slot >= kvm->nmemslots)
+		kvm->nmemslots = mem->slot + 1;
+
+	if (!kvm->n_requested_mmu_pages) {
+		unsigned int n_pages;
+
+		if (npages) {
+			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
+			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
+						 n_pages);
+		} else {
+			unsigned int nr_mmu_pages;
+
+			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
+			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
+			nr_mmu_pages = max(nr_mmu_pages,
+				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
+			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
+		}
+	}
+
+	*memslot = new;
+
+	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
+	kvm_flush_remote_tlbs(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	kvm_free_physmem_slot(&old, &new);
+	return 0;
+
+out_unlock:
+	mutex_unlock(&kvm->lock);
+	kvm_free_physmem_slot(&new, &old);
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
+					  u32 kvm_nr_mmu_pages)
+{
+	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
+		return -EINVAL;
+
+	mutex_lock(&kvm->lock);
+
+	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
+	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
+
+	mutex_unlock(&kvm->lock);
+	return 0;
+}
+
+static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
+{
+	return kvm->n_alloc_mmu_pages;
+}
+
+/*
+ * Set a new alias region.  Aliases map a portion of physical memory into
+ * another portion.  This is useful for memory windows, for example the PC
+ * VGA region.
+ */
+static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
+					 struct kvm_memory_alias *alias)
+{
+	int r, n;
+	struct kvm_mem_alias *p;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (alias->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->slot >= KVM_ALIAS_SLOTS)
+		goto out;
+	if (alias->guest_phys_addr + alias->memory_size
+	    < alias->guest_phys_addr)
+		goto out;
+	if (alias->target_phys_addr + alias->memory_size
+	    < alias->target_phys_addr)
+		goto out;
+
+	mutex_lock(&kvm->lock);
+
+	p = &kvm->aliases[alias->slot];
+	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
+	p->npages = alias->memory_size >> PAGE_SHIFT;
+	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
+
+	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
+		if (kvm->aliases[n - 1].npages)
+			break;
+	kvm->naliases = n;
+
+	kvm_mmu_zap_all(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	return 0;
+
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[0],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[1],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(&chip->chip.ioapic,
+			ioapic_irqchip(kvm),
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	return r;
+}
+
+static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&pic_irqchip(kvm)->pics[0],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&pic_irqchip(kvm)->pics[1],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(ioapic_irqchip(kvm),
+			&chip->chip.ioapic,
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	kvm_pic_update_irq(pic_irqchip(kvm));
+	return r;
+}
+
+long kvm_arch_vm_ioctl(struct file *filp,
+		       unsigned int ioctl, unsigned long arg)
+{
+	struct kvm *kvm = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	int r = -EINVAL;
+
+	switch(ioctl) {
+	case KVM_SET_MEMORY_REGION: {
+		struct kvm_memory_region kvm_mem;
+		struct kvm_userspace_memory_region kvm_userspace_mem;
+
+		r = -EFAULT;
+		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
+			goto out;
+		kvm_userspace_mem.slot = kvm_mem.slot;
+		kvm_userspace_mem.flags = kvm_mem.flags;
+		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
+		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
+		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_USER_MEMORY_REGION: {
+		struct kvm_userspace_memory_region kvm_userspace_mem;
+
+		r = -EFAULT;
+		if (copy_from_user(&kvm_userspace_mem, argp,
+						sizeof kvm_userspace_mem))
+			goto out;
+
+		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 1);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
+		if (r)
+			goto out;
+		break;
+	case KVM_GET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
+		break;
+	case KVM_SET_MEMORY_ALIAS: {
+		struct kvm_memory_alias alias;
+
+		r = -EFAULT;
+		if (copy_from_user(&alias, argp, sizeof alias))
+			goto out;
+		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_CREATE_IRQCHIP:
+		r = -ENOMEM;
+		kvm->vpic = kvm_create_pic(kvm);
+		if (kvm->vpic) {
+			r = kvm_ioapic_init(kvm);
+			if (r) {
+				kfree(kvm->vpic);
+				kvm->vpic = NULL;
+				goto out;
+			}
+		} else
+			goto out;
+		break;
+	case KVM_IRQ_LINE: {
+		struct kvm_irq_level irq_event;
+
+		r = -EFAULT;
+		if (copy_from_user(&irq_event, argp, sizeof irq_event))
+			goto out;
+		if (irqchip_in_kernel(kvm)) {
+			mutex_lock(&kvm->lock);
+			if (irq_event.irq < 16)
+				kvm_pic_set_irq(pic_irqchip(kvm),
+					irq_event.irq,
+					irq_event.level);
+			kvm_ioapic_set_irq(kvm->vioapic,
+					irq_event.irq,
+					irq_event.level);
+			mutex_unlock(&kvm->lock);
+			r = 0;
+		}
+		break;
+	}
+	case KVM_GET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(argp, &chip, sizeof chip))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_SET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = 0;
+		break;
+	}
+	}
+out:
+	return r;
+}
+
+void kvm_arch_destroy_vm(struct kvm *kvm)
+{
+	kvm_free_physmem(kvm);
+}
+
 static __init void kvm_init_msr_list(void)
 {
 	u32 dummy[2];



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Anthony Liguori]

Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
>
> Common ioctls for all architectures are:
> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG
>
> I'd really like to see more commonalities, but all others did not fit
> our needs. I would love to keep KVM_GET_DIRTY_LOG common, so that the
> ingenious migration code does not need to care too much about different
> architectures.
>
> x86 specific ioctls are:
> KVM_SET_MEMORY_REGION, KVM_SET_USER_MEMORY_REGION,
> KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
> KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP
>
> While the pic/apic related functions are obviously x86 specific, some
> other ioctls seem to be common at a first glance.
> KVM_SET_(USER)_MEMORY_REGION for example. We've got a total different
> address layout on s390: we cannot support multiple slots, and a user
> memory range always equals the guest physical memory [guest_phys + vm
> specific offset = host user address]. We don't have nor need dedicated
> vmas for the guest memory, we just use what the memory managment has in
> stock. This is true, because we reuse the page table for user and guest
> mode.
>   

You still need to tell the kernel about vm specific offset right?  So 
doesn't KVM_SET_USER_MEMORY_REGION for you just become that?  There's 
nothing wrong with s390 not supporting multiple memory slots, but 
there's no reason the ioctl interface can't be the same.

Regards,

Anthony Liguori

> Looks to me like the s390 might have a lot in common with a future AMD
> nested page table implementation. If AMD choose to reuse the page table
> too, we might share the same ioctl to set up guest addressing with them.
>
> signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> reviewed-by: Christian Borntraeger <borntraeger-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> reviewed-by: Christian Ehrhardt <ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Zhang, Xiantao]

Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c.
> 
> Common ioctls for all architectures are:
> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG
> 
> I'd really like to see more commonalities, but all others did not fit
> our needs. I would love to keep KVM_GET_DIRTY_LOG common, so that the
> ingenious migration code does not need to care too much about
> different architectures.

> x86 specific ioctls are:
> KVM_SET_MEMORY_REGION, KVM_SET_USER_MEMORY_REGION,
> KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
> KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP

I don't know why we not put KVM_SET_MEMORY_REGION,
KVM_SET_USER_MEMORY_REGION as common, although 
I have read the reasons you listed. I think they should work for most of
archs, although it is not very friendly with s390.  If we put them 
as arch-specific ones, we have to  duplicate many copies for them in KVM
code. 

One suggestion:  Maybe we can comment out current memory allocation
logic in userspace for S390, and s390 use your apporach to get its
memory.



> While the pic/apic related functions are obviously x86 specific, some
> other ioctls seem to be common at a first glance.
> KVM_SET_(USER)_MEMORY_REGION for example. We've got a total different
> address layout on s390: we cannot support multiple slots, and a user
> memory range always equals the guest physical memory [guest_phys + vm
> specific offset = host user address]. We don't have nor need dedicated
> vmas for the guest memory, we just use what the memory managment has
> in 
> stock. This is true, because we reuse the page table for user and
> guest 
> mode.
> Looks to me like the s390 might have a lot in common with a future AMD
> nested page table implementation. If AMD choose to reuse the page
> table 
> too, we might share the same ioctl to set up guest addressing with
> them. 



>
------------------------------------------------------------------------
-
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a
> browser. Download your FREE copy of Splunk now >>
> http://get.splunk.com/ _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Carsten Otte]

Anthony Liguori wrote:
>> While the pic/apic related functions are obviously x86 specific, some
>> other ioctls seem to be common at a first glance.
>> KVM_SET_(USER)_MEMORY_REGION for example. We've got a total different
>> address layout on s390: we cannot support multiple slots, and a user
>> memory range always equals the guest physical memory [guest_phys + vm
>> specific offset = host user address]. We don't have nor need dedicated
>> vmas for the guest memory, we just use what the memory managment has in
>> stock. This is true, because we reuse the page table for user and guest
>> mode.
>>   
> 
> You still need to tell the kernel about vm specific offset right?  So 
> doesn't KVM_SET_USER_MEMORY_REGION for you just become that?  There's 
> nothing wrong with s390 not supporting multiple memory slots, but 
> there's no reason the ioctl interface can't be the same.

I've though about that too. The thing is, the interface would really 
do something different on s390. I think it's more confusing to 
userland to have one interface that does two different things 
depending on the architecture rather then having different interfaces 
for different things.
You're right that KVM_SET_USER_MEMORY_REGION would cover our needs if 
we return -EINVAL in case slot != 0 or guest start address != 0. I'll 
talk it though with Martin on monday.

Carsten



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Carsten Otte]

Zhang, Xiantao wrote:
> I don't know why we not put KVM_SET_MEMORY_REGION,
> KVM_SET_USER_MEMORY_REGION as common, although 
> I have read the reasons you listed. I think they should work for most of
> archs, although it is not very friendly with s390.  If we put them 
> as arch-specific ones, we have to  duplicate many copies for them in KVM
> code. 
On s390, we use regular userspace memory to back our guest. Our 
architecture allows us to specify an offset and an address limit for 
the guest, and we don't need to have shaddow page tables and other 
tricks. We just use the userspace page table, and the guest memory is 
swapable to disk.

> One suggestion:  Maybe we can comment out current memory allocation
> logic in userspace for S390, and s390 use your apporach to get its
> memory.
Userspace will definetly need to do a special case for setting up the 
guest memory anyway due to major architectural differences. Thus I 
think it is fair to let it use two different ioctls for each case.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Avi Kivity]

Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
>
> Common ioctls for all architectures are:
> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG
>
> I'd really like to see more commonalities, but all others did not fit
> our needs. I would love to keep KVM_GET_DIRTY_LOG common, so that the
> ingenious migration code does not need to care too much about different
> architectures.
>
> x86 specific ioctls are:
> KVM_SET_MEMORY_REGION, KVM_SET_USER_MEMORY_REGION,
> KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
> KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP
>   

We need to distinguish between x86 specific (only x86 has this) and s390
special (everyone has it except s390). In the latter case it should
still be in common code to avoid duplication, with a guard to disable
compilation on s390.

KVM_SET_MEMORY_REGION is in theory applicable to non-s390 but I'd like
to deprecate it, so there's no point in implementing it on non-x86. The
rest are s390 special rather than x86 specific.

As related previously, KVM_SET_USER_MEMORY_REGION should work for s390
(with an extra check for the guest phsyical start address).

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Carsten Otte]

Avi Kivity wrote:
> We need to distinguish between x86 specific (only x86 has this) and s390
> special (everyone has it except s390). In the latter case it should
> still be in common code to avoid duplication, with a guard to disable
> compilation on s390.
> 
> KVM_SET_MEMORY_REGION is in theory applicable to non-s390 but I'd like
> to deprecate it, so there's no point in implementing it on non-x86. The
> rest are s390 special rather than x86 specific.
> 
> As related previously, KVM_SET_USER_MEMORY_REGION should work for s390
> (with an extra check for the guest phsyical start address).

That sounds reasonable to me. I'll make a patch that does this three 
way split on Monday. Let's see how it comes out.
KVM_SET_USER_MEMORY_REGION, and KVM_SET_MEMORY_REGION will go back to 
common, the irq related ones to a third place.

thanks for reviewing,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Carsten Otte]

Avi Kivity wrote:
> We need to distinguish between x86 specific (only x86 has this) and s390
> special (everyone has it except s390). In the latter case it should
> still be in common code to avoid duplication, with a guard to disable
> compilation on s390.
> 
> KVM_SET_MEMORY_REGION is in theory applicable to non-s390 but I'd like
> to deprecate it, so there's no point in implementing it on non-x86. The
> rest are s390 special rather than x86 specific.
> 
> As related previously, KVM_SET_USER_MEMORY_REGION should work for s390
> (with an extra check for the guest phsyical start address).
I thought about that all through the weekend. To me, it looks like I 
want to eliminate "s390 special" as far as possible. In given case, 
I'll follow Anthonys suggestion and support 
KVM_SET_USER_MEMORY_REGION. KVM_SET_MEMORY_REGION will also go common, 
and in case we're pushing the actual port before you deprecate that 
call, we'll #ifdef CONFIG_ARCH_S390 around it.

As for the pic/apic part, I think it is x86 specific. Christian 
Ehrhardt stated he believes ppc won't have that too.

Will create another patch on vm_ioctl that reflects this later today.

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Christian Ehrhardt]

Carsten Otte wrote:
> 
> Avi Kivity wrote:
>> We need to distinguish between x86 specific (only x86 has this) and s390
>> special (everyone has it except s390). In the latter case it should
>> still be in common code to avoid duplication, with a guard to disable
>> compilation on s390.
>>
>> KVM_SET_MEMORY_REGION is in theory applicable to non-s390 but I'd like
>> to deprecate it, so there's no point in implementing it on non-x86. The
>> rest are s390 special rather than x86 specific.
>>
>> As related previously, KVM_SET_USER_MEMORY_REGION should work for s390
>> (with an extra check for the guest phsyical start address).
> I thought about that all through the weekend. To me, it looks like I 
> want to eliminate "s390 special" as far as possible. In given case, 
> I'll follow Anthonys suggestion and support 
> KVM_SET_USER_MEMORY_REGION. KVM_SET_MEMORY_REGION will also go common, 
> and in case we're pushing the actual port before you deprecate that 
> call, we'll #ifdef CONFIG_ARCH_S390 around it.
> 
> As for the pic/apic part, I think it is x86 specific. Christian 
> Ehrhardt stated he believes ppc won't have that too.

At least not in that manner x86 has it atm (which would be  enough for
splitting it per arch), since I'm unsure in that topic I set Hollis on cc
who should know it better with his larger power experience.

> 
> Will create another patch on vm_ioctl that reflects this later today.
> 
> so long,
> Carsten
> 
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel


-- 

Grüsse / regards, 
Christian Ehrhardt

IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
Ehrhardt-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl

[Avi Kivity]

Carsten Otte wrote:
>
>
> Avi Kivity wrote:
>> We need to distinguish between x86 specific (only x86 has this) and s390
>> special (everyone has it except s390). In the latter case it should
>> still be in common code to avoid duplication, with a guard to disable
>> compilation on s390.
>>
>> KVM_SET_MEMORY_REGION is in theory applicable to non-s390 but I'd like
>> to deprecate it, so there's no point in implementing it on non-x86. The
>> rest are s390 special rather than x86 specific.
>>
>> As related previously, KVM_SET_USER_MEMORY_REGION should work for s390
>> (with an extra check for the guest phsyical start address).
> I thought about that all through the weekend. To me, it looks like I 
> want to eliminate "s390 special" as far as possible. In given case, 
> I'll follow Anthonys suggestion and support 
> KVM_SET_USER_MEMORY_REGION. KVM_SET_MEMORY_REGION will also go common, 
> and in case 

While KVM_SET_MEMORY_REGION can be supported on other archs, there's no 
reason to do so.  It will be supported on x86 for backward compatibility 
but newer ports need only support the newer APIs.

>
> As for the pic/apic part, I think it is x86 specific. Christian 
> Ehrhardt stated he believes ppc won't have that too.
>

Yes, it's x86 specific.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

This patch splits kvm_vm_ioctl into archtecture independent parts, and
x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 

Common ioctls for all architectures are:
KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION

KVM_SET_USER_MEMORY_REGION is actually implemented in x86.c now, because
the code behind looks arch specific to me. The ioctl however is generic
as suggested by Avi's review feedback.

x86 specific ioctls are:
KVM_SET_MEMORY_REGION, 
KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP

Signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
 kvm.h      |    7 +
 kvm_main.c |  402 -----------------------------------------------------------
 x86.c      |  415 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 426 insertions(+), 398 deletions(-)
---
diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index 08b5b21..a2f2536 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -610,6 +610,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg);
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
+int kvm_arch_vm_ioctl_set_memory_region(struct kvm *kvm,
+					struct kvm_userspace_memory_region
+					*mem,
+					int user_alloc);
+long kvm_arch_vm_ioctl(struct file *filp,
+		       unsigned int ioctl, unsigned long arg);
+void kvm_arch_destroy_vm(struct kvm *kvm);
 
 __init void kvm_arch_init(void);
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 6f7b31e..09a0826 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -42,7 +42,6 @@
 #include <linux/profile.h>
 #include <linux/kvm_para.h>
 #include <linux/pagemap.h>
-#include <linux/mman.h>
 
 #include <asm/processor.h>
 #include <asm/msr.h>
@@ -301,31 +300,6 @@ static struct kvm *kvm_create_vm(void)
 	return kvm;
 }
 
-/*
- * Free any memory in @free but not in @dont.
- */
-static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
-				  struct kvm_memory_slot *dont)
-{
-	if (!dont || free->rmap != dont->rmap)
-		vfree(free->rmap);
-
-	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
-		vfree(free->dirty_bitmap);
-
-	free->npages = 0;
-	free->dirty_bitmap = NULL;
-	free->rmap = NULL;
-}
-
-static void kvm_free_physmem(struct kvm *kvm)
-{
-	int i;
-
-	for (i = 0; i < kvm->nmemslots; ++i)
-		kvm_free_physmem_slot(&kvm->memslots[i], NULL);
-}
-
 static void free_pio_guest_pages(struct kvm_vcpu *vcpu)
 {
 	int i;
@@ -373,7 +347,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	kfree(kvm->vpic);
 	kfree(kvm->vioapic);
 	kvm_free_vcpus(kvm);
-	kvm_free_physmem(kvm);
+	kvm_arch_destroy_vm(kvm);
 	kfree(kvm);
 }
 
@@ -638,166 +612,6 @@ void fx_init(struct kvm_vcpu *vcpu)
 EXPORT_SYMBOL_GPL(fx_init);
 
 /*
- * Allocate some memory and give it an address in the guest physical address
- * space.
- *
- * Discontiguous memory is allowed, mostly for framebuffers.
- */
-static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
-					  struct
-					  kvm_userspace_memory_region *mem,
-					  int user_alloc)
-{
-	int r;
-	gfn_t base_gfn;
-	unsigned long npages;
-	unsigned long i;
-	struct kvm_memory_slot *memslot;
-	struct kvm_memory_slot old, new;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (mem->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (mem->slot >= KVM_MEMORY_SLOTS)
-		goto out;
-	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
-		goto out;
-
-	memslot = &kvm->memslots[mem->slot];
-	base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
-	npages = mem->memory_size >> PAGE_SHIFT;
-
-	if (!npages)
-		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
-
-	mutex_lock(&kvm->lock);
-
-	new = old = *memslot;
-
-	new.base_gfn = base_gfn;
-	new.npages = npages;
-	new.flags = mem->flags;
-
-	/* Disallow changing a memory slot's size. */
-	r = -EINVAL;
-	if (npages && old.npages && npages != old.npages)
-		goto out_unlock;
-
-	/* Check for overlaps */
-	r = -EEXIST;
-	for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
-		struct kvm_memory_slot *s = &kvm->memslots[i];
-
-		if (s == memslot)
-			continue;
-		if (!((base_gfn + npages <= s->base_gfn) ||
-		      (base_gfn >= s->base_gfn + s->npages)))
-			goto out_unlock;
-	}
-
-	/* Free page dirty bitmap if unneeded */
-	if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
-		new.dirty_bitmap = NULL;
-
-	r = -ENOMEM;
-
-	/* Allocate if a slot is being created */
-	if (npages && !new.rmap) {
-		new.rmap = vmalloc(npages * sizeof(struct page *));
-
-		if (!new.rmap)
-			goto out_unlock;
-
-		memset(new.rmap, 0, npages * sizeof(*new.rmap));
-
-		if (user_alloc)
-			new.userspace_addr = mem->userspace_addr;
-		else {
-			down_write(&current->mm->mmap_sem);
-			new.userspace_addr = do_mmap(NULL, 0,
-						     npages * PAGE_SIZE,
-						     PROT_READ | PROT_WRITE,
-						     MAP_SHARED | MAP_ANONYMOUS,
-						     0);
-			up_write(&current->mm->mmap_sem);
-
-			if (IS_ERR((void *)new.userspace_addr))
-				goto out_unlock;
-		}
-	}
-
-	/* Allocate page dirty bitmap if needed */
-	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
-		unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
-
-		new.dirty_bitmap = vmalloc(dirty_bytes);
-		if (!new.dirty_bitmap)
-			goto out_unlock;
-		memset(new.dirty_bitmap, 0, dirty_bytes);
-	}
-
-	if (mem->slot >= kvm->nmemslots)
-		kvm->nmemslots = mem->slot + 1;
-
-	if (!kvm->n_requested_mmu_pages) {
-		unsigned int n_pages;
-
-		if (npages) {
-			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
-						 n_pages);
-		} else {
-			unsigned int nr_mmu_pages;
-
-			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
-			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
-			nr_mmu_pages = max(nr_mmu_pages,
-				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
-			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
-		}
-	}
-
-	*memslot = new;
-
-	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
-	kvm_flush_remote_tlbs(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	kvm_free_physmem_slot(&old, &new);
-	return 0;
-
-out_unlock:
-	mutex_unlock(&kvm->lock);
-	kvm_free_physmem_slot(&new, &old);
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
-					  u32 kvm_nr_mmu_pages)
-{
-	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
-		return -EINVAL;
-
-	mutex_lock(&kvm->lock);
-
-	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
-	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
-
-	mutex_unlock(&kvm->lock);
-	return 0;
-}
-
-static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
-{
-	return kvm->n_alloc_mmu_pages;
-}
-
-/*
  * Get (and clear) the dirty memory log for a memory slot.
  */
 static int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
@@ -842,111 +656,6 @@ out:
 	return r;
 }
 
-/*
- * Set a new alias region.  Aliases map a portion of physical memory into
- * another portion.  This is useful for memory windows, for example the PC
- * VGA region.
- */
-static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
-					 struct kvm_memory_alias *alias)
-{
-	int r, n;
-	struct kvm_mem_alias *p;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (alias->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->slot >= KVM_ALIAS_SLOTS)
-		goto out;
-	if (alias->guest_phys_addr + alias->memory_size
-	    < alias->guest_phys_addr)
-		goto out;
-	if (alias->target_phys_addr + alias->memory_size
-	    < alias->target_phys_addr)
-		goto out;
-
-	mutex_lock(&kvm->lock);
-
-	p = &kvm->aliases[alias->slot];
-	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
-	p->npages = alias->memory_size >> PAGE_SHIFT;
-	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
-
-	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
-		if (kvm->aliases[n - 1].npages)
-			break;
-	kvm->naliases = n;
-
-	kvm_mmu_zap_all(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	return 0;
-
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[0],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[1],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(&chip->chip.ioapic,
-			ioapic_irqchip(kvm),
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	return r;
-}
-
-static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&pic_irqchip(kvm)->pics[0],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&pic_irqchip(kvm)->pics[1],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(ioapic_irqchip(kvm),
-			&chip->chip.ioapic,
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	kvm_pic_update_irq(pic_irqchip(kvm));
-	return r;
-}
-
 int is_error_page(struct page *page)
 {
 	return page == bad_page;
@@ -2921,7 +2630,7 @@ static long kvm_vm_ioctl(struct file *filp,
 {
 	struct kvm *kvm = filp->private_data;
 	void __user *argp = (void __user *)arg;
-	int r = -EINVAL;
+	int r;
 
 	switch (ioctl) {
 	case KVM_CREATE_VCPU:
@@ -2929,22 +2638,6 @@ static long kvm_vm_ioctl(struct file *filp,
 		if (r < 0)
 			goto out;
 		break;
-	case KVM_SET_MEMORY_REGION: {
-		struct kvm_memory_region kvm_mem;
-		struct kvm_userspace_memory_region kvm_userspace_mem;
-
-		r = -EFAULT;
-		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
-			goto out;
-		kvm_userspace_mem.slot = kvm_mem.slot;
-		kvm_userspace_mem.flags = kvm_mem.flags;
-		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
-		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
-		if (r)
-			goto out;
-		break;
-	}
 	case KVM_SET_USER_MEMORY_REGION: {
 		struct kvm_userspace_memory_region kvm_userspace_mem;
 
@@ -2953,19 +2646,11 @@ static long kvm_vm_ioctl(struct file *filp,
 						sizeof kvm_userspace_mem))
 			goto out;
 
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 1);
+		r = kvm_arch_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 1);
 		if (r)
 			goto out;
 		break;
 	}
-	case KVM_SET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
-		if (r)
-			goto out;
-		break;
-	case KVM_GET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
-		break;
 	case KVM_GET_DIRTY_LOG: {
 		struct kvm_dirty_log log;
 
@@ -2977,87 +2662,8 @@ static long kvm_vm_ioctl(struct file *filp,
 			goto out;
 		break;
 	}
-	case KVM_SET_MEMORY_ALIAS: {
-		struct kvm_memory_alias alias;
-
-		r = -EFAULT;
-		if (copy_from_user(&alias, argp, sizeof alias))
-			goto out;
-		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_CREATE_IRQCHIP:
-		r = -ENOMEM;
-		kvm->vpic = kvm_create_pic(kvm);
-		if (kvm->vpic) {
-			r = kvm_ioapic_init(kvm);
-			if (r) {
-				kfree(kvm->vpic);
-				kvm->vpic = NULL;
-				goto out;
-			}
-		} else
-			goto out;
-		break;
-	case KVM_IRQ_LINE: {
-		struct kvm_irq_level irq_event;
-
-		r = -EFAULT;
-		if (copy_from_user(&irq_event, argp, sizeof irq_event))
-			goto out;
-		if (irqchip_in_kernel(kvm)) {
-			mutex_lock(&kvm->lock);
-			if (irq_event.irq < 16)
-				kvm_pic_set_irq(pic_irqchip(kvm),
-					irq_event.irq,
-					irq_event.level);
-			kvm_ioapic_set_irq(kvm->vioapic,
-					irq_event.irq,
-					irq_event.level);
-			mutex_unlock(&kvm->lock);
-			r = 0;
-		}
-		break;
-	}
-	case KVM_GET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(argp, &chip, sizeof chip))
-			goto out;
-		r = 0;
-		break;
-	}
-	case KVM_SET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = 0;
-		break;
-	}
 	default:
-		;
+		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
index 1fe209d..4e786f6 100644
--- a/drivers/kvm/x86.c
+++ b/drivers/kvm/x86.c
@@ -21,6 +21,7 @@
 #include <linux/kvm.h>
 #include <linux/fs.h>
 #include <linux/vmalloc.h>
+#include <linux/mman.h>
 
 #include <asm/uaccess.h>
 
@@ -300,6 +301,420 @@ out:
 	return r;
 }
 
+/*
+ * Free any memory in @free but not in @dont.
+ */
+static void kvm_free_physmem_slot(struct kvm_memory_slot *free,
+				  struct kvm_memory_slot *dont)
+{
+	if (!dont || free->rmap != dont->rmap)
+		vfree(free->rmap);
+
+	if (!dont || free->dirty_bitmap != dont->dirty_bitmap)
+		vfree(free->dirty_bitmap);
+
+	free->npages = 0;
+	free->dirty_bitmap = NULL;
+	free->rmap = NULL;
+}
+
+static void kvm_free_physmem(struct kvm *kvm)
+{
+	int i;
+
+	for (i = 0; i < kvm->nmemslots; ++i)
+		kvm_free_physmem_slot(&kvm->memslots[i], NULL);
+}
+
+/*
+ * Allocate some memory and give it an address in the guest physical address
+ * space.
+ *
+ * Discontiguous memory is allowed, mostly for framebuffers.
+ */
+int kvm_arch_vm_ioctl_set_memory_region(struct kvm *kvm,
+					struct kvm_userspace_memory_region
+					*mem,
+					int user_alloc)
+{
+	int r;
+	gfn_t base_gfn;
+	unsigned long npages;
+	unsigned long i;
+	struct kvm_memory_slot *memslot;
+	struct kvm_memory_slot old, new;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (mem->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (mem->slot >= KVM_MEMORY_SLOTS)
+		goto out;
+	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
+		goto out;
+
+	memslot = &kvm->memslots[mem->slot];
+	base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
+	npages = mem->memory_size >> PAGE_SHIFT;
+
+	if (!npages)
+		mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
+
+	mutex_lock(&kvm->lock);
+
+	new = old = *memslot;
+
+	new.base_gfn = base_gfn;
+	new.npages = npages;
+	new.flags = mem->flags;
+
+	/* Disallow changing a memory slot's size. */
+	r = -EINVAL;
+	if (npages && old.npages && npages != old.npages)
+		goto out_unlock;
+
+	/* Check for overlaps */
+	r = -EEXIST;
+	for (i = 0; i < KVM_MEMORY_SLOTS; ++i) {
+		struct kvm_memory_slot *s = &kvm->memslots[i];
+
+		if (s == memslot)
+			continue;
+		if (!((base_gfn + npages <= s->base_gfn) ||
+		      (base_gfn >= s->base_gfn + s->npages)))
+			goto out_unlock;
+	}
+
+	/* Free page dirty bitmap if unneeded */
+	if (!(new.flags & KVM_MEM_LOG_DIRTY_PAGES))
+		new.dirty_bitmap = NULL;
+
+	r = -ENOMEM;
+
+	/* Allocate if a slot is being created */
+	if (npages && !new.rmap) {
+		new.rmap = vmalloc(npages * sizeof(struct page *));
+
+		if (!new.rmap)
+			goto out_unlock;
+
+		memset(new.rmap, 0, npages * sizeof(*new.rmap));
+
+		if (user_alloc)
+			new.userspace_addr = mem->userspace_addr;
+		else {
+			down_write(&current->mm->mmap_sem);
+			new.userspace_addr = do_mmap(NULL, 0,
+						     npages * PAGE_SIZE,
+						     PROT_READ | PROT_WRITE,
+						     MAP_SHARED | MAP_ANONYMOUS,
+						     0);
+			up_write(&current->mm->mmap_sem);
+
+			if (IS_ERR((void *)new.userspace_addr))
+				goto out_unlock;
+		}
+	}
+
+	/* Allocate page dirty bitmap if needed */
+	if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
+		unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
+
+		new.dirty_bitmap = vmalloc(dirty_bytes);
+		if (!new.dirty_bitmap)
+			goto out_unlock;
+		memset(new.dirty_bitmap, 0, dirty_bytes);
+	}
+
+	if (mem->slot >= kvm->nmemslots)
+		kvm->nmemslots = mem->slot + 1;
+
+	if (!kvm->n_requested_mmu_pages) {
+		unsigned int n_pages;
+
+		if (npages) {
+			n_pages = npages * KVM_PERMILLE_MMU_PAGES / 1000;
+			kvm_mmu_change_mmu_pages(kvm, kvm->n_alloc_mmu_pages +
+						 n_pages);
+		} else {
+			unsigned int nr_mmu_pages;
+
+			n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / 1000;
+			nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages;
+			nr_mmu_pages = max(nr_mmu_pages,
+				        (unsigned int) KVM_MIN_ALLOC_MMU_PAGES);
+			kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages);
+		}
+	}
+
+	*memslot = new;
+
+	kvm_mmu_slot_remove_write_access(kvm, mem->slot);
+	kvm_flush_remote_tlbs(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	kvm_free_physmem_slot(&old, &new);
+	return 0;
+
+out_unlock:
+	mutex_unlock(&kvm->lock);
+	kvm_free_physmem_slot(&new, &old);
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
+					  u32 kvm_nr_mmu_pages)
+{
+	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
+		return -EINVAL;
+
+	mutex_lock(&kvm->lock);
+
+	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
+	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
+
+	mutex_unlock(&kvm->lock);
+	return 0;
+}
+
+static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
+{
+	return kvm->n_alloc_mmu_pages;
+}
+
+/*
+ * Set a new alias region.  Aliases map a portion of physical memory into
+ * another portion.  This is useful for memory windows, for example the PC
+ * VGA region.
+ */
+static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
+					 struct kvm_memory_alias *alias)
+{
+	int r, n;
+	struct kvm_mem_alias *p;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (alias->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->slot >= KVM_ALIAS_SLOTS)
+		goto out;
+	if (alias->guest_phys_addr + alias->memory_size
+	    < alias->guest_phys_addr)
+		goto out;
+	if (alias->target_phys_addr + alias->memory_size
+	    < alias->target_phys_addr)
+		goto out;
+
+	mutex_lock(&kvm->lock);
+
+	p = &kvm->aliases[alias->slot];
+	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
+	p->npages = alias->memory_size >> PAGE_SHIFT;
+	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
+
+	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
+		if (kvm->aliases[n - 1].npages)
+			break;
+	kvm->naliases = n;
+
+	kvm_mmu_zap_all(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	return 0;
+
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[0],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[1],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(&chip->chip.ioapic,
+			ioapic_irqchip(kvm),
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	return r;
+}
+
+static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&pic_irqchip(kvm)->pics[0],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&pic_irqchip(kvm)->pics[1],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(ioapic_irqchip(kvm),
+			&chip->chip.ioapic,
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	kvm_pic_update_irq(pic_irqchip(kvm));
+	return r;
+}
+
+long kvm_arch_vm_ioctl(struct file *filp,
+		       unsigned int ioctl, unsigned long arg)
+{
+	struct kvm *kvm = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	int r = -EINVAL;
+
+	switch (ioctl) {
+	case KVM_SET_MEMORY_REGION: {
+		struct kvm_memory_region kvm_mem;
+		struct kvm_userspace_memory_region kvm_userspace_mem;
+
+		r = -EFAULT;
+		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
+			goto out;
+		kvm_userspace_mem.slot = kvm_mem.slot;
+		kvm_userspace_mem.flags = kvm_mem.flags;
+		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
+		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
+		r = kvm_arch_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
+		if (r)
+			goto out;
+		break;
+	case KVM_GET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
+		break;
+	case KVM_SET_MEMORY_ALIAS: {
+		struct kvm_memory_alias alias;
+
+		r = -EFAULT;
+		if (copy_from_user(&alias, argp, sizeof alias))
+			goto out;
+		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_CREATE_IRQCHIP:
+		r = -ENOMEM;
+		kvm->vpic = kvm_create_pic(kvm);
+		if (kvm->vpic) {
+			r = kvm_ioapic_init(kvm);
+			if (r) {
+				kfree(kvm->vpic);
+				kvm->vpic = NULL;
+				goto out;
+			}
+		} else
+			goto out;
+		break;
+	case KVM_IRQ_LINE: {
+		struct kvm_irq_level irq_event;
+
+		r = -EFAULT;
+		if (copy_from_user(&irq_event, argp, sizeof irq_event))
+			goto out;
+		if (irqchip_in_kernel(kvm)) {
+			mutex_lock(&kvm->lock);
+			if (irq_event.irq < 16)
+				kvm_pic_set_irq(pic_irqchip(kvm),
+					irq_event.irq,
+					irq_event.level);
+			kvm_ioapic_set_irq(kvm->vioapic,
+					irq_event.irq,
+					irq_event.level);
+			mutex_unlock(&kvm->lock);
+			r = 0;
+		}
+		break;
+	}
+	case KVM_GET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(argp, &chip, sizeof chip))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_SET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = 0;
+		break;
+	}
+	default:
+		;
+	}
+out:
+	return r;
+}
+
+void kvm_arch_destroy_vm(struct kvm *kvm)
+{
+
+	kvm_free_physmem(kvm);
+}
+
 static __init void kvm_init_msr_list(void)
 {
 	u32 dummy[2];



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

On Thu, 2007-10-25 at 17:48 +0200, Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
> 
> Common ioctls for all architectures are:
> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
> 
> KVM_SET_USER_MEMORY_REGION is actually implemented in x86.c now, because
> the code behind looks arch specific to me. 
i think it is much better just to split the parts that allocate the
rmap, and the part that set the number of shadow pages mmu,
beside this parts it seems to me that it isnt arch specific.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Hollis Blanchard]

On Thu, 2007-10-25 at 17:48 +0200, Izik Eidus wrote:
> On Thu, 2007-10-25 at 17:48 +0200, Carsten Otte wrote:
> > This patch splits kvm_vm_ioctl into archtecture independent parts, and
> > x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
> > 
> > Common ioctls for all architectures are:
> > KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
> > 
> > KVM_SET_USER_MEMORY_REGION is actually implemented in x86.c now, because
> > the code behind looks arch specific to me. 

Reviewed-by: Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

> i think it is much better just to split the parts that allocate the
> rmap, and the part that set the number of shadow pages mmu,
> beside this parts it seems to me that it isnt arch specific.

Carsten omitted the explanation about memslots he had in his original
patch. To quote that here:
> We've got a total different
> address layout on s390: we cannot support multiple slots, and a user
> memory range always equals the guest physical memory [guest_phys + vm
> specific offset = host user address]. We don't have nor need dedicated
> vmas for the guest memory, we just use what the memory managment has
> in stock. This is true, because we reuse the page table for user and
> guest mode.

Given that explanation, and that kvm_vm_ioctl_set_memory_region() is
entirely about memslots, I'm inclined to agree with this code movement.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

On Thu, 2007-10-25 at 11:22 -0500, Hollis Blanchard wrote:
> On Thu, 2007-10-25 at 17:48 +0200, Izik Eidus wrote:
> > On Thu, 2007-10-25 at 17:48 +0200, Carsten Otte wrote:
> > > This patch splits kvm_vm_ioctl into archtecture independent parts, and
> > > x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
> > > 
> > > Common ioctls for all architectures are:
> > > KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
> > > 
> > > KVM_SET_USER_MEMORY_REGION is actually implemented in x86.c now, because
> > > the code behind looks arch specific to me. 
> 
> Reviewed-by: Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> 
> > i think it is much better just to split the parts that allocate the
> > rmap, and the part that set the number of shadow pages mmu,
> > beside this parts it seems to me that it isnt arch specific.
> 
> Carsten omitted the explanation about memslots he had in his original
> patch. To quote that here:
> > We've got a total different
> > address layout on s390: we cannot support multiple slots, and a user
> > memory range always equals the guest physical memory [guest_phys + vm
> > specific offset = host user address]. We don't have nor need dedicated
> > vmas for the guest memory, we just use what the memory managment has
> > in stock. This is true, because we reuse the page table for user and
> > guest mode.
> 
> Given that explanation, and that kvm_vm_ioctl_set_memory_region() is
> entirely about memslots, I'm inclined to agree with this code movement.
> 
after reading this, i agree.
sorry.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Hollis Blanchard]

On Fri, 2007-10-26 at 00:12 +0200, Izik Eidus wrote:
> ok i was thinking,
> maybe we can rewrite the way kvm hold memory so more code would be shared,
> lets say we throw away all the slots and arch depended stuff, and we let kvm
> just hold the userspace allocated memory address,

With that approach, how would you create a sparse (guest physical)
memory map in userspace? I guess you would need to use repeated
mmap(MAP_FIXED), and that's starting to look very brittle.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

Hollis Blanchard wrote:
> On Thu, 2007-10-25 at 17:48 +0200, Izik Eidus wrote:
>   
>> On Thu, 2007-10-25 at 17:48 +0200, Carsten Otte wrote:
>>     
>>> This patch splits kvm_vm_ioctl into archtecture independent parts, and
>>> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
>>>
>>> Common ioctls for all architectures are:
>>> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
>>>
>>> KVM_SET_USER_MEMORY_REGION is actually implemented in x86.c now, because
>>> the code behind looks arch specific to me. 
>>>       
>
> Reviewed-by: Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>
>   
>> i think it is much better just to split the parts that allocate the
>> rmap, and the part that set the number of shadow pages mmu,
>> beside this parts it seems to me that it isnt arch specific.
>>     
>
> Carsten omitted the explanation about memslots he had in his original
> patch. To quote that here:
>   
>> We've got a total different
>> address layout on s390: we cannot support multiple slots, and a user
>> memory range always equals the guest physical memory [guest_phys + vm
>> specific offset = host user address]. We don't have nor need dedicated
>> vmas for the guest memory, we just use what the memory managment has
>> in stock. This is true, because we reuse the page table for user and
>> guest mode.
>>     
>
> Given that explanation, and that kvm_vm_ioctl_set_memory_region() is
> entirely about memslots, I'm inclined to agree with this code movement.
>
>   
ok i was thinking,
maybe we can rewrite the way kvm hold memory so more code would be shared,
lets say we throw away all the slots and arch depended stuff, and we let kvm
just hold the userspace allocated memory address,

then we will will have to each arch "arch specific" functions that will 
map the memory as it will need.
for example for x86 we will make gfn_to_page map on the fly how the 
memory should look.
i think i will write patch to example this, but it might take me some time,

anyway what do you think about this idea?



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

Hollis Blanchard wrote:
> On Fri, 2007-10-26 at 00:12 +0200, Izik Eidus wrote:
>   
>> ok i was thinking,
>> maybe we can rewrite the way kvm hold memory so more code would be shared,
>> lets say we throw away all the slots and arch depended stuff, and we let kvm
>> just hold the userspace allocated memory address,
>>     
>
> With that approach, how would you create a sparse (guest physical)
> memory map in userspace? I guess you would need to use repeated
> mmap(MAP_FIXED), and that's starting to look very brittle.
>
>   
i would just allocate one single block of memory to kvm,
and then i will use ioctls to tell kvm how to map it.

after this the kernel will map back to the userspace the memory by the 
mmap system call,
this would be infact the same mechanisem that would be used by gfn_to_page
(infact the mmap syscall will call gfn_to_page)

so accessing memory from the userspace would skip "holes"....

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

Izik Eidus wrote:
> Hollis Blanchard wrote:
>> On Fri, 2007-10-26 at 00:12 +0200, Izik Eidus wrote:
>>  
>>> ok i was thinking,
>>> maybe we can rewrite the way kvm hold memory so more code would be 
>>> shared,
>>> lets say we throw away all the slots and arch depended stuff, and we 
>>> let kvm
>>> just hold the userspace allocated memory address,
>>>     
>>
>> With that approach, how would you create a sparse (guest physical)
>> memory map in userspace? I guess you would need to use repeated
>> mmap(MAP_FIXED), and that's starting to look very brittle.
>>
>>  
btw, if you look at kvmctl, we already do it,
so it is good question if it better to leave this work to userspace 
(like it do now)
or do the mapping to userspace from the kernel to userspace (using the 
mmap syscall)

(i like the secoend option beacuse it would be easier to work with qemu 
like this)
> i would just allocate one single block of memory to kvm,
> and then i will use ioctls to tell kvm how to map it.
>
> after this the kernel will map back to the userspace the memory by the 
> mmap system call,
> this would be infact the same mechanisem that would be used by 
> gfn_to_page
> (infact the mmap syscall will call gfn_to_page)
>
> so accessing memory from the userspace would skip "holes"....
>


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Zhang, Xiantao]

Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c.
> 
> Common ioctls for all architectures are:
> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
> 
> KVM_SET_USER_MEMORY_REGION is actually implemented in x86.c now,
Hi Carsten,

I don't think we can move the whole function to arch-specific part,
because  it should work well (or with few issues) for most archs.
Basically, IA64 mostly can use it directly. If we move them as
arch-specific, it will introduces many duplicates. As you said, S390 has
quite difference about this side, but I think maybe we can use macros,
such as #ifndef CONFIG_S390 to comment out them, and S390 define it in
your arch-specific portions. Any other good ideas ?

Thanks 
Xiantao

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Hollis Blanchard wrote:
> On Fri, 2007-10-26 at 00:12 +0200, Izik Eidus wrote:
>   
>> ok i was thinking,
>> maybe we can rewrite the way kvm hold memory so more code would be shared,
>> lets say we throw away all the slots and arch depended stuff, and we let kvm
>> just hold the userspace allocated memory address,
>>     
>
> With that approach, how would you create a sparse (guest physical)
> memory map in userspace? I guess you would need to use repeated
> mmap(MAP_FIXED), and that's starting to look very brittle.
>
>   

It can't work on i386 due to the limited host virtual address space.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Izik Eidus wrote:
> btw, if you look at kvmctl, we already do it,
> so it is good question if it better to leave this work to userspace 
> (like it do now)
> or do the mapping to userspace from the kernel to userspace (using the 
> mmap syscall)
> 
> (i like the secoend option beacuse it would be easier to work with qemu 
> like this)
I think we should leave it to userland. This was userland is free to 
use whatever it wants:
- malloc() to get anonymous memory
- mmap() to get file backed memory
- posix or sysv shared memory
- hugetlbfs to save translation steps

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Avi Kivity wrote:
> Hollis Blanchard wrote:
>> On Fri, 2007-10-26 at 00:12 +0200, Izik Eidus wrote:
>>   
>>> ok i was thinking,
>>> maybe we can rewrite the way kvm hold memory so more code would be shared,
>>> lets say we throw away all the slots and arch depended stuff, and we let kvm
>>> just hold the userspace allocated memory address,
>>>     
>> With that approach, how would you create a sparse (guest physical)
>> memory map in userspace? I guess you would need to use repeated
>> mmap(MAP_FIXED), and that's starting to look very brittle.
>>
>>   
> 
> It can't work on i386 due to the limited host virtual address space.
That's why memory allocation/preparation needs to be arch dependent: 
i386 needs a memory layout different from userspace page table due to 
your argument, and s390 needs to use the userspace page table due to 
hardware features we want to exploit.
As Xiantao pointed out, x86 and ia64 can share the same memory setup 
code. But s390 and ppc don't. Therefore, I vote for putting it into 
x86 for now, and come up with an approach to share between x86 and 
ia64 later on.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Izik Eidus wrote:
> ok i was thinking,
> maybe we can rewrite the way kvm hold memory so more code would be shared,
> lets say we throw away all the slots and arch depended stuff, and we let kvm
> just hold the userspace allocated memory address,
> 
> then we will will have to each arch "arch specific" functions that will 
> map the memory as it will need.
> for example for x86 we will make gfn_to_page map on the fly how the 
> memory should look.
> i think i will write patch to example this, but it might take me some time,
> 
> anyway what do you think about this idea?
I do strongly vote for it. This way, we'd have memory handling common 
over all architectures. For a sane end result of a real portably 
hypervisor, this is very preferable over "have every arch to its own 
thing". After reading your suggestion, I think memory allocation needs 
more engineering work to be done until we find a proper arch split 
then just move it to x86.c. Therefore, I'll modify the patch to leave 
memory handling in common for now. This way, the rest of the patch can 
be picked up for the time being.

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Carsten Otte wrote:
> That's why memory allocation/preparation needs to be arch dependent: 
> i386 needs a memory layout different from userspace page table due to 
> your argument, and s390 needs to use the userspace page table due to 
> hardware features we want to exploit.
> As Xiantao pointed out, x86 and ia64 can share the same memory setup 
> code. But s390 and ppc don't. Therefore, I vote for putting it into 
> x86 for now, and come up with an approach to share between x86 and 
> ia64 later on.
After reading Izik's idea: Maybe we should go Izik's way, and have an 
i386 special case that we can deprecate later on?

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>   
>> Hollis Blanchard wrote:
>>     
>>> On Fri, 2007-10-26 at 00:12 +0200, Izik Eidus wrote:
>>>   
>>>       
>>>> ok i was thinking,
>>>> maybe we can rewrite the way kvm hold memory so more code would be shared,
>>>> lets say we throw away all the slots and arch depended stuff, and we let kvm
>>>> just hold the userspace allocated memory address,
>>>>     
>>>>         
>>> With that approach, how would you create a sparse (guest physical)
>>> memory map in userspace? I guess you would need to use repeated
>>> mmap(MAP_FIXED), and that's starting to look very brittle.
>>>
>>>   
>>>       
>> It can't work on i386 due to the limited host virtual address space.
>>     
> That's why memory allocation/preparation needs to be arch dependent: 
> i386 needs a memory layout different from userspace page table due to 
> your argument, and s390 needs to use the userspace page table due to 
> hardware features we want to exploit.
> As Xiantao pointed out, x86 and ia64 can share the same memory setup 
> code. But s390 and ppc don't. Therefore, I vote for putting it into 
> x86 for now, and come up with an approach to share between x86 and 
> ia64 later on.
>   

But can't s390 and ppc use a subset?  If you limit the number of memory 
slots to one, it's equivalent to base + limit.

No?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Carsten Otte wrote:
> Carsten Otte wrote:
>   
>> That's why memory allocation/preparation needs to be arch dependent: 
>> i386 needs a memory layout different from userspace page table due to 
>> your argument, and s390 needs to use the userspace page table due to 
>> hardware features we want to exploit.
>> As Xiantao pointed out, x86 and ia64 can share the same memory setup 
>> code. But s390 and ppc don't. Therefore, I vote for putting it into 
>> x86 for now, and come up with an approach to share between x86 and 
>> ia64 later on.
>>     
> After reading Izik's idea: Maybe we should go Izik's way, and have an 
> i386 special case that we can deprecate later on?
>
>   

I don't really see a big difference between what we have now (sparse 
userspace, sparse guest) and Izik's idea (contiguous userspace, sparse 
guest).  In both cases you need something like memory slots to describe 
the different sections.

Moreover, on x86 you may want different properties for different 
sections (4K pages for the framebuffer, large pages for main memory), so 
you can't allocate memory in one big chunk.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Avi Kivity wrote:
> I don't really see a big difference between what we have now (sparse 
> userspace, sparse guest) and Izik's idea (contiguous userspace, sparse 
> guest).  In both cases you need something like memory slots to describe 
> the different sections.
We don't on s390: we receive a page fault by the guest once it 
accesses a sparse hole in its address space. We check the user space's 
VMA and either page it in or submit an addressing exception to the guest.

> Moreover, on x86 you may want different properties for different 
> sections (4K pages for the framebuffer, large pages for main memory), so 
> you can't allocate memory in one big chunk.
That's right. On s390, we can live with whatever properties a section 
has with regard to page size, backing device and such. So memory may 
well come to live by different alloations, and different allocation 
methods. All we need is a permanent contiguous mapping of the guest 
physical addresses to host user addresses. So Izik's idea would work 
for us even if we have different sections.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>> I don't really see a big difference between what we have now (sparse 
>> userspace, sparse guest) and Izik's idea (contiguous userspace, 
>> sparse guest).  In both cases you need something like memory slots to 
>> describe the different sections.
> We don't on s390: we receive a page fault by the guest once it 
> accesses a sparse hole in its address space. We check the user space's 
> VMA and either page it in or submit an addressing exception to the guest.

I was talking about x86.

On x86, you need contiguous userspace, contiguous guest, but again, 
what's the problem with one memory slot?


>
>> Moreover, on x86 you may want different properties for different 
>> sections (4K pages for the framebuffer, large pages for main memory), 
>> so you can't allocate memory in one big chunk.
> That's right. On s390, we can live with whatever properties a section 
> has with regard to page size, backing device and such. So memory may 
> well come to live by different alloations, and different allocation 
> methods. All we need is a permanent contiguous mapping of the guest 
> physical addresses to host user addresses. So Izik's idea would work 
> for us even if we have different sections.

So would the current memory slot thing, no?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Avi Kivity wrote:
> I was talking about x86.
> 
> On x86, you need contiguous userspace, contiguous guest, but again, 
> what's the problem with one memory slot?
There is no problem with one memory slot: it works. It's just that 
Izik's idea gives us the opportunity to have common code for all four 
architectures here.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>> I was talking about x86.
>>
>> On x86, you need contiguous userspace, contiguous guest, but again, 
>> what's the problem with one memory slot?
> There is no problem with one memory slot: it works. It's just that 
> Izik's idea gives us the opportunity to have common code for all four 
> architectures here.

Why aren't memory slots common too?  Only their number is different, 
while the implementation is the same.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Avi Kivity wrote:
> Why aren't memory slots common too?  Only their number is different, 
> while the implementation is the same.
Your approach makes the meaning of memory slot somewhat useless on 
s390, if that one may be sparse and may be result of different 
allocations: On x86, there has to be one memory slot per allocation, 
versus on s390 there has to be exactly one memory slot with multiple 
allocations behind.

For userspace that means, with your approach it has to do total 
different memory setup for different archs _if_ it wants to use 
multiple allocations and/or sparse:
- on x86 it does allocations to random userspace address, and 
registers each of them as memory slot
- on s390 it does allocations to a specific address layout similar to 
the guest, and registers only one memory slot for the whole thing

With Izik's approach however, this is transparent to userspace: it has 
multiple memory slots, one per allocation. Regardless of the CPU 
architecture.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>> Why aren't memory slots common too?  Only their number is different, 
>> while the implementation is the same.
> Your approach makes the meaning of memory slot somewhat useless on 
> s390, if that one may be sparse and may be result of different 
> allocations: On x86, there has to be one memory slot per allocation, 
> versus on s390 there has to be exactly one memory slot with multiple 
> allocations behind.

No, a memory slot can span multiple backing stores.  But it must be 
contiguous in both the host userspace and guest physical address spaces.

>
> For userspace that means, with your approach it has to do total 
> different memory setup for different archs _if_ it wants to use 
> multiple allocations and/or sparse:
> - on x86 it does allocations to random userspace address, and 
> registers each of them as memory slot
> - on s390 it does allocations to a specific address layout similar to 
> the guest, and registers only one memory slot for the whole thing
>
> With Izik's approach however, this is transparent to userspace: it has 
> multiple memory slots, one per allocation. Regardless of the CPU 
> architecture.

You can do this with the current memory slots as well.  Although I'm 
feeling that I misunderstood Izik's idea.  I'll go talk to him.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

This patch splits kvm_vm_ioctl into archtecture independent parts, and
x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
The patch has been updated to current git, and it leaves out memory slot
registration work which is currently subject to a detailed discussion.

Common ioctls for all architectures are:
KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION

KVM_SET_USER_MEMORY_REGION implementation is no longer moved to x86.c.
It seems to me that more fine-grained refinement then just moving the
code is required here.

x86 specific ioctls are:
KVM_SET_MEMORY_REGION, 
KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP
KVM_SET_TSS_ADDR

KVM_SET_TSS_ADDR has been added to the list of x86 specifics, as Izik's
commit states it is used for emulating real mode on intel.


 kvm.h      |    7 +
 kvm_main.c |  255
+-----------------------------------------------------------
 x86.c      |  258
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 271 insertions(+), 249 deletions(-)
Signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
---
diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
index b08fc9e..438d4a9 100644
--- a/drivers/kvm/kvm.h
+++ b/drivers/kvm/kvm.h
@@ -621,6 +621,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 			 unsigned int ioctl, unsigned long arg);
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
+int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
+				   struct
+				   kvm_userspace_memory_region *mem,
+				   int user_alloc);
+long kvm_arch_vm_ioctl(struct file *filp,
+		       unsigned int ioctl, unsigned long arg);
+void kvm_arch_destroy_vm(struct kvm *kvm);
 
 __init void kvm_arch_init(void);
 
diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
index 9a3d663..7a85be9 100644
--- a/drivers/kvm/kvm_main.c
+++ b/drivers/kvm/kvm_main.c
@@ -792,36 +792,16 @@ out:
 }
 EXPORT_SYMBOL_GPL(kvm_set_memory_region);
 
-static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
-					  struct
-					  kvm_userspace_memory_region *mem,
-					  int user_alloc)
+int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
+				   struct
+				   kvm_userspace_memory_region *mem,
+				   int user_alloc)
 {
 	if (mem->slot >= KVM_MEMORY_SLOTS)
 		return -EINVAL;
 	return kvm_set_memory_region(kvm, mem, user_alloc);
 }
 
-static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
-					  u32 kvm_nr_mmu_pages)
-{
-	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
-		return -EINVAL;
-
-	mutex_lock(&kvm->lock);
-
-	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
-	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
-
-	mutex_unlock(&kvm->lock);
-	return 0;
-}
-
-static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
-{
-	return kvm->n_alloc_mmu_pages;
-}
-
 /*
  * Get (and clear) the dirty memory log for a memory slot.
  */
@@ -867,111 +847,6 @@ out:
 	return r;
 }
 
-/*
- * Set a new alias region.  Aliases map a portion of physical memory into
- * another portion.  This is useful for memory windows, for example the PC
- * VGA region.
- */
-static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
-					 struct kvm_memory_alias *alias)
-{
-	int r, n;
-	struct kvm_mem_alias *p;
-
-	r = -EINVAL;
-	/* General sanity checks */
-	if (alias->memory_size & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
-		goto out;
-	if (alias->slot >= KVM_ALIAS_SLOTS)
-		goto out;
-	if (alias->guest_phys_addr + alias->memory_size
-	    < alias->guest_phys_addr)
-		goto out;
-	if (alias->target_phys_addr + alias->memory_size
-	    < alias->target_phys_addr)
-		goto out;
-
-	mutex_lock(&kvm->lock);
-
-	p = &kvm->aliases[alias->slot];
-	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
-	p->npages = alias->memory_size >> PAGE_SHIFT;
-	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
-
-	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
-		if (kvm->aliases[n - 1].npages)
-			break;
-	kvm->naliases = n;
-
-	kvm_mmu_zap_all(kvm);
-
-	mutex_unlock(&kvm->lock);
-
-	return 0;
-
-out:
-	return r;
-}
-
-static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[0],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&chip->chip.pic,
-			&pic_irqchip(kvm)->pics[1],
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(&chip->chip.ioapic,
-			ioapic_irqchip(kvm),
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	return r;
-}
-
-static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
-{
-	int r;
-
-	r = 0;
-	switch (chip->chip_id) {
-	case KVM_IRQCHIP_PIC_MASTER:
-		memcpy(&pic_irqchip(kvm)->pics[0],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_PIC_SLAVE:
-		memcpy(&pic_irqchip(kvm)->pics[1],
-			&chip->chip.pic,
-			sizeof(struct kvm_pic_state));
-		break;
-	case KVM_IRQCHIP_IOAPIC:
-		memcpy(ioapic_irqchip(kvm),
-			&chip->chip.ioapic,
-			sizeof(struct kvm_ioapic_state));
-		break;
-	default:
-		r = -EINVAL;
-		break;
-	}
-	kvm_pic_update_irq(pic_irqchip(kvm));
-	return r;
-}
-
 int is_error_page(struct page *page)
 {
 	return page == bad_page;
@@ -2669,16 +2544,6 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu)
 	return fd;
 }
 
-static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr)
-{
-	int ret;
-
-	if (addr > (unsigned int)(-3 * PAGE_SIZE))
-		return -1;
-	ret = kvm_x86_ops->set_tss_addr(kvm, addr);
-	return ret;
-}
-
 /*
  * Creates some virtual cpus.  Good luck creating more than one.
  */
@@ -2972,35 +2837,14 @@ static long kvm_vm_ioctl(struct file *filp,
 {
 	struct kvm *kvm = filp->private_data;
 	void __user *argp = (void __user *)arg;
-	int r = -EINVAL;
+	int r;
 
 	switch (ioctl) {
-	case KVM_SET_TSS_ADDR:
-		r = kvm_vm_ioctl_set_tss_addr(kvm, arg);
-		if (r < 0)
-			goto out;
-		break;
 	case KVM_CREATE_VCPU:
 		r = kvm_vm_ioctl_create_vcpu(kvm, arg);
 		if (r < 0)
 			goto out;
 		break;
-	case KVM_SET_MEMORY_REGION: {
-		struct kvm_memory_region kvm_mem;
-		struct kvm_userspace_memory_region kvm_userspace_mem;
-
-		r = -EFAULT;
-		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
-			goto out;
-		kvm_userspace_mem.slot = kvm_mem.slot;
-		kvm_userspace_mem.flags = kvm_mem.flags;
-		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
-		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
-		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
-		if (r)
-			goto out;
-		break;
-	}
 	case KVM_SET_USER_MEMORY_REGION: {
 		struct kvm_userspace_memory_region kvm_userspace_mem;
 
@@ -3014,14 +2858,6 @@ static long kvm_vm_ioctl(struct file *filp,
 			goto out;
 		break;
 	}
-	case KVM_SET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
-		if (r)
-			goto out;
-		break;
-	case KVM_GET_NR_MMU_PAGES:
-		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
-		break;
 	case KVM_GET_DIRTY_LOG: {
 		struct kvm_dirty_log log;
 
@@ -3033,87 +2869,8 @@ static long kvm_vm_ioctl(struct file *filp,
 			goto out;
 		break;
 	}
-	case KVM_SET_MEMORY_ALIAS: {
-		struct kvm_memory_alias alias;
-
-		r = -EFAULT;
-		if (copy_from_user(&alias, argp, sizeof alias))
-			goto out;
-		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
-		if (r)
-			goto out;
-		break;
-	}
-	case KVM_CREATE_IRQCHIP:
-		r = -ENOMEM;
-		kvm->vpic = kvm_create_pic(kvm);
-		if (kvm->vpic) {
-			r = kvm_ioapic_init(kvm);
-			if (r) {
-				kfree(kvm->vpic);
-				kvm->vpic = NULL;
-				goto out;
-			}
-		} else
-			goto out;
-		break;
-	case KVM_IRQ_LINE: {
-		struct kvm_irq_level irq_event;
-
-		r = -EFAULT;
-		if (copy_from_user(&irq_event, argp, sizeof irq_event))
-			goto out;
-		if (irqchip_in_kernel(kvm)) {
-			mutex_lock(&kvm->lock);
-			if (irq_event.irq < 16)
-				kvm_pic_set_irq(pic_irqchip(kvm),
-					irq_event.irq,
-					irq_event.level);
-			kvm_ioapic_set_irq(kvm->vioapic,
-					irq_event.irq,
-					irq_event.level);
-			mutex_unlock(&kvm->lock);
-			r = 0;
-		}
-		break;
-	}
-	case KVM_GET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = -EFAULT;
-		if (copy_to_user(argp, &chip, sizeof chip))
-			goto out;
-		r = 0;
-		break;
-	}
-	case KVM_SET_IRQCHIP: {
-		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
-		struct kvm_irqchip chip;
-
-		r = -EFAULT;
-		if (copy_from_user(&chip, argp, sizeof chip))
-			goto out;
-		r = -ENXIO;
-		if (!irqchip_in_kernel(kvm))
-			goto out;
-		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
-		if (r)
-			goto out;
-		r = 0;
-		break;
-	}
 	default:
-		;
+		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
 	}
 out:
 	return r;
diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
index 1fe209d..b84cb67 100644
--- a/drivers/kvm/x86.c
+++ b/drivers/kvm/x86.c
@@ -300,6 +300,264 @@ out:
 	return r;
 }
 
+static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr)
+{
+	int ret;
+
+	if (addr > (unsigned int)(-3 * PAGE_SIZE))
+		return -1;
+	ret = kvm_x86_ops->set_tss_addr(kvm, addr);
+	return ret;
+}
+
+static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
+					  u32 kvm_nr_mmu_pages)
+{
+	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
+		return -EINVAL;
+
+	mutex_lock(&kvm->lock);
+
+	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
+	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
+
+	mutex_unlock(&kvm->lock);
+	return 0;
+}
+
+static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
+{
+	return kvm->n_alloc_mmu_pages;
+}
+
+/*
+ * Set a new alias region.  Aliases map a portion of physical memory into
+ * another portion.  This is useful for memory windows, for example the PC
+ * VGA region.
+ */
+static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
+					 struct kvm_memory_alias *alias)
+{
+	int r, n;
+	struct kvm_mem_alias *p;
+
+	r = -EINVAL;
+	/* General sanity checks */
+	if (alias->memory_size & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
+		goto out;
+	if (alias->slot >= KVM_ALIAS_SLOTS)
+		goto out;
+	if (alias->guest_phys_addr + alias->memory_size
+	    < alias->guest_phys_addr)
+		goto out;
+	if (alias->target_phys_addr + alias->memory_size
+	    < alias->target_phys_addr)
+		goto out;
+
+	mutex_lock(&kvm->lock);
+
+	p = &kvm->aliases[alias->slot];
+	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
+	p->npages = alias->memory_size >> PAGE_SHIFT;
+	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
+
+	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
+		if (kvm->aliases[n - 1].npages)
+			break;
+	kvm->naliases = n;
+
+	kvm_mmu_zap_all(kvm);
+
+	mutex_unlock(&kvm->lock);
+
+	return 0;
+
+out:
+	return r;
+}
+
+static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[0],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&chip->chip.pic,
+			&pic_irqchip(kvm)->pics[1],
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(&chip->chip.ioapic,
+			ioapic_irqchip(kvm),
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	return r;
+}
+
+static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
+{
+	int r;
+
+	r = 0;
+	switch (chip->chip_id) {
+	case KVM_IRQCHIP_PIC_MASTER:
+		memcpy(&pic_irqchip(kvm)->pics[0],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_PIC_SLAVE:
+		memcpy(&pic_irqchip(kvm)->pics[1],
+			&chip->chip.pic,
+			sizeof(struct kvm_pic_state));
+		break;
+	case KVM_IRQCHIP_IOAPIC:
+		memcpy(ioapic_irqchip(kvm),
+			&chip->chip.ioapic,
+			sizeof(struct kvm_ioapic_state));
+		break;
+	default:
+		r = -EINVAL;
+		break;
+	}
+	kvm_pic_update_irq(pic_irqchip(kvm));
+	return r;
+}
+
+long kvm_arch_vm_ioctl(struct file *filp,
+		       unsigned int ioctl, unsigned long arg)
+{
+	struct kvm *kvm = filp->private_data;
+	void __user *argp = (void __user *)arg;
+	int r = -EINVAL;
+
+	switch (ioctl) {
+	case KVM_SET_TSS_ADDR:
+		r = kvm_vm_ioctl_set_tss_addr(kvm, arg);
+		if (r < 0)
+			goto out;
+		break;
+	case KVM_SET_MEMORY_REGION: {
+		struct kvm_memory_region kvm_mem;
+		struct kvm_userspace_memory_region kvm_userspace_mem;
+
+		r = -EFAULT;
+		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
+			goto out;
+		kvm_userspace_mem.slot = kvm_mem.slot;
+		kvm_userspace_mem.flags = kvm_mem.flags;
+		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
+		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
+		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_SET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
+		if (r)
+			goto out;
+		break;
+	case KVM_GET_NR_MMU_PAGES:
+		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
+		break;
+	case KVM_SET_MEMORY_ALIAS: {
+		struct kvm_memory_alias alias;
+
+		r = -EFAULT;
+		if (copy_from_user(&alias, argp, sizeof alias))
+			goto out;
+		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
+		if (r)
+			goto out;
+		break;
+	}
+	case KVM_CREATE_IRQCHIP:
+		r = -ENOMEM;
+		kvm->vpic = kvm_create_pic(kvm);
+		if (kvm->vpic) {
+			r = kvm_ioapic_init(kvm);
+			if (r) {
+				kfree(kvm->vpic);
+				kvm->vpic = NULL;
+				goto out;
+			}
+		} else
+			goto out;
+		break;
+	case KVM_IRQ_LINE: {
+		struct kvm_irq_level irq_event;
+
+		r = -EFAULT;
+		if (copy_from_user(&irq_event, argp, sizeof irq_event))
+			goto out;
+		if (irqchip_in_kernel(kvm)) {
+			mutex_lock(&kvm->lock);
+			if (irq_event.irq < 16)
+				kvm_pic_set_irq(pic_irqchip(kvm),
+					irq_event.irq,
+					irq_event.level);
+			kvm_ioapic_set_irq(kvm->vioapic,
+					irq_event.irq,
+					irq_event.level);
+			mutex_unlock(&kvm->lock);
+			r = 0;
+		}
+		break;
+	}
+	case KVM_GET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = -EFAULT;
+		if (copy_to_user(argp, &chip, sizeof chip))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_SET_IRQCHIP: {
+		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
+		struct kvm_irqchip chip;
+
+		r = -EFAULT;
+		if (copy_from_user(&chip, argp, sizeof chip))
+			goto out;
+		r = -ENXIO;
+		if (!irqchip_in_kernel(kvm))
+			goto out;
+		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
+		if (r)
+			goto out;
+		r = 0;
+		break;
+	}
+	default:
+		;
+	}
+out:
+	return r;
+}
+
 static __init void kvm_init_msr_list(void)
 {
 	u32 dummy[2];



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Avi Kivity wrote:
> Carsten Otte wrote:
>> Avi Kivity wrote:
>>> Why aren't memory slots common too?  Only their number is different, 
>>> while the implementation is the same.
>> Your approach makes the meaning of memory slot somewhat useless on 
>> s390, if that one may be sparse and may be result of different 
>> allocations: On x86, there has to be one memory slot per allocation, 
>> versus on s390 there has to be exactly one memory slot with multiple 
>> allocations behind.
> 
> No, a memory slot can span multiple backing stores.  But it must be 
> contiguous in both the host userspace and guest physical address spaces.
I was not preceise enough: I meant to state that x86 demands one 
memory slot per contiguous allocation. But with your "s390 has only 
one memory slot" idea, this introduces a severe restriction for us: 
our "single memory slot" does not need to be contiguous, neither in 
guest physical nor in host userspace. All we need, is a certain 
1:1+offset relationship between guest physical and host user. Page 
size, backing, sparse are all variable.
Izik's idea, at least how I understood him, makes the best of both 
worlds: we keep above addressing relationship intact, and have 
multiple memory slots for all architectures.

>> For userspace that means, with your approach it has to do total 
>> different memory setup for different archs _if_ it wants to use 
>> multiple allocations and/or sparse:
>> - on x86 it does allocations to random userspace address, and 
>> registers each of them as memory slot
>> - on s390 it does allocations to a specific address layout similar to 
>> the guest, and registers only one memory slot for the whole thing
>>
>> With Izik's approach however, this is transparent to userspace: it has 
>> multiple memory slots, one per allocation. Regardless of the CPU 
>> architecture.
> 
> You can do this with the current memory slots as well.  Although I'm 
> feeling that I misunderstood Izik's idea.  I'll go talk to him.
No we can't: because current memory slots don't have a permanent 
relationship between user and guest physical addresses that we do need 
on s390. We cannot guarantee that over multiple slots, and we cannot 
keep the guest from addressing memory around the memory slots unless 
we refuse to use more than only one slot which has to start at guest 
physical zero.

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>   
>> Carsten Otte wrote:
>>     
>>> Avi Kivity wrote:
>>>       
>>>> Why aren't memory slots common too?  Only their number is different, 
>>>> while the implementation is the same.
>>>>         
>>> Your approach makes the meaning of memory slot somewhat useless on 
>>> s390, if that one may be sparse and may be result of different 
>>> allocations: On x86, there has to be one memory slot per allocation, 
>>> versus on s390 there has to be exactly one memory slot with multiple 
>>> allocations behind.
>>>       
>> No, a memory slot can span multiple backing stores.  But it must be 
>> contiguous in both the host userspace and guest physical address spaces.
>>     
> I was not preceise enough: I meant to state that x86 demands one 
> memory slot per contiguous allocation. But with your "s390 has only 
> one memory slot" idea, this introduces a severe restriction for us: 
> our "single memory slot" does not need to be contiguous, neither in 
> guest physical nor in host userspace. All we need, is a certain 
> 1:1+offset relationship between guest physical and host user. Page 
> size, backing, sparse are all variable.
>   

Well, a slot may be sparse (though I don't think that's supported now).

To me, a slot is exactly a guest offset, size, and host offset.  How the 
memory is populated (or not) is up to the user.

> Izik's idea, at least how I understood him, makes the best of both 
> worlds: we keep above addressing relationship intact, and have 
> multiple memory slots for all architectures.
>   

Can you explain the idea again.  Izik's not here so I can't ask him.

>   
>>> For userspace that means, with your approach it has to do total 
>>> different memory setup for different archs _if_ it wants to use 
>>> multiple allocations and/or sparse:
>>> - on x86 it does allocations to random userspace address, and 
>>> registers each of them as memory slot
>>> - on s390 it does allocations to a specific address layout similar to 
>>> the guest, and registers only one memory slot for the whole thing
>>>
>>> With Izik's approach however, this is transparent to userspace: it has 
>>> multiple memory slots, one per allocation. Regardless of the CPU 
>>> architecture.
>>>       
>> You can do this with the current memory slots as well.  Although I'm 
>> feeling that I misunderstood Izik's idea.  I'll go talk to him.
>>     
> No we can't: because current memory slots don't have a permanent 
> relationship between user and guest physical addresses that we do need 
> on s390. We cannot guarantee that over multiple slots, and we cannot 
> keep the guest from addressing memory around the memory slots unless 
> we refuse to use more than only one slot which has to start at guest 
> physical zero.
>   

Well, you could allow multiple slots and return -EINVAL if they don't 
fit the "host = guest + offset formula".  I'm not sure how that's 
different from one big, possibly sparse, slot.

But again, I don't think I understood Izik's idea, so I'm not sure 
what's the alternative.

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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Hollis Blanchard]

On Fri, 2007-10-26 at 10:21 +0200, Carsten Otte wrote:
> 
> As Xiantao pointed out, x86 and ia64 can share the same memory setup 
> code. But s390 and ppc don't. Therefore, I vote for putting it into 
> x86 for now, and come up with an approach to share between x86 and 
> ia64 later on.

The only thing in the current ioctl code that doesn't make sense for
PowerPC is the rmap stuff. The concept of "slots" as memory extents is
just fine.

Originally, I think the only question was how to register the slots on
the kernel side. Today that is done with separate
KVM_SET_USER_MEMORY_REGION ioctls, one for each guest physical region.

Izik's idea (as I understand it) is to have one ioctl registering all of
RAM, and then multiple "configure slot" ioctls that tell the kernel how
to divide that area into the guest physical address space. That seems
more awkward to me and I don't seem a benefit.

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Hollis Blanchard wrote:
> Izik's idea (as I understand it) is to have one ioctl registering all of
> RAM, and then multiple "configure slot" ioctls that tell the kernel how
> to divide that area into the guest physical address space. That seems
> more awkward to me and I don't seem a benefit.
I think we need to wait for Izik to explain more. In this 
interpretation it really does'nt make sense for s390 as well. My 
interpretation of Izik's idea was just like memory slots are today. 
Avi clarified my misunderstanding of memory slots on x86.

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Zhang, Xiantao wrote:
> I don't think we can move the whole function to arch-specific part,
> because  it should work well (or with few issues) for most archs.
> Basically, IA64 mostly can use it directly. If we move them as
> arch-specific, it will introduces many duplicates. As you said, S390 has
> quite difference about this side, but I think maybe we can use macros,
> such as #ifndef CONFIG_S390 to comment out them, and S390 define it in
> your arch-specific portions. Any other good ideas ?
I really dislike CONFIG_ARCH sections. They are usually a strong 
indication that proper interfaces between arch dependent and arch 
independent code have not been found just yet. If you look at old 
subsystems where lots of engineers had time to optimize the code, like 
memory management for example, you rarely find #ifdefs at all. I think 
that should be our goal.
As for KVM_SET_USER_MEMORY_REGION, the function you pointed at, I've 
left it in common for now in patch version 3.

thanks for your review,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Hollis Blanchard]

Acked-by: Hollis Blanchard <hollisb-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

On Fri, 2007-10-26 at 14:01 +0200, Carsten Otte wrote:
> This patch splits kvm_vm_ioctl into archtecture independent parts, and
> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
> The patch has been updated to current git, and it leaves out memory slot
> registration work which is currently subject to a detailed discussion.
> 
> Common ioctls for all architectures are:
> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
> 
> KVM_SET_USER_MEMORY_REGION implementation is no longer moved to x86.c.
> It seems to me that more fine-grained refinement then just moving the
> code is required here.
> 
> x86 specific ioctls are:
> KVM_SET_MEMORY_REGION, 
> KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
> KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP
> KVM_SET_TSS_ADDR
> 
> KVM_SET_TSS_ADDR has been added to the list of x86 specifics, as Izik's
> commit states it is used for emulating real mode on intel.
> 
> 
>  kvm.h      |    7 +
>  kvm_main.c |  255
> +-----------------------------------------------------------
>  x86.c      |  258
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 271 insertions(+), 249 deletions(-)
> Signed-off-by: Carsten Otte <cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org>
> ---
> diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h
> index b08fc9e..438d4a9 100644
> --- a/drivers/kvm/kvm.h
> +++ b/drivers/kvm/kvm.h
> @@ -621,6 +621,13 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  			 unsigned int ioctl, unsigned long arg);
>  void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
>  void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu);
> +int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
> +				   struct
> +				   kvm_userspace_memory_region *mem,
> +				   int user_alloc);
> +long kvm_arch_vm_ioctl(struct file *filp,
> +		       unsigned int ioctl, unsigned long arg);
> +void kvm_arch_destroy_vm(struct kvm *kvm);
> 
>  __init void kvm_arch_init(void);
> 
> diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c
> index 9a3d663..7a85be9 100644
> --- a/drivers/kvm/kvm_main.c
> +++ b/drivers/kvm/kvm_main.c
> @@ -792,36 +792,16 @@ out:
>  }
>  EXPORT_SYMBOL_GPL(kvm_set_memory_region);
> 
> -static int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
> -					  struct
> -					  kvm_userspace_memory_region *mem,
> -					  int user_alloc)
> +int kvm_vm_ioctl_set_memory_region(struct kvm *kvm,
> +				   struct
> +				   kvm_userspace_memory_region *mem,
> +				   int user_alloc)
>  {
>  	if (mem->slot >= KVM_MEMORY_SLOTS)
>  		return -EINVAL;
>  	return kvm_set_memory_region(kvm, mem, user_alloc);
>  }
> 
> -static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
> -					  u32 kvm_nr_mmu_pages)
> -{
> -	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
> -		return -EINVAL;
> -
> -	mutex_lock(&kvm->lock);
> -
> -	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
> -	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
> -
> -	mutex_unlock(&kvm->lock);
> -	return 0;
> -}
> -
> -static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
> -{
> -	return kvm->n_alloc_mmu_pages;
> -}
> -
>  /*
>   * Get (and clear) the dirty memory log for a memory slot.
>   */
> @@ -867,111 +847,6 @@ out:
>  	return r;
>  }
> 
> -/*
> - * Set a new alias region.  Aliases map a portion of physical memory into
> - * another portion.  This is useful for memory windows, for example the PC
> - * VGA region.
> - */
> -static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
> -					 struct kvm_memory_alias *alias)
> -{
> -	int r, n;
> -	struct kvm_mem_alias *p;
> -
> -	r = -EINVAL;
> -	/* General sanity checks */
> -	if (alias->memory_size & (PAGE_SIZE - 1))
> -		goto out;
> -	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
> -		goto out;
> -	if (alias->slot >= KVM_ALIAS_SLOTS)
> -		goto out;
> -	if (alias->guest_phys_addr + alias->memory_size
> -	    < alias->guest_phys_addr)
> -		goto out;
> -	if (alias->target_phys_addr + alias->memory_size
> -	    < alias->target_phys_addr)
> -		goto out;
> -
> -	mutex_lock(&kvm->lock);
> -
> -	p = &kvm->aliases[alias->slot];
> -	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
> -	p->npages = alias->memory_size >> PAGE_SHIFT;
> -	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
> -
> -	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
> -		if (kvm->aliases[n - 1].npages)
> -			break;
> -	kvm->naliases = n;
> -
> -	kvm_mmu_zap_all(kvm);
> -
> -	mutex_unlock(&kvm->lock);
> -
> -	return 0;
> -
> -out:
> -	return r;
> -}
> -
> -static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
> -{
> -	int r;
> -
> -	r = 0;
> -	switch (chip->chip_id) {
> -	case KVM_IRQCHIP_PIC_MASTER:
> -		memcpy(&chip->chip.pic,
> -			&pic_irqchip(kvm)->pics[0],
> -			sizeof(struct kvm_pic_state));
> -		break;
> -	case KVM_IRQCHIP_PIC_SLAVE:
> -		memcpy(&chip->chip.pic,
> -			&pic_irqchip(kvm)->pics[1],
> -			sizeof(struct kvm_pic_state));
> -		break;
> -	case KVM_IRQCHIP_IOAPIC:
> -		memcpy(&chip->chip.ioapic,
> -			ioapic_irqchip(kvm),
> -			sizeof(struct kvm_ioapic_state));
> -		break;
> -	default:
> -		r = -EINVAL;
> -		break;
> -	}
> -	return r;
> -}
> -
> -static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
> -{
> -	int r;
> -
> -	r = 0;
> -	switch (chip->chip_id) {
> -	case KVM_IRQCHIP_PIC_MASTER:
> -		memcpy(&pic_irqchip(kvm)->pics[0],
> -			&chip->chip.pic,
> -			sizeof(struct kvm_pic_state));
> -		break;
> -	case KVM_IRQCHIP_PIC_SLAVE:
> -		memcpy(&pic_irqchip(kvm)->pics[1],
> -			&chip->chip.pic,
> -			sizeof(struct kvm_pic_state));
> -		break;
> -	case KVM_IRQCHIP_IOAPIC:
> -		memcpy(ioapic_irqchip(kvm),
> -			&chip->chip.ioapic,
> -			sizeof(struct kvm_ioapic_state));
> -		break;
> -	default:
> -		r = -EINVAL;
> -		break;
> -	}
> -	kvm_pic_update_irq(pic_irqchip(kvm));
> -	return r;
> -}
> -
>  int is_error_page(struct page *page)
>  {
>  	return page == bad_page;
> @@ -2669,16 +2544,6 @@ static int create_vcpu_fd(struct kvm_vcpu *vcpu)
>  	return fd;
>  }
> 
> -static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr)
> -{
> -	int ret;
> -
> -	if (addr > (unsigned int)(-3 * PAGE_SIZE))
> -		return -1;
> -	ret = kvm_x86_ops->set_tss_addr(kvm, addr);
> -	return ret;
> -}
> -
>  /*
>   * Creates some virtual cpus.  Good luck creating more than one.
>   */
> @@ -2972,35 +2837,14 @@ static long kvm_vm_ioctl(struct file *filp,
>  {
>  	struct kvm *kvm = filp->private_data;
>  	void __user *argp = (void __user *)arg;
> -	int r = -EINVAL;
> +	int r;
> 
>  	switch (ioctl) {
> -	case KVM_SET_TSS_ADDR:
> -		r = kvm_vm_ioctl_set_tss_addr(kvm, arg);
> -		if (r < 0)
> -			goto out;
> -		break;
>  	case KVM_CREATE_VCPU:
>  		r = kvm_vm_ioctl_create_vcpu(kvm, arg);
>  		if (r < 0)
>  			goto out;
>  		break;
> -	case KVM_SET_MEMORY_REGION: {
> -		struct kvm_memory_region kvm_mem;
> -		struct kvm_userspace_memory_region kvm_userspace_mem;
> -
> -		r = -EFAULT;
> -		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
> -			goto out;
> -		kvm_userspace_mem.slot = kvm_mem.slot;
> -		kvm_userspace_mem.flags = kvm_mem.flags;
> -		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
> -		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
> -		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
> -		if (r)
> -			goto out;
> -		break;
> -	}
>  	case KVM_SET_USER_MEMORY_REGION: {
>  		struct kvm_userspace_memory_region kvm_userspace_mem;
> 
> @@ -3014,14 +2858,6 @@ static long kvm_vm_ioctl(struct file *filp,
>  			goto out;
>  		break;
>  	}
> -	case KVM_SET_NR_MMU_PAGES:
> -		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
> -		if (r)
> -			goto out;
> -		break;
> -	case KVM_GET_NR_MMU_PAGES:
> -		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
> -		break;
>  	case KVM_GET_DIRTY_LOG: {
>  		struct kvm_dirty_log log;
> 
> @@ -3033,87 +2869,8 @@ static long kvm_vm_ioctl(struct file *filp,
>  			goto out;
>  		break;
>  	}
> -	case KVM_SET_MEMORY_ALIAS: {
> -		struct kvm_memory_alias alias;
> -
> -		r = -EFAULT;
> -		if (copy_from_user(&alias, argp, sizeof alias))
> -			goto out;
> -		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
> -		if (r)
> -			goto out;
> -		break;
> -	}
> -	case KVM_CREATE_IRQCHIP:
> -		r = -ENOMEM;
> -		kvm->vpic = kvm_create_pic(kvm);
> -		if (kvm->vpic) {
> -			r = kvm_ioapic_init(kvm);
> -			if (r) {
> -				kfree(kvm->vpic);
> -				kvm->vpic = NULL;
> -				goto out;
> -			}
> -		} else
> -			goto out;
> -		break;
> -	case KVM_IRQ_LINE: {
> -		struct kvm_irq_level irq_event;
> -
> -		r = -EFAULT;
> -		if (copy_from_user(&irq_event, argp, sizeof irq_event))
> -			goto out;
> -		if (irqchip_in_kernel(kvm)) {
> -			mutex_lock(&kvm->lock);
> -			if (irq_event.irq < 16)
> -				kvm_pic_set_irq(pic_irqchip(kvm),
> -					irq_event.irq,
> -					irq_event.level);
> -			kvm_ioapic_set_irq(kvm->vioapic,
> -					irq_event.irq,
> -					irq_event.level);
> -			mutex_unlock(&kvm->lock);
> -			r = 0;
> -		}
> -		break;
> -	}
> -	case KVM_GET_IRQCHIP: {
> -		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
> -		struct kvm_irqchip chip;
> -
> -		r = -EFAULT;
> -		if (copy_from_user(&chip, argp, sizeof chip))
> -			goto out;
> -		r = -ENXIO;
> -		if (!irqchip_in_kernel(kvm))
> -			goto out;
> -		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
> -		if (r)
> -			goto out;
> -		r = -EFAULT;
> -		if (copy_to_user(argp, &chip, sizeof chip))
> -			goto out;
> -		r = 0;
> -		break;
> -	}
> -	case KVM_SET_IRQCHIP: {
> -		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
> -		struct kvm_irqchip chip;
> -
> -		r = -EFAULT;
> -		if (copy_from_user(&chip, argp, sizeof chip))
> -			goto out;
> -		r = -ENXIO;
> -		if (!irqchip_in_kernel(kvm))
> -			goto out;
> -		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
> -		if (r)
> -			goto out;
> -		r = 0;
> -		break;
> -	}
>  	default:
> -		;
> +		r = kvm_arch_vm_ioctl(filp, ioctl, arg);
>  	}
>  out:
>  	return r;
> diff --git a/drivers/kvm/x86.c b/drivers/kvm/x86.c
> index 1fe209d..b84cb67 100644
> --- a/drivers/kvm/x86.c
> +++ b/drivers/kvm/x86.c
> @@ -300,6 +300,264 @@ out:
>  	return r;
>  }
> 
> +static int kvm_vm_ioctl_set_tss_addr(struct kvm *kvm, unsigned long addr)
> +{
> +	int ret;
> +
> +	if (addr > (unsigned int)(-3 * PAGE_SIZE))
> +		return -1;
> +	ret = kvm_x86_ops->set_tss_addr(kvm, addr);
> +	return ret;
> +}
> +
> +static int kvm_vm_ioctl_set_nr_mmu_pages(struct kvm *kvm,
> +					  u32 kvm_nr_mmu_pages)
> +{
> +	if (kvm_nr_mmu_pages < KVM_MIN_ALLOC_MMU_PAGES)
> +		return -EINVAL;
> +
> +	mutex_lock(&kvm->lock);
> +
> +	kvm_mmu_change_mmu_pages(kvm, kvm_nr_mmu_pages);
> +	kvm->n_requested_mmu_pages = kvm_nr_mmu_pages;
> +
> +	mutex_unlock(&kvm->lock);
> +	return 0;
> +}
> +
> +static int kvm_vm_ioctl_get_nr_mmu_pages(struct kvm *kvm)
> +{
> +	return kvm->n_alloc_mmu_pages;
> +}
> +
> +/*
> + * Set a new alias region.  Aliases map a portion of physical memory into
> + * another portion.  This is useful for memory windows, for example the PC
> + * VGA region.
> + */
> +static int kvm_vm_ioctl_set_memory_alias(struct kvm *kvm,
> +					 struct kvm_memory_alias *alias)
> +{
> +	int r, n;
> +	struct kvm_mem_alias *p;
> +
> +	r = -EINVAL;
> +	/* General sanity checks */
> +	if (alias->memory_size & (PAGE_SIZE - 1))
> +		goto out;
> +	if (alias->guest_phys_addr & (PAGE_SIZE - 1))
> +		goto out;
> +	if (alias->slot >= KVM_ALIAS_SLOTS)
> +		goto out;
> +	if (alias->guest_phys_addr + alias->memory_size
> +	    < alias->guest_phys_addr)
> +		goto out;
> +	if (alias->target_phys_addr + alias->memory_size
> +	    < alias->target_phys_addr)
> +		goto out;
> +
> +	mutex_lock(&kvm->lock);
> +
> +	p = &kvm->aliases[alias->slot];
> +	p->base_gfn = alias->guest_phys_addr >> PAGE_SHIFT;
> +	p->npages = alias->memory_size >> PAGE_SHIFT;
> +	p->target_gfn = alias->target_phys_addr >> PAGE_SHIFT;
> +
> +	for (n = KVM_ALIAS_SLOTS; n > 0; --n)
> +		if (kvm->aliases[n - 1].npages)
> +			break;
> +	kvm->naliases = n;
> +
> +	kvm_mmu_zap_all(kvm);
> +
> +	mutex_unlock(&kvm->lock);
> +
> +	return 0;
> +
> +out:
> +	return r;
> +}
> +
> +static int kvm_vm_ioctl_get_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
> +{
> +	int r;
> +
> +	r = 0;
> +	switch (chip->chip_id) {
> +	case KVM_IRQCHIP_PIC_MASTER:
> +		memcpy(&chip->chip.pic,
> +			&pic_irqchip(kvm)->pics[0],
> +			sizeof(struct kvm_pic_state));
> +		break;
> +	case KVM_IRQCHIP_PIC_SLAVE:
> +		memcpy(&chip->chip.pic,
> +			&pic_irqchip(kvm)->pics[1],
> +			sizeof(struct kvm_pic_state));
> +		break;
> +	case KVM_IRQCHIP_IOAPIC:
> +		memcpy(&chip->chip.ioapic,
> +			ioapic_irqchip(kvm),
> +			sizeof(struct kvm_ioapic_state));
> +		break;
> +	default:
> +		r = -EINVAL;
> +		break;
> +	}
> +	return r;
> +}
> +
> +static int kvm_vm_ioctl_set_irqchip(struct kvm *kvm, struct kvm_irqchip *chip)
> +{
> +	int r;
> +
> +	r = 0;
> +	switch (chip->chip_id) {
> +	case KVM_IRQCHIP_PIC_MASTER:
> +		memcpy(&pic_irqchip(kvm)->pics[0],
> +			&chip->chip.pic,
> +			sizeof(struct kvm_pic_state));
> +		break;
> +	case KVM_IRQCHIP_PIC_SLAVE:
> +		memcpy(&pic_irqchip(kvm)->pics[1],
> +			&chip->chip.pic,
> +			sizeof(struct kvm_pic_state));
> +		break;
> +	case KVM_IRQCHIP_IOAPIC:
> +		memcpy(ioapic_irqchip(kvm),
> +			&chip->chip.ioapic,
> +			sizeof(struct kvm_ioapic_state));
> +		break;
> +	default:
> +		r = -EINVAL;
> +		break;
> +	}
> +	kvm_pic_update_irq(pic_irqchip(kvm));
> +	return r;
> +}
> +
> +long kvm_arch_vm_ioctl(struct file *filp,
> +		       unsigned int ioctl, unsigned long arg)
> +{
> +	struct kvm *kvm = filp->private_data;
> +	void __user *argp = (void __user *)arg;
> +	int r = -EINVAL;
> +
> +	switch (ioctl) {
> +	case KVM_SET_TSS_ADDR:
> +		r = kvm_vm_ioctl_set_tss_addr(kvm, arg);
> +		if (r < 0)
> +			goto out;
> +		break;
> +	case KVM_SET_MEMORY_REGION: {
> +		struct kvm_memory_region kvm_mem;
> +		struct kvm_userspace_memory_region kvm_userspace_mem;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&kvm_mem, argp, sizeof kvm_mem))
> +			goto out;
> +		kvm_userspace_mem.slot = kvm_mem.slot;
> +		kvm_userspace_mem.flags = kvm_mem.flags;
> +		kvm_userspace_mem.guest_phys_addr = kvm_mem.guest_phys_addr;
> +		kvm_userspace_mem.memory_size = kvm_mem.memory_size;
> +		r = kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0);
> +		if (r)
> +			goto out;
> +		break;
> +	}
> +	case KVM_SET_NR_MMU_PAGES:
> +		r = kvm_vm_ioctl_set_nr_mmu_pages(kvm, arg);
> +		if (r)
> +			goto out;
> +		break;
> +	case KVM_GET_NR_MMU_PAGES:
> +		r = kvm_vm_ioctl_get_nr_mmu_pages(kvm);
> +		break;
> +	case KVM_SET_MEMORY_ALIAS: {
> +		struct kvm_memory_alias alias;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&alias, argp, sizeof alias))
> +			goto out;
> +		r = kvm_vm_ioctl_set_memory_alias(kvm, &alias);
> +		if (r)
> +			goto out;
> +		break;
> +	}
> +	case KVM_CREATE_IRQCHIP:
> +		r = -ENOMEM;
> +		kvm->vpic = kvm_create_pic(kvm);
> +		if (kvm->vpic) {
> +			r = kvm_ioapic_init(kvm);
> +			if (r) {
> +				kfree(kvm->vpic);
> +				kvm->vpic = NULL;
> +				goto out;
> +			}
> +		} else
> +			goto out;
> +		break;
> +	case KVM_IRQ_LINE: {
> +		struct kvm_irq_level irq_event;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&irq_event, argp, sizeof irq_event))
> +			goto out;
> +		if (irqchip_in_kernel(kvm)) {
> +			mutex_lock(&kvm->lock);
> +			if (irq_event.irq < 16)
> +				kvm_pic_set_irq(pic_irqchip(kvm),
> +					irq_event.irq,
> +					irq_event.level);
> +			kvm_ioapic_set_irq(kvm->vioapic,
> +					irq_event.irq,
> +					irq_event.level);
> +			mutex_unlock(&kvm->lock);
> +			r = 0;
> +		}
> +		break;
> +	}
> +	case KVM_GET_IRQCHIP: {
> +		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
> +		struct kvm_irqchip chip;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&chip, argp, sizeof chip))
> +			goto out;
> +		r = -ENXIO;
> +		if (!irqchip_in_kernel(kvm))
> +			goto out;
> +		r = kvm_vm_ioctl_get_irqchip(kvm, &chip);
> +		if (r)
> +			goto out;
> +		r = -EFAULT;
> +		if (copy_to_user(argp, &chip, sizeof chip))
> +			goto out;
> +		r = 0;
> +		break;
> +	}
> +	case KVM_SET_IRQCHIP: {
> +		/* 0: PIC master, 1: PIC slave, 2: IOAPIC */
> +		struct kvm_irqchip chip;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&chip, argp, sizeof chip))
> +			goto out;
> +		r = -ENXIO;
> +		if (!irqchip_in_kernel(kvm))
> +			goto out;
> +		r = kvm_vm_ioctl_set_irqchip(kvm, &chip);
> +		if (r)
> +			goto out;
> +		r = 0;
> +		break;
> +	}
> +	default:
> +		;
> +	}
> +out:
> +	return r;
> +}
> +
>  static __init void kvm_init_msr_list(void)
>  {
>  	u32 dummy[2];
> 
> 
-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

Carsten Otte wrote:
> Hollis Blanchard wrote:
>   
>> Izik's idea (as I understand it) is to have one ioctl registering all of
>> RAM, and then multiple "configure slot" ioctls that tell the kernel how
>> to divide that area into the guest physical address space. That seems
>> more awkward to me and I don't seem a benefit.
>>     
> I think we need to wait for Izik to explain more. In this 
> interpretation it really does'nt make sense for s390 as well. My 
> interpretation of Izik's idea was just like memory slots are today. 
> Avi clarified my misunderstanding of memory slots on x86.
>   
my idea was to let kvm register userspace memory that will hold the 
guest memory,
and then much like qemu do with its cpu_register_physical_memory 
function, run ioctls
that will tell the kernel how to map this memory,
what i wanted to achieved is making the memory allocation code shared, 
but still
let each arch to use any kind of "arch depended" mapping rules it want,
but after reading the last posts it seems like you can use the normal 
memslots, so it is kind of useless i guess and i will
give up the idea of writing patch to make this, unless you still have 
problem with the memslots?

thanks.

> so long,
> Carsten
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Splunk Inc.
> Still grepping through log files to find problems?  Stop.
> Now Search log events and configuration files using AJAX and a browser.
> Download your FREE copy of Splunk now >> http://get.splunk.com/
> _______________________________________________
> kvm-devel mailing list
> kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/kvm-devel
>   


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Hollis Blanchard wrote:
> On Fri, 2007-10-26 at 10:21 +0200, Carsten Otte wrote:
>   
>> As Xiantao pointed out, x86 and ia64 can share the same memory setup 
>> code. But s390 and ppc don't. Therefore, I vote for putting it into 
>> x86 for now, and come up with an approach to share between x86 and 
>> ia64 later on.
>>     
>
> The only thing in the current ioctl code that doesn't make sense for
> PowerPC is the rmap stuff. The concept of "slots" as memory extents is
> just fine.
>
> Originally, I think the only question was how to register the slots on
> the kernel side. Today that is done with separate
> KVM_SET_USER_MEMORY_REGION ioctls, one for each guest physical region.
>   

Wht about aliases?  Aliases allow you go have two different physical
addresses for the same page.  This is useful for mapping the framebuffer
on x86 (both at the pci region and at 0xa0000, the traditional ISA
location).

Are they useful for ia64?


-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Zhang, Xiantao]

Avi Kivity wrote:
> Hollis Blanchard wrote:
>> On Fri, 2007-10-26 at 10:21 +0200, Carsten Otte wrote:
>> 
>>> As Xiantao pointed out, x86 and ia64 can share the same memory setup
>>> code. But s390 and ppc don't. Therefore, I vote for putting it into
>>> x86 for now, and come up with an approach to share between x86 and
>>> ia64 later on. 
>>> 
>> 
>> The only thing in the current ioctl code that doesn't make sense for
>> PowerPC is the rmap stuff. The concept of "slots" as memory extents
>> is just fine. 
>> 
>> Originally, I think the only question was how to register the slots
>> on the kernel side. Today that is done with separate
>> KVM_SET_USER_MEMORY_REGION ioctls, one for each guest physical
>> region. 
>> 
> 
> Wht about aliases?  Aliases allow you go have two different physical
> addresses for the same page.  This is useful for mapping the
> framebuffer on x86 (both at the pci region and at 0xa0000, the
> traditional ISA location).
> 
> Are they useful for ia64?

Sure,  it also should benefit IA64 side :)
thanks 
Xiantao 

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Avi Kivity]

Zhang, Xiantao wrote:
> Avi Kivity wrote:
>   
>> Hollis Blanchard wrote:
>>     
>>> On Fri, 2007-10-26 at 10:21 +0200, Carsten Otte wrote:
>>>
>>>       
>>>> As Xiantao pointed out, x86 and ia64 can share the same memory setup
>>>> code. But s390 and ppc don't. Therefore, I vote for putting it into
>>>> x86 for now, and come up with an approach to share between x86 and
>>>> ia64 later on. 
>>>>
>>>>         
>>> The only thing in the current ioctl code that doesn't make sense for
>>> PowerPC is the rmap stuff. The concept of "slots" as memory extents
>>> is just fine. 
>>>
>>> Originally, I think the only question was how to register the slots
>>> on the kernel side. Today that is done with separate
>>> KVM_SET_USER_MEMORY_REGION ioctls, one for each guest physical
>>> region. 
>>>
>>>       
>> Wht about aliases?  Aliases allow you go have two different physical
>> addresses for the same page.  This is useful for mapping the
>> framebuffer on x86 (both at the pci region and at 0xa0000, the
>> traditional ISA location).
>>
>> Are they useful for ia64?
>>     
>
> Sure,  it also should benefit IA64 side :)
>   

Actually, thinking about it, regular memory slots support this too.  
Simply create a new memory slot to that memory.  So aliases can be x86 
specific (for compatibility reasons).


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


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Avi Kivity wrote:
> Zhang, Xiantao wrote:
>> Avi Kivity wrote:
>>> Wht about aliases?  Aliases allow you go have two different physical
>>> addresses for the same page.  This is useful for mapping the
>>> framebuffer on x86 (both at the pci region and at 0xa0000, the
>>> traditional ISA location).
>>>
>>> Are they useful for ia64?
>>>     
>>
>> Sure,  it also should benefit IA64 side :)
>>   
> 
> Actually, thinking about it, regular memory slots support this too.  
> Simply create a new memory slot to that memory.  So aliases can be x86 
> specific (for compatibility reasons).

Memory slots turn out to be surprisingly flexible :-).

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Izik Eidus]

On Mon, 2007-10-29 at 09:01 +0100, Carsten Otte wrote:
> Avi Kivity wrote:
> > Zhang, Xiantao wrote:
> >> Avi Kivity wrote:
> >>> Wht about aliases?  Aliases allow you go have two different physical
> >>> addresses for the same page.  This is useful for mapping the
> >>> framebuffer on x86 (both at the pci region and at 0xa0000, the
> >>> traditional ISA location).
> >>>
> >>> Are they useful for ia64?
> >>>     
> >>
> >> Sure,  it also should benefit IA64 side :)
> >>   
> > 
> > Actually, thinking about it, regular memory slots support this too.  
> > Simply create a new memory slot to that memory.  So aliases can be x86 
> > specific (for compatibility reasons).
> 
> Memory slots turn out to be surprisingly flexible :-).

yep,
another cleanup is needed for:
the rmap part, and the mmu cache size setting that found it way to the
memory slot allocation function.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Izik Eidus wrote:
> my idea was to let kvm register userspace memory that will hold the 
> guest memory,
> and then much like qemu do with its cpu_register_physical_memory 
> function, run ioctls
> that will tell the kernel how to map this memory,
> what i wanted to achieved is making the memory allocation code shared, 
> but still
> let each arch to use any kind of "arch depended" mapping rules it want,
> but after reading the last posts it seems like you can use the normal 
> memslots, so it is kind of useless i guess and i will
> give up the idea of writing patch to make this, unless you still have 
> problem with the memslots?
Looks like we shared the same misunderstanding about the current 
memslot implementation *shaking hands*. After being educated on 
memslots by Avi, I agree that memslots do the right thing.

cheers,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: [kvm-ppc-devel] RFC/patch portability: split kvm_vm_ioctl v2

[Carsten Otte]

Izik Eidus wrote:
> yep,
> another cleanup is needed for:
> the rmap part, and the mmu cache size setting that found it way to the
> memory slot allocation function.
That's true, but can be subject of one of the next portability paches. 
I want to keep each of them trivial and easily reviewable.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 1489 bytes --]

 

>-----Original Message-----
>From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>[mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>Carsten Otte
>Sent: 2007年10月26日 20:02
>To: Avi Kivity; Zhang, Xiantao; Hollis Blanchard
>Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: Re: [kvm-devel] RFC/patch portability: split kvm_vm_ioctl v3
>
>This patch splits kvm_vm_ioctl into archtecture independent parts, and
>x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
>The patch has been updated to current git, and it leaves out 
>memory slot
>registration work which is currently subject to a detailed discussion.
>
>Common ioctls for all architectures are:
>KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
>
>KVM_SET_USER_MEMORY_REGION implementation is no longer moved to x86.c.
>It seems to me that more fine-grained refinement then just moving the
>code is required here.
>
>x86 specific ioctls are:
>KVM_SET_MEMORY_REGION, 
>KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
>KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP
>KVM_SET_TSS_ADDR
>
>KVM_SET_TSS_ADDR has been added to the list of x86 specifics, as Izik's
>commit states it is used for emulating real mode on intel.
>
Why KVM_IRQ_LINE is X86b specific? The original idea are based on ACPI spec which
I assume to be generic though S390 may not take.

thx,eddie


[-- Attachment #2: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Avi Kivity]

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

Dong, Eddie wrote:
>  
>
>   
>> -----Original Message-----
>> From: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org 
>> [mailto:kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org] On Behalf Of 
>> Carsten Otte
>> Sent: 2007年10月26日 20:02
>> To: Avi Kivity; Zhang, Xiantao; Hollis Blanchard
>> Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>> Subject: Re: [kvm-devel] RFC/patch portability: split kvm_vm_ioctl v3
>>
>> This patch splits kvm_vm_ioctl into archtecture independent parts, and
>> x86 specific parts which go to kvm_arch_vcpu_ioctl in x86.c. 
>> The patch has been updated to current git, and it leaves out 
>> memory slot
>> registration work which is currently subject to a detailed discussion.
>>
>> Common ioctls for all architectures are:
>> KVM_CREATE_VCPU, KVM_GET_DIRTY_LOG, KVM_SET_USER_MEMORY_REGION
>>
>> KVM_SET_USER_MEMORY_REGION implementation is no longer moved to x86.c.
>> It seems to me that more fine-grained refinement then just moving the
>> code is required here.
>>
>> x86 specific ioctls are:
>> KVM_SET_MEMORY_REGION, 
>> KVM_GET/SET_NR_MMU_PAGES, KVM_SET_MEMORY_ALIAS, KVM_CREATE_IRQCHIP,
>> KVM_CREATE_IRQ_LINE, KVM_GET/SET_IRQCHIP
>> KVM_SET_TSS_ADDR
>>
>> KVM_SET_TSS_ADDR has been added to the list of x86 specifics, as Izik's
>> commit states it is used for emulating real mode on intel.
>>
>>     
> Why KVM_IRQ_LINE is X86b specific? The original idea are based on ACPI spec which
> I assume to be generic though S390 may not take.
>
>   

ia64 can probably share much. ppc will probably want KVM_IRQ_LINE with
with different parameters. s390, as far as I understand, will not.


-- 
Any sufficiently difficult bug is indistinguishable from a feature.



[-- Attachment #2: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Dong, Eddie]

     
>> Why KVM_IRQ_LINE is X86b specific? The original idea are 
>based on ACPI spec which
>> I assume to be generic though S390 may not take.
>>
>>   
>
>ia64 can probably share much. ppc will probably want KVM_IRQ_LINE with
>with different parameters. s390, as far as I understand, will not.
>

How does PPC use different parameter? Current approach use gsi irq line
# 
as parameter, though the comments says something related to APIC/PIC.
It is OK for PPC to map gsi irq line to its internal signal, but the API
itself
can be same.

Yes, S390 is very special (even no PCI), since it is special, Carsten
probably
can handle it in special way, or just not use it :-)

BTW, how S390 user/kernel interrupt signal is communicated?

thx,eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Dong, Eddie wrote:
> Why KVM_IRQ_LINE is X86b specific? The original idea are based on ACPI spec which
> I assume to be generic though S390 may not take.
ACPI is not present on s390 and ppc. In fact, I doubt it is present on 
any architecture except those two intel ones: at least my mips router 
and my arm pda don't have it either. It's kind of based on the idea of 
having a bios alike code.

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]

OK, so how can a device inform kernel for an IRQ in S390? 

>-----Original Message-----
>From: Carsten Otte [mailto:cotte-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org] 
>Sent: 2007年10月30日 19:30
>To: Dong, Eddie
>Cc: Avi Kivity; Zhang, Xiantao; Hollis Blanchard; 
>kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>Subject: Re: [kvm-devel] RFC/patch portability: split kvm_vm_ioctl v3
>
>Dong, Eddie wrote:
>> Why KVM_IRQ_LINE is X86b specific? The original idea are 
>based on ACPI spec which
>> I assume to be generic though S390 may not take.
>ACPI is not present on s390 and ppc. In fact, I doubt it is present on 
>any architecture except those two intel ones: at least my mips router 
>and my arm pda don't have it either. It's kind of based on the idea of 
>having a bios alike code.
>
>so long,
>Carsten
>


[-- Attachment #2: Type: text/plain, Size: 314 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

[-- Attachment #3: Type: text/plain, Size: 186 bytes --]

_______________________________________________
kvm-devel mailing list
kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Avi Kivity wrote:
>> Why KVM_IRQ_LINE is X86b specific? The original idea are based on ACPI spec which
>> I assume to be generic though S390 may not take.
>>
>>   
> 
> ia64 can probably share much. ppc will probably want KVM_IRQ_LINE with
> with different parameters. s390, as far as I understand, will not.
I think we'll have to come up with a more modular approach later on: 
various aspects are of interest to various architectures and/or 
platforms. The generic kernel has CONFIG_FEATURE toggles for that.
The portability patches are not intended to split kvm into components 
at this stage, I believe that is something that we will have to come 
up when actual ports are being integrated. In my optinion, a 
reasonable next-step refinement here would be to come up with a 
generic interrupt injection call that can inject an interrupt on any 
architecture and platform. After userspace has adopted to use that 
one, we can keep the old call for backward compatibility reasons in a 
deprecated state for some time before removing it.
For now, my goal is to seperate what is generic in a way that it is a 
functionality that a portable user space program that uses kvm can 
expect to work the same way on all architectures and platforms.

so long,
Carsten


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Avi Kivity]

Carsten Otte wrote:
> I think we'll have to come up with a more modular approach later on: 
> various aspects are of interest to various architectures and/or 
> platforms. The generic kernel has CONFIG_FEATURE toggles for that.
> The portability patches are not intended to split kvm into components 
> at this stage, I believe that is something that we will have to come 
> up when actual ports are being integrated. In my optinion, a 
> reasonable next-step refinement here would be to come up with a 
> generic interrupt injection call that can inject an interrupt on any 
> architecture and platform. After userspace has adopted to use that 
> one, we can keep the old call for backward compatibility reasons in a 
> deprecated state for some time before removing it.
> For now, my goal is to seperate what is generic in a way that it is a 
> functionality that a portable user space program that uses kvm can 
> expect to work the same way on all architectures and platforms.
>

We have to be careful not to force too much portability on the code.  
After all, the instruction set is different and some of the hardware 
philosophy is different.  You will never be able to run the same guest 
on different archs, or have exactly the same virtual devices.  The 
differences are real, and the goal is not portability at any cost; it is 
to share as much as possible, but not more.

Architectures which have interrupt request lines that are edge-triggered 
or level-triggered and emulate the interrupt controller in the kernel 
can share the KVM_IRQ_LINE API in some way; architectures that don't 
will need another method.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Dong, Eddie wrote:
> BTW, how S390 user/kernel interrupt signal is communicated?
Our s90host prototype does inject it from userspace, so there is no 
need to have a call for that.

Nevertheless, I really love the lightweight exits that kvm has 
invented and I want to use it on s390 with kvm too. In that case, 
we'll have an interrupt facility in the kernel that is in terms of 
functionality close to the pic/lapic work for x86.

For that, I think we could encode an interrupt number in an __u64 that 
has a platform specific helper function in userspace to generate that 
interrupt number: for x86 that would just equal the irq line, for s390 
that would be more complex. The common kvm code can then take that 
__u64 number and call an architecture specific callback that injects 
the interrupt.

In addition, I would love to be able to specify which target CPUs may 
receive that interrupt because our IPI equivalent comes out just like 
a regular interrupt on just one target CPU.

That boils down to something like this:
struct kvm_interrupt_data {
	__u64	 interrupt_number;
	cpuset_t possible_target_cpus;
}
and an KVM_INJECT_INTERRUPT common ioctl for the vm to provide this.

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Avi Kivity]

Carsten Otte wrote:
> Dong, Eddie wrote:
>> BTW, how S390 user/kernel interrupt signal is communicated?
> Our s90host prototype does inject it from userspace, so there is no 
> need to have a call for that.
>
> Nevertheless, I really love the lightweight exits that kvm has 
> invented and I want to use it on s390 with kvm too.

It originated as a Xen hvm thing which Eddie & co. ported over.

> In that case, we'll have an interrupt facility in the kernel that is 
> in terms of functionality close to the pic/lapic work for x86.
>
> For that, I think we could encode an interrupt number in an __u64 that 
> has a platform specific helper function in userspace to generate that 
> interrupt number: for x86 that would just equal the irq line, for s390 
> that would be more complex. The common kvm code can then take that 
> __u64 number and call an architecture specific callback that injects 
> the interrupt.
>

But that doesn't make the code portable.  The s390 userspace has to know 
how to encode the number, and x86 will do it differently.

If it's really different, let's keep it different.  Unless you can push 
the encoding so far back it's transparent to userspace (e.g. qemu).

> In addition, I would love to be able to specify which target CPUs may 
> receive that interrupt because our IPI equivalent comes out just like 
> a regular interrupt on just one target CPU.
>
> That boils down to something like this:
> struct kvm_interrupt_data {
>     __u64     interrupt_number;
>     cpuset_t possible_target_cpus;
> }
> and an KVM_INJECT_INTERRUPT common ioctl for the vm to provide this.

Are cpusets exported to userspace?

x86 has something similar (IPI to a set of cpus) but it's handled 100% 
in the kernel these days.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Dong, Eddie wrote:
> OK, so how can a device inform kernel for an IRQ in S390? 
Oooh, that is a looong explanation. If you want to peek at it, see 
http://publibz.boulder.ibm.com/epubs/pdf/a2278324.pdf . Chapter 6 
covers Interruptions. I'd recommend to start with reading external 
interruptions, because that is the one we'll be primarily be using 
with kvm. External interruptions are used for things like timers, 
hypercalls, and IPIs. The "Program Interruption Coditions" are also 
worth reading, they cover things similar to general protection fault 
on x86. Chapter 11 covers a different type of Interruptions, such as 
error detection of hardware failures and hot-standby component 
failover. Chapter 16 is also of interrest, it covers I/O interruptions.

Whenever you see me in person (next kvm forum maybe?), you are invited 
to a lot of beer: I'll bring pen and paper and try to give you an 
overview while we get drunk :-).

so long,
Carsten

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Avi Kivity wrote:
> But that doesn't make the code portable.  The s390 userspace has to know 
> how to encode the number, and x86 will do it differently.
> 
> If it's really different, let's keep it different.  Unless you can push 
> the encoding so far back it's transparent to userspace (e.g. qemu).
I agree that we should keep it seperate unless it makes sense to have 
commonality.
A paravirt driver for example could make use of this abstraction: it 
could request an interrupt, and hand the __u64 that it got back to a 
function that actually sends the interrupt over.
But for now, I agree we should keep it seperate. I am just thinking 
loud here.

>> In addition, I would love to be able to specify which target CPUs may 
>> receive that interrupt because our IPI equivalent comes out just like 
>> a regular interrupt on just one target CPU.
>>
>> That boils down to something like this:
>> struct kvm_interrupt_data {
>>     __u64     interrupt_number;
>>     cpuset_t possible_target_cpus;
>> }
>> and an KVM_INJECT_INTERRUPT common ioctl for the vm to provide this.
> 
> Are cpusets exported to userspace?
> 
> x86 has something similar (IPI to a set of cpus) but it's handled 100% 
> in the kernel these days.
> 
No they are'nt. We'd need to come up with a different data structure 
for that. Does IPI have an interrupt number too?

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Avi Kivity]

Carsten Otte wrote:
>
>>> In addition, I would love to be able to specify which target CPUs 
>>> may receive that interrupt because our IPI equivalent comes out just 
>>> like a regular interrupt on just one target CPU.
>>>
>>> That boils down to something like this:
>>> struct kvm_interrupt_data {
>>>     __u64     interrupt_number;
>>>     cpuset_t possible_target_cpus;
>>> }
>>> and an KVM_INJECT_INTERRUPT common ioctl for the vm to provide this.
>>
>> Are cpusets exported to userspace?
>>
>> x86 has something similar (IPI to a set of cpus) but it's handled 
>> 100% in the kernel these days.
>>
> No they are'nt. We'd need to come up with a different data structure 
> for that.

A bitmap would do it, but what size?  Expandable ones are messy.

> Does IPI have an interrupt number too?

No, it's a command (mmio) to the APIC, you tell it which vector you want 
and to which cpus you want it delivered.  So you can have many IPI 
interrupt vectors.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Avi Kivity wrote:
> A bitmap would do it, but what size?  Expandable ones are messy.
We could have a #define KVM_CPU_BITMAP_SIZE in the arch specific 
header files that go to include/asm/. For s390, we have one of our 
rocket science virtualization accelerating facilities that limits us 
to 64 cpus per guest. This may well be extended later on, but for now 
that would be sufficient. Thinking about Christoph Lameter with his 4k 
CPU boxes, I believe ia64 would want faaaar more than that.

> No, it's a command (mmio) to the APIC, you tell it which vector you want 
> and to which cpus you want it delivered.  So you can have many IPI 
> interrupt vectors.
I see. But the interrupt vector can be encoded in __u64?



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Avi Kivity]

Carsten Otte wrote:
> Avi Kivity wrote:
>> A bitmap would do it, but what size?  Expandable ones are messy.
> We could have a #define KVM_CPU_BITMAP_SIZE in the arch specific 
> header files that go to include/asm/. For s390, we have one of our 
> rocket science virtualization accelerating facilities that limits us 
> to 64 cpus per guest. This may well be extended later on, but for now 
> that would be sufficient. Thinking about Christoph Lameter with his 4k 
> CPU boxes, I believe ia64 would want faaaar more than that.
>

If there's a single variable length array (which is the case here) it 
can be tucked on at the end:

    struct kvm_ipi {
          __u64 vector;
          __u32 size; /* bytes, must be multiple of 8 */
          __u32 pad;
          __u64 cpuset[0];
     };

We have this in a few places.  Not pretty, but serviceable.

>> No, it's a command (mmio) to the APIC, you tell it which vector you 
>> want and to which cpus you want it delivered.  So you can have many 
>> IPI interrupt vectors.
> I see. But the interrupt vector can be encoded in __u64?
>

The vector is just a u8.

The x86 interrupt path looks like this:

   [devices] -- irq --> [interrupt controllers] ---- vector ---> [processor]

The interrupt controllers translate irq lines into vectors, which the 
processor consumes.  Before kvm-irqchip, the API taked about vectors 
since the interrupt controller was in userspace.  Nowadays userspace 
talks irq lines to the kernel, which converts them into vectors.

If I uderstand correctly, s390 is interrupt vector oriented, no?

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Dong, Eddie]

kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org wrote:
> Avi Kivity wrote:
>> A bitmap would do it, but what size?  Expandable ones are messy.
> We could have a #define KVM_CPU_BITMAP_SIZE in the arch specific
> header files that go to include/asm/. For s390, we have one of our
> rocket science virtualization accelerating facilities that limits us
> to 64 cpus per guest. This may well be extended later on, but for now
> that would be sufficient. Thinking about Christoph Lameter with his 4k
> CPU boxes, I believe ia64 would want faaaar more than that.
> 
IA64/KVM will handle interrupt in kernel including IPI IMO, so what
user level need to tell kernel is which platform IRQ pin is set/cleared.

Can't S390 do in similar way? From platform point of view, each
irq can have a unique # and the device itself doesn;t need to know
 which CPU will receive it.  Are talking about having your interrupt
 controller in user space? or I missed something. 

Love to study the spec later :-)
Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Dong, Eddie wrote:
> IA64/KVM will handle interrupt in kernel including IPI IMO, so what
> user level need to tell kernel is which platform IRQ pin is set/cleared.
> 
> Can't S390 do in similar way? From platform point of view, each
> irq can have a unique # and the device itself doesn;t need to know
>  which CPU will receive it.  Are talking about having your interrupt
>  controller in user space? or I missed something. 
We don't have interrupt controllers in the first place, and therefore 
we don't need to emulate them. We want to handle IPI inside the kernel 
too, and we also need to be able to inject interrupts from userspace.
Would you be able to encode your interrupt related information into an 
__u64 data type? Do all CPUs have the same interrupts pending, or is 
the information per-cpu? Does the data structure that Avi suggested 
fit your interrupt injection needs?

struct kvm_interrupt {
          __u64 vector;
          __u32 size; /* bytes, must be multiple of 8 */
          __u32 pad;
          __u64 cpuset[0];
     };

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Dong, Eddie]

Carsten Otte wrote:
> Dong, Eddie wrote:
>> IA64/KVM will handle interrupt in kernel including IPI IMO, so what
>> user level need to tell kernel is which platform IRQ pin is
>> set/cleared. 
>> 
>> Can't S390 do in similar way? From platform point of view, each
>> irq can have a unique # and the device itself doesn;t need to know
>>  which CPU will receive it.  Are talking about having your interrupt
>>  controller in user space? or I missed something.
> We don't have interrupt controllers in the first place, and therefore
> we don't need to emulate them. We want to handle IPI inside the kernel
> too, and we also need to be able to inject interrupts from userspace.
> Would you be able to encode your interrupt related information into an
> __u64 data type? Do all CPUs have the same interrupts pending, or is
> the information per-cpu? Does the data structure that Avi suggested
> fit your interrupt injection needs?
> 
> struct kvm_interrupt {
>          __u64 vector;
>          __u32 size; /* bytes, must be multiple of 8 */         
>          __u32 pad; __u64 cpuset[0];
>     };

Since IA64 & X86 doesn't carry CPU info in the KVM_IRQ_LINE,
only irq # is carried, so a u32 is enough, but definitely structure like
above can be too.

BTW, why we use vector here? shouldn't it be irq_line or irq_no?
Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Dong, Eddie]

> BTW, why we use vector here? shouldn't it be irq_line or irq_no?

Maybe you mean the Channel Subsystem (1st piece of knowledge and 
surprise known from s390 doc)  are emulated in Qemu, correct?

Eddie

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Dong, Eddie wrote:
>> BTW, why we use vector here? shouldn't it be irq_line or irq_no?
> 
> Maybe you mean the Channel Subsystem (1st piece of knowledge and 
> surprise known from s390 doc)  are emulated in Qemu, correct?
The vector field was introduced by Avi's comment. I just copied that 
over.
On s390, we only have irq numbers, no vectors. For now, we don't want 
to emulate the channel subsystem, just paravirt. Technically, we could 
do a passthrough in the long term just like pci devices can be 
dedicated to a guest using an iommu in the memory mapped I/O world.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Arnd Bergmann]

On Monday 05 November 2007, Carsten Otte wrote:
> Dong, Eddie wrote:
> >> BTW, why we use vector here? shouldn't it be irq_line or irq_no?
> > 
> > Maybe you mean the Channel Subsystem (1st piece of knowledge and 
> > surprise known from s390 doc)  are emulated in Qemu, correct?
> The vector field was introduced by Avi's comment. I just copied that 
> over.
> On s390, we only have irq numbers, no vectors.

Actually, you have neither irq numbers nor vectors on s390 right now.
I/O subchannels are do not fit into the IRQ handling in Linux at
all, and external interrupts are sufficiently different that you
should not treat them as IRQ lines in Linux.

However, I would suggest that you use either one external interrupt or
the "thin" interrupt as an event source for an interrupt controller for
all the virtio devices, and use the generic IRQ subsystem for that,
including interrupt lines and vectors.

In case of the thin interrupt, your virtual interrupt controller would
more or less just consist of one lowcore address from which you can
read the pending interrupt vector after an interrupt has been caused,
as well as a single hcall that does a 'acknowledge interrupt, get
next pending irq vector into lowcore and tell me whether there was
one' operation.

You'll also need an operation to associate a virtio device with an
interrupt vector, but that belongs into virtio.

	Arnd <><

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Carsten Otte]

Arnd Bergmann wrote:
> On Monday 05 November 2007, Carsten Otte wrote:
>> Dong, Eddie wrote:
>>>> BTW, why we use vector here? shouldn't it be irq_line or irq_no?
>>> Maybe you mean the Channel Subsystem (1st piece of knowledge and 
>>> surprise known from s390 doc)  are emulated in Qemu, correct?
>> The vector field was introduced by Avi's comment. I just copied that 
>> over.
>> On s390, we only have irq numbers, no vectors.
> 
> Actually, you have neither irq numbers nor vectors on s390 right now.
> I/O subchannels are do not fit into the IRQ handling in Linux at
> all, and external interrupts are sufficiently different that you
> should not treat them as IRQ lines in Linux.
We're not emulating the I/O subsystem, and thus no I/O subchannels.

> However, I would suggest that you use either one external interrupt or
> the "thin" interrupt as an event source for an interrupt controller for
> all the virtio devices, and use the generic IRQ subsystem for that,
> including interrupt lines and vectors.
> 
> In case of the thin interrupt, your virtual interrupt controller would
> more or less just consist of one lowcore address from which you can
> read the pending interrupt vector after an interrupt has been caused,
> as well as a single hcall that does a 'acknowledge interrupt, get
> next pending irq vector into lowcore and tell me whether there was
> one' operation.
> 
> You'll also need an operation to associate a virtio device with an
> interrupt vector, but that belongs into virtio.

The irq subsystem does not fit the external interrupt model, and you'd 
definitely want to argue with Martin before suggesting to introduce 
the IRQ subsystem on s390. "Only over my dead body" was the last 
statement I do remember.
Plus I don't see a benefit from pretending to have an interrupt 
controller: virtio abstracts from this, and can well be implemented 
over extint and hypercall like Christian has done it. What's the 
problem you're trying to solve?

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Arnd Bergmann]

On Monday 05 November 2007, Carsten Otte wrote:
> > Actually, you have neither irq numbers nor vectors on s390 right now.
> > I/O subchannels are do not fit into the IRQ handling in Linux at
> > all, and external interrupts are sufficiently different that you
> > should not treat them as IRQ lines in Linux.

<snip>

> The irq subsystem does not fit the external interrupt model, and you'd 
> definitely want to argue with Martin before suggesting to introduce 
> the IRQ subsystem on s390. "Only over my dead body" was the last 
> statement I do remember.

Read again what I wrote above. I'm suggesting to have just one external
interrupt for virtio and use the generic IRQ abstraction to handle
everything that comes below that.

> Plus I don't see a benefit from pretending to have an interrupt 
> controller: virtio abstracts from this, and can well be implemented 
> over extint and hypercall like Christian has done it. What's the 
> problem you're trying to solve?

Sorry, I can't find Christian's code right now, do you have a pointer
to the patches?

I suspect that he has done exactly what I was trying to explain, except
that the implementation is not using the generic IRQ layer, which means
you're duplicating some of the code.

	Arnd <><

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/

Re: RFC/patch portability: split kvm_vm_ioctl v3

[Christian Borntraeger]

Am Montag, 5. November 2007 schrieb Arnd Bergmann:
> Read again what I wrote above. I'm suggesting to have just one external
> interrupt for virtio and use the generic IRQ abstraction to handle
> everything that comes below that.

So you basically suggest to implement wrapper code around extint and lowcore
memory to be able to use request_irq/free_irq?

> > Plus I don't see a benefit from pretending to have an interrupt 
> > controller: virtio abstracts from this, and can well be implemented 
> > over extint and hypercall like Christian has done it. What's the 
> > problem you're trying to solve?
> 
> Sorry, I can't find Christian's code right now, do you have a pointer
> to the patches?

The code was only used for our prototype hypervisor. I never posted these
virtio patches as Rusty was quicker in changing virtio than I was able to
re-add them to our prototype code. ;-)

> I suspect that he has done exactly what I was trying to explain, except
> that the implementation is not using the generic IRQ layer, which means
> you're duplicating some of the code.

I used one external interrupt and I reserved an area in lowcore for a 64bit
extint parameter. (I use the same address as z/VM for the PFAULT token). I 
defined a hypercall in which the guest could specify this 64bit value for a
given virtqueue. That allowed me to get the virtqueue pointer without looking
it up in the list of (maybe many) virtqueues. 

Christian

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/