On Thu, Apr 26, 2012 at 12:19:03PM +0200, Alexander Graf wrote: > So switch the code over to call into the Linux C handlers from C code, > speeding up everything along the way. I have to say this patch makes me pretty uneasy. There are a few things that look wrong to me, but more than that, it seems to me that there would be a lot of careful thought needed to make sure that the approach is bullet-proof. The first thing is that you are filling in the registers, and in particular r1, in a subroutine, so you are potentially making regs.r1 point to a stack frame that no longer exists by the time we look at it inside do_IRQ or timer_interrupt. So, for example, a stack trace could go completely off the rails at that point. Quite possibly gcc will inline the kvmppc_fill_pt_regs function, which would probably save you, but I don't think you should rely on that. The second thing is, why do you save just r1, ip, msr, and lr? Why those ones and no others? A performance monitor interrupt might well decide to save all the registers away plus a stack trace, and to see all the GPRs as 0 could be very confusing. Thirdly, if preemption is enabled, it could well be that the interrupt will wake up a higher priority task which should run before we continue. On 64-bit you are probably saved by the soft_irq_enable calls, which will (I think) call schedule() if a reschedule is pending, but on 32-bit soft_irq_enable does nothing. Fourthly, as Ben said, you should be setting regs->trap. Have you measured a performance improvement with this patch? If so how big was it? Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
