On 05/07/2013 07:50 AM, Scott Wood wrote:
On 05/05/2013 10:13:17 PM, tiejun.chen wrote:
On 05/06/2013 11:10 AM, Tiejun Chen wrote:
For the external interrupt, the decrementer exception and the doorbell
excpetion, we also need to soft-disable interrupts while doing as host
interrupt handlers since the DO_KVM hook is always performed to skip
EXCEPTION_COMMON then miss this original chance with the 'ints' (INTS_DISABLE).

http://patchwork.ozlabs.org/patch/241344/
http://patchwork.ozlabs.org/patch/241412/

:-)

I'm observing the same behaviour as well:

        WARN_ON_ONCE(!irqs_disabled());


Signed-off-by: Tiejun Chen <tiejun.c...@windriver.com>
---
  arch/powerpc/kvm/bookehv_interrupts.S |    9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/arch/powerpc/kvm/bookehv_interrupts.S
b/arch/powerpc/kvm/bookehv_interrupts.S
index e8ed7d6..2fd62bf 100644
--- a/arch/powerpc/kvm/bookehv_interrupts.S
+++ b/arch/powerpc/kvm/bookehv_interrupts.S
@@ -33,6 +33,8 @@

  #ifdef CONFIG_64BIT
  #include <asm/exception-64e.h>
+#include <asm/hw_irq.h>
+#include <asm/irqflags.h>
  #else
  #include "../kernel/head_booke.h" /* for THREAD_NORMSAVE() */
  #endif
@@ -469,6 +471,13 @@ _GLOBAL(kvmppc_resume_host)
      PPC_LL    r3, HOST_RUN(r1)
      mr    r5, r14 /* intno */
      mr    r14, r4 /* Save vcpu pointer. */
+#ifdef CONFIG_64BIT
+    /* Should we soft-disable interrupts? */
+    andi.    r6, r5, BOOKE_INTERRUPT_EXTERNAL | BOOKE_INTERRUPT_DECREMENTER
| BOOKE_INTERRUPT_DOORBELL
+    beq    skip_soft_dis
+    SOFT_DISABLE_INTS(r7,r8)
+skip_soft_dis:
+#endif

Why wouldn't we always disable them?  kvmppc_handle_exit() will enable
interrupts when it's ready.

This only disable soft interrupt for kvmppc_restart_interrupt() that restarts interrupts if they were meant for the host:

a. SOFT_DISABLE_INTS() only for BOOKE_INTERRUPT_EXTERNAL | BOOKE_INTERRUPT_DECREMENTER | BOOKE_INTERRUPT_DOORBELL

b. bl      kvmppc_handle_exit

c. kvmppc_handle_exit()
{
        int r = RESUME_HOST;
        int s;

        /* update before a new last_exit_type is rewritten */
        kvmppc_update_timing_stats(vcpu);

        /* restart interrupts if they were meant for the host */
        kvmppc_restart_interrupt(vcpu, exit_nr);

        local_irq_enable();     ==> Enable again.
....

And shouldn't we handle kvmppc_restart_interrupt() like the original HOST flow?

#define MASKABLE_EXCEPTION(trapnum, intnum, label, hdlr, ack)           \
        START_EXCEPTION(label);                                         \
        NORMAL_EXCEPTION_PROLOG(trapnum, intnum, PROLOG_ADDITION_MASKABLE)\
        EXCEPTION_COMMON(trapnum, PACA_EXGEN, *INTS_DISABLE*)             \
        ...

So I think this should be reasonable :)

Tiejun

--
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

Reply via email to