On 15.09.2011, at 22:52, Scott Wood wrote: > On 09/08/2011 10:39 AM, Alexander Graf wrote: >> >> On 08.09.2011, at 17:34, Scott Wood wrote: >> >>> On Wed, Sep 07, 2011 at 12:41:35PM +0200, Alexander Graf wrote: >>>> Yes, but why can't we do this in the vcpu thread's context so we only >>>> ever have a single instance accessing the vcpu struct? It makes a lot >>>> of things a lot easier. >>> >>> Why? We don't do it for external interrupts. It would complicate >>> locking, not simplify it -- we'd need to defer the execution to some >>> context which can block, and then kick the guest out of guest mode, and >>> make sure it doesn't reenter before we get the mutex... >> >> We need to kick the vcpu thread out of context anyways because we're >> otherwise delaying delivery of the decr interrupt. So we can just as >> well handle it all there then. Being lazy is good, but please not at >> the cost of latency. > > That means we need a way to tell the thread to do this. Instead of > using set_bit(), you want dedicated variables for pending_decr, > pending_fit, pending_watchdog? And then we check them on every exit? Well, whatever we do we have to kvm_vcpu_kick() the receiving vcpu out of context. So we can just as well tell it to inject its own interrupt and don't have to deal with atomic operations in most cases then, no? > > Besides avoiding the overhead of an atomic operation (premature > optimization), what does this buy us? The ability to move the > dequeue-after-mask from kvmppc_booke_irqprio_deliver() to > kvmppc_set_tcr() (only to be replaced by the check for pending_foo)? > > How is using set_bits() for TSR any different from the set_bit() we use > in kvmppc_booke_queue_irqprio()? It keeps guest visible registers vcpu private. I want the vcpu thread to own all vcpu registers - and TSR is a vcpu register. > >>>> And for the watchdog, we don't have to set the bit unless it's clean, >>>> right? >>> >>> Not sure what you mean by "clean". We always set the bit when the timer >>> expires. >>> >>> Likewise for the decrementer. >> >> What I mean is that we don't want to have a timer periodically triggering in the background for watchdog when the guest never clears the watchdog delivered bit. > > I suppose we could stop the timer if the watchdog has hit final > expiration with no action configured, and restart it if the bits get > cleared. It's not really a case that warrants optimization (in the > default case where the guest doesn't use the watchdog, the period will > be so high that the timer will never expire), but we do want to avoid a > situation where the guest sets a period that is too short and ends up > timer-storming the host. or where the guest simply has the watchdog only used once, then keeps the irq masked and we still happily burn CPU by triggering a timer when it's not even affecting state. Yup :) Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
