Skip to contentSkip to conversation

KVM: PPC: disable preemption when using hard_irq_disable()

PATCH Benjamin Herrenschmidt 2 participants 2 messages Jul 12, 2013 3 messages not in the archive

Subsystems (3)
KVM PowerPC KVM PowerPC
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 1373559480.8183.258@snotra

Benjamin Herrenschmidt

author
#1 lore raw
Quoted 3 lines · Scott Wood

If we set IRQs as soft-disabled prior to calling hard_irq_disable(), then hard_irq_disable() will fail to call trace_hardirqs_off().

Sure because setting them as soft-disabled will have done it.

However by doing so, you also create the possibility of latching a new event in irq_happened.

Quoted 8 lines
Quoted 1 line

or by disabling preemption until we enter the guest for real.

I don't follow this one. We're exiting, not entering.

Quoted 1 line

Any preferences?

Use arch_local_save_flags() in hard_irq_disable() instead of reading soft_enabled with C code.

That or just use local_paca... Though we'd had problems in the past where gcc would defeat us there and essentially copy r13 to another register and start indexing from there. That kills you if you preempt.

Cheers,
Ben.

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 1373560585.8183.261@snotra

#4 lore raw
Quoted 17 lines · Scott Wood
Quoted 5 lines · Alexander Graf
Quoted 2 lines · Scott Wood
Quoted 1 line · Alexander Graf

get_paca() warns when we're preemptible. We're only not preemptible when

Quoted 8 lines

either preempt is disabled or irqs are disabled. Irqs are disabled, but arch_irqs_disabled() doesn't know, because it only checks for soft disabled IRQs.

Quoted 3 lines
Quoted 1 line

So we can fix this either by setting IRQs as soft disabled as well

If we set IRQs as soft-disabled prior to calling hard_irq_disable(), then

hard_irq_disable() will fail to call trace_hardirqs_off().

Right...

Plus we'd have the same problem trying to set soft_enabled to 0.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Any preferences?

Use arch_local_save_flags() in hard_irq_disable() instead of reading

soft_enabled with C code.

That only operates on the soft_enabled bit. We also need to access irq_happened.

OK, so we'll need more inline asm.

If so, why not to remove directly hard_irq_disable() inside kvmppc_handle_exit() by reverting that commit, "kvm/ppc/booke64: Fix lazy ee handling in kvmppc_handle_exit()"?

Then we can use SOFT_DISABLE_INTS() explicitly before call kvmppc_handle_exit() like this:

KVM: PPC: Book3E HV: call SOFT_DISABLE_INTS to sync the software state

We enter with interrupts disabled in hardware, but we need to call SOFT_DISABLE_INTS anyway to ensure that the software state is kept in sync.

Signed-off-by Tiejun Chen <[email protected]>
arch/powerpc/kvm/bookehv_interrupts.S +10 -0
index e8ed7d6..b521d21 100644
@@ -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,14 @@ _GLOBAL(kvmppc_resume_host)
        PPC_LL  r3, HOST_RUN(r1)
        mr      r5, r14 /* intno */
        mr      r14, r4 /* Save vcpu pointer. */
+#ifdef CONFIG_64BIT
+       /*
+        * We enter with interrupts disabled in hardware, but
+        * we need to call SOFT_DISABLE_INTS anyway to ensure
+        * that the software state is kept in sync.
+        */
+       SOFT_DISABLE_INTS(r7,r8)
+#endif
        bl      kvmppc_handle_exit
 
        /* Restore vcpu pointer and the nonvolatiles we used. */

Tiejun