On 05/05/2013 04:03:08 PM, Benjamin Herrenschmidt wrote:
On Fri, 2013-05-03 at 18:45 -0500, Scott Wood wrote:
> kvmppc_lazy_ee_enable() was causing interrupts to be soft-enabled
> (albeit hard-disabled) in kvmppc_restart_interrupt(). This led to
> warnings, and possibly breakage if the interrupt state was later
saved
> and then restored (leading to interrupts being hard-and-soft enabled
> when they should be at least soft-disabled).
>
> Simply removing kvmppc_lazy_ee_enable() leaves interrupts only
> soft-disabled when we enter the guest, but they will be
hard-disabled
> when we exit the guest -- without PACA_IRQ_HARD_DIS ever being set,
so
> the local_irq_enable() fails to hard-enable.
>
> While we could just set PACA_IRQ_HARD_DIS after an exit to
compensate,
> instead hard-disable interrupts before entering the guest. This
way,
> we won't have to worry about interactions if we take an interrupt
> during the guest entry code. While I don't see any obvious
> interactions, it could change in the future (e.g. it would be bad if
> the non-hv code were used on 64-bit or if 32-bit guest lazy
interrupt
> disabling, since the non-hv code changes IVPR among other things).
Shouldn't the interrupts be marked soft-enabled (even if hard
disabled)
when entering the guest ?
Ie. The last stage of entry will hard enable, so they should be
soft-enabled too... if not, latency trackers will consider the whole
guest periods as "interrupt disabled"...
OK... I guess we already have that problem on 32-bit as well?
Now, kvmppc_lazy_ee_enable() seems to be clearly bogus to me. It will
unconditionally set soft_enabled and clear irq_happened from a
soft-disabled state, thus potentially losing a pending event.
Book3S "HV" seems to be keeping interrupts fully enabled all the way
until the asm hard disables, which would be fine except that I'm
worried
we are racy vs. need_resched & signals.
One thing you may be able to do is call prep_irq_for_idle(). This will
tell you if something happened, giving you a chance to abort/re-enable
before you go the guest.
As long as we go straight from IRQs fully enabled to hard-disabled,
before we check for signals and such, I don't think we need that (and
using it would raise the question of what to do on 32-bit).
What if we just take this patch, and add trace_hardirqs_on() just
before entering the guest? This would be similar to what the 32-bit
non-KVM exception return code does (except it would be in C code).
Perhaps we could set soft_enabled as well, but then we'd have to clear
it again before calling kvmppc_restart_interrupt() -- since the KVM
exception handlers don't actually care about soft_enabled (it would
just be for consistency), I'd rather just leave soft_enabled off.
We also don't want PACA_IRQ_HARD_DIS to be cleared the way
prep_irq_for_idle() does, because that's what lets the
local_irq_enable() do the hard-enabling after we exit the guest.
-Scott
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
