From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Hansen Subject: [PATCH 1/4] reduce kvm stack usage in kvm_arch_vm_ioctl() Date: Mon, 28 Jul 2008 11:15:09 -0700 Message-ID: <1217268909-13349-1-git-send-email-dave@linux.vnet.ibm.com> Cc: kvm-devel , Anthony Liguori , Dave Hansen To: Avi Kivity Return-path: Received: from e4.ny.us.ibm.com ([32.97.182.144]:55815 "EHLO e4.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755615AbYG1SPM (ORCPT ); Mon, 28 Jul 2008 14:15:12 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e4.ny.us.ibm.com (8.13.8/8.13.8) with ESMTP id m6SIFCdb016796 for ; Mon, 28 Jul 2008 14:15:12 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v9.0) with ESMTP id m6SIFBY9157670 for ; Mon, 28 Jul 2008 14:15:11 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m6SIFB7g003135 for ; Mon, 28 Jul 2008 14:15:11 -0400 Sender: kvm-owner@vger.kernel.org List-ID: On my machine with gcc 3.4, kvm uses ~2k of stack in a few select functions. This is mostly because gcc fails to notice that the different case: statements could have their stack usage combined. It overflows very nicely if interrupts happen during one of these large uses. This patch uses two methods for reducing stack usage. 1. dynamically allocate large objects instead of putting on the stack. 2. Use a union{} member for all of the case variables. This tricks gcc into combining them all into a single stack allocation. Signed-off-by: Dave Hansen --- arch/x86/kvm/x86.c | 116 ++++++++++++++++++++++++++++++++-------------------- 1 files changed, 72 insertions(+), 44 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 5916191..c8f94ae 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1621,12 +1621,73 @@ out: return r; } +static inline int kvm_arch_vm_irqchip_ioctl(struct kvm *kvm, void *argp, + unsigned int ioctl) +{ + int ret = 0; + struct kvm_irqchip *chip = kmalloc(sizeof(struct kvm_irqchip), GFP_KERNEL); + + if (!chip) + return -ENOMEM; + + /* cheaper than the copy, so do this first */ + if (!irqchip_in_kernel(kvm)) { + ret = -ENXIO; + goto out; + } + if (copy_from_user(chip, argp, sizeof(struct kvm_irqchip))) { + ret = -EFAULT; + goto out; + } + switch (ioctl) { + case KVM_GET_IRQCHIP: + ret = kvm_vm_ioctl_get_irqchip(kvm, chip); + if (ret) + goto out; + ret = copy_to_user(argp, chip, sizeof(struct kvm_irqchip)); + if (ret) { + ret = -EFAULT; + goto out; + } + break; + case KVM_SET_IRQCHIP: + ret = kvm_vm_ioctl_set_irqchip(kvm, chip); + break; + default: + ret = -EINVAL; + break; + } +out: + kfree(chip); + return ret; +} + + +static inline int x86_kvm_vm_ioctl_set_memory_region(struct kvm *kvm, void *argp) +{ + struct kvm_memory_region kvm_mem; + struct kvm_userspace_memory_region kvm_userspace_mem; + + if (copy_from_user(&kvm_mem, argp, sizeof(struct kvm_memory_region))) + return -EFAULT; + 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; + return kvm_vm_ioctl_set_memory_region(kvm, &kvm_userspace_mem, 0); +} + 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; + union { + /* 0: PIC master, 1: PIC slave, 2: IOAPIC */ + struct kvm_pit_state ps; + struct kvm_memory_alias alias; + } u; switch (ioctl) { case KVM_SET_TSS_ADDR: @@ -1658,17 +1719,14 @@ long kvm_arch_vm_ioctl(struct file *filp, 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; - + case KVM_SET_MEMORY_ALIAS: r = -EFAULT; - if (copy_from_user(&alias, argp, sizeof alias)) + if (copy_from_user(&u.alias, argp, sizeof(struct kvm_memory_alias))) goto out; - r = kvm_vm_ioctl_set_memory_alias(kvm, &alias); + r = kvm_vm_ioctl_set_memory_alias(kvm, &u.alias); if (r) goto out; break; - } case KVM_CREATE_IRQCHIP: r = -ENOMEM; kvm->arch.vpic = kvm_create_pic(kvm); @@ -1708,67 +1766,37 @@ long kvm_arch_vm_ioctl(struct file *filp, } 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); + case KVM_GET_IRQCHIP: + case KVM_SET_IRQCHIP: + r = kvm_arch_vm_irqchip_ioctl(kvm, argp, ioctl); if (r) goto out; r = 0; break; - } case KVM_GET_PIT: { - struct kvm_pit_state ps; r = -EFAULT; - if (copy_from_user(&ps, argp, sizeof ps)) + if (copy_from_user(&u.ps, argp, sizeof(struct kvm_pit_state))) goto out; r = -ENXIO; if (!kvm->arch.vpit) goto out; - r = kvm_vm_ioctl_get_pit(kvm, &ps); + r = kvm_vm_ioctl_get_pit(kvm, &u.ps); if (r) goto out; r = -EFAULT; - if (copy_to_user(argp, &ps, sizeof ps)) + if (copy_to_user(argp, &u.ps, sizeof(struct kvm_pit_state))) goto out; r = 0; break; } case KVM_SET_PIT: { - struct kvm_pit_state ps; r = -EFAULT; - if (copy_from_user(&ps, argp, sizeof ps)) + if (copy_from_user(&u.ps, argp, sizeof u.ps)) goto out; r = -ENXIO; if (!kvm->arch.vpit) goto out; - r = kvm_vm_ioctl_set_pit(kvm, &ps); + r = kvm_vm_ioctl_set_pit(kvm, &u.ps); if (r) goto out; r = 0; -- 1.5.4.3