Re: [v1][PATCH 1/1] KVM: PPC: disable preemption when using hard_irq_disable()

["tiejun.chen" <tiejun.chen@windriver.com>]

On 07/12/2013 12:36 AM, Scott Wood wrote:
> On 07/11/2013 11:30:41 AM, Alexander Graf wrote:
>>
>> On 11.07.2013, at 18:18, Scott Wood wrote:
>>
>> > On 07/11/2013 08:07:30 AM, Alexander Graf wrote:
>> >> get_paca() warns when we're preemptible. We're only not preemptible when
>> 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.
>> >> 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.
>
>> >> 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 <tiejun.chen@windriver.com>

diff --git a/arch/powerpc/kvm/bookehv_interrupts.S 
b/arch/powerpc/kvm/bookehv_interrupts.S
index e8ed7d6..b521d21 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,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