KVM uses a lot of kernel stack on x86, especially with gcc 3.x. It
likes to overflow it and make my poor machine go boom. This patch takes
the worst stack users and makes them use kmalloc(). It also saves ~30
bytes in kvm_arch_vm_ioctl() by using a union.
I haven't tested this at all yet. Just posting to get some comments.
Avi, you said that one of these was a hot path. Which one is that? I
kinda doubt you'll be able to see any impact anyway.
before:
0x000042d3 kvm_arch_vcpu_ioctl [kvm]: 2148
0x000012e3 kvm_vcpu_ioctl [kvm]: 1620
0x00004a83 kvm_arch_vm_ioctl [kvm]: 1332
0x0000d472 kvm_pv_mmu_op [kvm]: 536
0x0000d4eb kvm_pv_mmu_op [kvm]: 536
0x00000476 __kvm_set_memory_region [kvm]: 184
0x000007e3 __kvm_set_memory_region [kvm]: 184
0x00007eb3 kvm_task_switch_32 [kvm]: 128
0x0000b553 paging64_page_fault [kvm]: 112
0x0000bfa3 paging32_page_fault [kvm]: 112
0x0000e48b x86_emulate_insn [kvm]: 112
0x0000e785 x86_emulate_insn [kvm]: 112
after:
0x00000476 __kvm_set_memory_region [kvm]: 184
0x000007e3 __kvm_set_memory_region [kvm]: 184
0x00004c83 kvm_arch_vm_ioctl [kvm]: 164
0x000012e3 kvm_vcpu_ioctl [kvm]: 152
0x00008013 kvm_task_switch_32 [kvm]: 128
0x0000b6b3 paging64_page_fault [kvm]: 112
0x0000c103 paging32_page_fault [kvm]: 112
0x0000e5fb x86_emulate_insn [kvm]: 112
0x0000e8f5 x86_emulate_insn [kvm]: 112
0x00004353 kvm_arch_vcpu_ioctl [kvm]: 104
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 7e7c396..f4e62f2 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2154,18 +2154,24 @@ int kvm_pv_mmu_op(struct kvm_vcpu *vcpu, unsigned long
bytes,
gpa_t addr, unsigned long *ret)
{
int r;
- struct kvm_pv_mmu_op_buffer buffer;
+ struct kvm_pv_mmu_op_buffer *buffer =
+ kmalloc(GFP_KERNEL, sizeof(struct kvm_pv_mmu_op_buffer));
- buffer.ptr = buffer.buf;
- buffer.len = min_t(unsigned long, bytes, sizeof buffer.buf);
- buffer.processed = 0;
+ if (!buffer) {
+ *ret = 0;
+ return -ENOMEM;
+ }
+
+ buffer->ptr = buffer->buf;
+ buffer->len = min_t(unsigned long, bytes, sizeof buffer->buf);
+ buffer->processed = 0;
- r = kvm_read_guest(vcpu->kvm, addr, buffer.buf, buffer.len);
+ r = kvm_read_guest(vcpu->kvm, addr, buffer->buf, buffer->len);
if (r)
goto out;
- while (buffer.len) {
- r = kvm_pv_mmu_op_one(vcpu, &buffer);
+ while (buffer->len) {
+ r = kvm_pv_mmu_op_one(vcpu, buffer);
if (r < 0)
goto out;
if (r == 0)
@@ -2174,7 +2180,8 @@ int kvm_pv_mmu_op(struct kvm_vcpu *vcpu, unsigned long
bytes,
r = 1;
out:
- *ret = buffer.processed;
+ *ret = buffer->processed;
+ kfree(buffer);
return r;
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 63a77ca..dbb0edd 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1283,12 +1283,15 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
struct kvm_vcpu *vcpu = filp->private_data;
void __user *argp = (void __user *)arg;
int r;
+ struct kvm_lapic_state *lapic = NULL;
switch (ioctl) {
case KVM_GET_LAPIC: {
- struct kvm_lapic_state lapic;
+ lapic = kzalloc(GFP_KERNEL, sizeof(*lapic));
- memset(&lapic, 0, sizeof lapic);
+ r = -ENOMEM;
+ if (!lapic)
+ goto out;
r = kvm_vcpu_ioctl_get_lapic(vcpu, &lapic);
if (r)
goto out;
@@ -1299,8 +1302,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
break;
}
case KVM_SET_LAPIC: {
- struct kvm_lapic_state lapic;
-
+ lapic = kmalloc(GFP_KERNEL, sizeof(*lapic));
+ r = -ENOMEM;
+ if (!lapic)
+ goto out;
r = -EFAULT;
if (copy_from_user(&lapic, argp, sizeof lapic))
goto out;
@@ -1402,6 +1407,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
r = -EINVAL;
}
out:
+ if (lapic)
+ kfree(lapic);
return r;
}
@@ -1602,12 +1609,73 @@ out:
return r;
}
+static int kvm_arch_vm_irqchip_ioctl(struct kvm *kvm, void *argp,
+ unsigned int ioctl)
+{
+ int ret = 0;
+ struct kvm_irqchip *chip = kmalloc(GFP_KERNEL, sizeof(*chip));
+
+ 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 *chip)) {
+ 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 *chip);
+ 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;
+}
+
+
+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 kvm_mem))
+ 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:
@@ -1639,17 +1707,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 u.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);
@@ -1689,67 +1754,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 u.ps))
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 u.ps))
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;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2d29e26..be2816a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -905,6 +905,9 @@ static long kvm_vcpu_ioctl(struct file *filp,
struct kvm_vcpu *vcpu = filp->private_data;
void __user *argp = (void __user *)arg;
int r;
+ struct kvm_fpu *fpu = NULL;
+ struct kvm_sregs *kvm_sregs = NULL;
+
if (vcpu->kvm->mm != current->mm)
return -EIO;
@@ -952,25 +955,29 @@ out_free2:
break;
}
case KVM_GET_SREGS: {
- struct kvm_sregs kvm_sregs;
-
- memset(&kvm_sregs, 0, sizeof kvm_sregs);
- r = kvm_arch_vcpu_ioctl_get_sregs(vcpu, &kvm_sregs);
+ kvm_sregs = kzalloc(GFP_KERNEL, sizeof *kvm_sregs);
+ r = -ENOMEM;
+ if (!kvm_sregs)
+ goto out;
+ memset(kvm_sregs, 0, sizeof kvm_sregs);
+ r = kvm_arch_vcpu_ioctl_get_sregs(vcpu, kvm_sregs);
if (r)
goto out;
r = -EFAULT;
- if (copy_to_user(argp, &kvm_sregs, sizeof kvm_sregs))
+ if (copy_to_user(argp, kvm_sregs, sizeof *kvm_sregs))
goto out;
r = 0;
break;
}
case KVM_SET_SREGS: {
- struct kvm_sregs kvm_sregs;
-
+ kvm_sregs = kmalloc(GFP_KERNEL, sizeof kvm_sregs);
+ r = -ENOMEM;
+ if (!kvm_sregs)
+ goto out;
r = -EFAULT;
- if (copy_from_user(&kvm_sregs, argp, sizeof kvm_sregs))
+ if (copy_from_user(kvm_sregs, argp, sizeof *kvm_sregs))
goto out;
- r = kvm_arch_vcpu_ioctl_set_sregs(vcpu, &kvm_sregs);
+ r = kvm_arch_vcpu_ioctl_set_sregs(vcpu, kvm_sregs);
if (r)
goto out;
r = 0;
@@ -1051,25 +1058,28 @@ out_free2:
break;
}
case KVM_GET_FPU: {
- struct kvm_fpu fpu;
-
- memset(&fpu, 0, sizeof fpu);
- r = kvm_arch_vcpu_ioctl_get_fpu(vcpu, &fpu);
+ fpu = kzalloc(GFP_KERNEL, sizeof(*fpu));
+ r = -ENOMEM;
+ if (!fpu)
+ goto out;
+ r = kvm_arch_vcpu_ioctl_get_fpu(vcpu, fpu);
if (r)
goto out;
r = -EFAULT;
- if (copy_to_user(argp, &fpu, sizeof fpu))
+ if (copy_to_user(argp, fpu, sizeof *fpu))
goto out;
r = 0;
break;
}
case KVM_SET_FPU: {
- struct kvm_fpu fpu;
-
+ fpu = kmalloc(GFP_KERNEL, sizeof(*fpu));
+ r = -ENOMEM;
+ if (!fpu)
+ goto out;
r = -EFAULT;
- if (copy_from_user(&fpu, argp, sizeof fpu))
+ if (copy_from_user(fpu, argp, sizeof *fpu))
goto out;
- r = kvm_arch_vcpu_ioctl_set_fpu(vcpu, &fpu);
+ r = kvm_arch_vcpu_ioctl_set_fpu(vcpu, fpu);
if (r)
goto out;
r = 0;
@@ -1079,6 +1089,10 @@ out_free2:
r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
}
out:
+ if (fpu)
+ kfree(fpu);
+ if (kvm_sregs)
+ kfree(kvm_sregs);
return r;
}
-- Dave
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
