From 67c50bf292c1c02ec7ca548049c53cc08dc75ed1 Mon Sep 17 00:00:00 2001 From: Martin Kepplinger Date: Tue, 24 Mar 2015 11:51:20 +1030 Subject: lguest: explicitly set miscdevice's private_data NULL There is a proposed change to the miscdevice's behaviour on open(). Currently file->private_data stays NULL, but only because we don't have an open-entry in struct file_operations. This may change so that private_data, more consistently, is always set to struct miscdevice, not only *if* the driver has it's own open() routine and fops-entry, see https://lkml.org/lkml/2014/12/4/939 and commit 94e4fe2cab3d43b3ba7c3f721743006a8c9d913a In short: If we rely on file->private_data being NULL, we should ensure it is NULL ourselves. Signed-off-by: Martin Kepplinger Signed-off-by: Rusty Russell --- drivers/lguest/lguest_user.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/lguest') diff --git a/drivers/lguest/lguest_user.c b/drivers/lguest/lguest_user.c index c4c6113eb9a6..30c60687d277 100644 --- a/drivers/lguest/lguest_user.c +++ b/drivers/lguest/lguest_user.c @@ -339,6 +339,13 @@ static ssize_t write(struct file *file, const char __user *in, } } +static int open(struct inode *inode, struct file *file) +{ + file->private_data = NULL; + + return 0; +} + /*L:060 * The final piece of interface code is the close() routine. It reverses * everything done in initialize(). This is usually called because the @@ -409,6 +416,7 @@ static int close(struct inode *inode, struct file *file) */ static const struct file_operations lguest_fops = { .owner = THIS_MODULE, + .open = open, .release = close, .write = write, .read = read, -- cgit v1.2.3 From 2f921b5bb0511fb698681d8ef35c48be7a9116bf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 24 Mar 2015 11:51:39 +1030 Subject: lguest: suppress interrupts for single insn, not range. The last patch reduced our interrupt-suppression region to one address, so simplify the code somewhat. Also, remove the obsolete undefined instruction ranges and the comment which refers to lguest_guest.S instead of head_32.S. Signed-off-by: Rusty Russell --- drivers/lguest/hypercalls.c | 5 ++--- drivers/lguest/interrupts_and_traps.c | 8 ++++---- drivers/lguest/lg.h | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) (limited to 'drivers/lguest') diff --git a/drivers/lguest/hypercalls.c b/drivers/lguest/hypercalls.c index 1219af493c0f..19a32280731d 100644 --- a/drivers/lguest/hypercalls.c +++ b/drivers/lguest/hypercalls.c @@ -211,10 +211,9 @@ static void initialize(struct lg_cpu *cpu) /* * The Guest tells us where we're not to deliver interrupts by putting - * the range of addresses into "struct lguest_data". + * the instruction address into "struct lguest_data". */ - if (get_user(cpu->lg->noirq_start, &cpu->lg->lguest_data->noirq_start) - || get_user(cpu->lg->noirq_end, &cpu->lg->lguest_data->noirq_end)) + if (get_user(cpu->lg->noirq_iret, &cpu->lg->lguest_data->noirq_iret)) kill_guest(cpu, "bad guest page %p", cpu->lg->lguest_data); /* diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c index 70dfcdc29f1f..6d4c072b61e1 100644 --- a/drivers/lguest/interrupts_and_traps.c +++ b/drivers/lguest/interrupts_and_traps.c @@ -204,8 +204,7 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more) * They may be in the middle of an iret, where they asked us never to * deliver interrupts. */ - if (cpu->regs->eip >= cpu->lg->noirq_start && - (cpu->regs->eip < cpu->lg->noirq_end)) + if (cpu->regs->eip == cpu->lg->noirq_iret) return; /* If they're halted, interrupts restart them. */ @@ -395,8 +394,9 @@ static bool direct_trap(unsigned int num) * The Guest has the ability to turn its interrupt gates into trap gates, * if it is careful. The Host will let trap gates can go directly to the * Guest, but the Guest needs the interrupts atomically disabled for an - * interrupt gate. It can do this by pointing the trap gate at instructions - * within noirq_start and noirq_end, where it can safely disable interrupts. + * interrupt gate. The Host could provide a mechanism to register more + * "no-interrupt" regions, and the Guest could point the trap gate at + * instructions within that region, where it can safely disable interrupts. */ /*M:006 diff --git a/drivers/lguest/lg.h b/drivers/lguest/lg.h index 307e8b39e7d1..ac8ad0461e80 100644 --- a/drivers/lguest/lg.h +++ b/drivers/lguest/lg.h @@ -102,7 +102,7 @@ struct lguest { struct pgdir pgdirs[4]; - unsigned long noirq_start, noirq_end; + unsigned long noirq_iret; unsigned int stack_pages; u32 tsc_khz; -- cgit v1.2.3 From 3eebd233fcb392286d385e764eb91e90d6218cdf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 1 Apr 2015 13:32:20 +1030 Subject: lguest: handle traps on the "interrupt suppressed" iret instruction. Lguest's "iret" is non-atomic, as it needs to restore the interrupt state before the real iret (the guest can't actually suppress interrupts). For this reason, the host discards an interrupt if it occurs in this (1-instruction) window. We can do better, by emulating the iret execution, then immediately setting up the interrupt handler. In fact, we don't need to do much, as emulating the iret and setting up th stack for the interrupt handler basically cancel each other out. Signed-off-by: Rusty Russell --- drivers/lguest/interrupts_and_traps.c | 99 +++++++++++++++++++++++++---------- 1 file changed, 72 insertions(+), 27 deletions(-) (limited to 'drivers/lguest') diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c index 6d4c072b61e1..5e7559be222a 100644 --- a/drivers/lguest/interrupts_and_traps.c +++ b/drivers/lguest/interrupts_and_traps.c @@ -56,21 +56,16 @@ static void push_guest_stack(struct lg_cpu *cpu, unsigned long *gstack, u32 val) } /*H:210 - * The set_guest_interrupt() routine actually delivers the interrupt or - * trap. The mechanics of delivering traps and interrupts to the Guest are the - * same, except some traps have an "error code" which gets pushed onto the - * stack as well: the caller tells us if this is one. - * - * "lo" and "hi" are the two parts of the Interrupt Descriptor Table for this - * interrupt or trap. It's split into two parts for traditional reasons: gcc - * on i386 used to be frightened by 64 bit numbers. + * The push_guest_interrupt_stack() routine saves Guest state on the stack for + * an interrupt or trap. The mechanics of delivering traps and interrupts to + * the Guest are the same, except some traps have an "error code" which gets + * pushed onto the stack as well: the caller tells us if this is one. * * We set up the stack just like the CPU does for a real interrupt, so it's * identical for the Guest (and the standard "iret" instruction will undo * it). */ -static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi, - bool has_err) +static void push_guest_interrupt_stack(struct lg_cpu *cpu, bool has_err) { unsigned long gstack, origstack; u32 eflags, ss, irq_enable; @@ -130,12 +125,28 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi, if (has_err) push_guest_stack(cpu, &gstack, cpu->regs->errcode); - /* - * Now we've pushed all the old state, we change the stack, the code - * segment and the address to execute. - */ + /* Adjust the stack pointer and stack segment. */ cpu->regs->ss = ss; cpu->regs->esp = virtstack + (gstack - origstack); +} + +/* + * This actually makes the Guest start executing the given interrupt/trap + * handler. + * + * "lo" and "hi" are the two parts of the Interrupt Descriptor Table for this + * interrupt or trap. It's split into two parts for traditional reasons: gcc + * on i386 used to be frightened by 64 bit numbers. + */ +static void guest_run_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi) +{ + /* If we're already in the kernel, we don't change stacks. */ + if ((cpu->regs->ss&0x3) != GUEST_PL) + cpu->regs->ss = cpu->esp1; + + /* + * Set the code segment and the address to execute. + */ cpu->regs->cs = (__KERNEL_CS|GUEST_PL); cpu->regs->eip = idt_address(lo, hi); @@ -158,6 +169,24 @@ static void set_guest_interrupt(struct lg_cpu *cpu, u32 lo, u32 hi, kill_guest(cpu, "Disabling interrupts"); } +/* This restores the eflags word which was pushed on the stack by a trap */ +static void restore_eflags(struct lg_cpu *cpu) +{ + /* This is the physical address of the stack. */ + unsigned long stack_pa = guest_pa(cpu, cpu->regs->esp); + + /* + * Stack looks like this: + * Address Contents + * esp EIP + * esp + 4 CS + * esp + 8 EFLAGS + */ + cpu->regs->eflags = lgread(cpu, stack_pa + 8, u32); + cpu->regs->eflags &= + ~(X86_EFLAGS_TF|X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT); +} + /*H:205 * Virtual Interrupts. * @@ -200,13 +229,6 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more) BUG_ON(irq >= LGUEST_IRQS); - /* - * They may be in the middle of an iret, where they asked us never to - * deliver interrupts. - */ - if (cpu->regs->eip == cpu->lg->noirq_iret) - return; - /* If they're halted, interrupts restart them. */ if (cpu->halted) { /* Re-enable interrupts. */ @@ -236,12 +258,34 @@ void try_deliver_interrupt(struct lg_cpu *cpu, unsigned int irq, bool more) if (idt_present(idt->a, idt->b)) { /* OK, mark it no longer pending and deliver it. */ clear_bit(irq, cpu->irqs_pending); + /* - * set_guest_interrupt() takes the interrupt descriptor and a - * flag to say whether this interrupt pushes an error code onto - * the stack as well: virtual interrupts never do. + * They may be about to iret, where they asked us never to + * deliver interrupts. In this case, we can emulate that iret + * then immediately deliver the interrupt. This is basically + * a noop: the iret would pop the interrupt frame and restore + * eflags, and then we'd set it up again. So just restore the + * eflags word and jump straight to the handler in this case. + * + * Denys Vlasenko points out that this isn't quite right: if + * the iret was returning to userspace, then that interrupt + * would reset the stack pointer (which the Guest told us + * about via LHCALL_SET_STACK). But unless the Guest is being + * *really* weird, that will be the same as the current stack + * anyway. */ - set_guest_interrupt(cpu, idt->a, idt->b, false); + if (cpu->regs->eip == cpu->lg->noirq_iret) { + restore_eflags(cpu); + } else { + /* + * set_guest_interrupt() takes a flag to say whether + * this interrupt pushes an error code onto the stack + * as well: virtual interrupts never do. + */ + push_guest_interrupt_stack(cpu, false); + } + /* Actually make Guest cpu jump to handler. */ + guest_run_interrupt(cpu, idt->a, idt->b); } /* @@ -352,8 +396,9 @@ bool deliver_trap(struct lg_cpu *cpu, unsigned int num) */ if (!idt_present(cpu->arch.idt[num].a, cpu->arch.idt[num].b)) return false; - set_guest_interrupt(cpu, cpu->arch.idt[num].a, - cpu->arch.idt[num].b, has_err(num)); + push_guest_interrupt_stack(cpu, has_err(num)); + guest_run_interrupt(cpu, cpu->arch.idt[num].a, + cpu->arch.idt[num].b); return true; } -- cgit v1.2.3