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

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

On 07/13/2013 01:50 AM, Scott Wood wrote:
> On 07/11/2013 10:22:28 PM, tiejun.chen wrote:
>> 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
>
> This will clobber the arguments we want to pass to kvmppc_handle_exit.  That can
> be fixed by moving SOFT_DISABLE_INTS[1] earlier, and maybe it's more idiomatic

Okay. Once we have a final name to replace SOFT_DISABLE_INTS, I can regenerate 
this as you comment.

> to use SOFT_DISABLE_INTS rather than what we currently do, but we still want to
> fix hard_irq_disable().  There are other places where we call hard_irq_disable()
> where interrupts (and I believe preemption) were previously enabled.

Yes, I had a preliminary change ACKed by Ben, and I guess you also saw :) so 
I'll send that firstly.

Thanks,

Tiejun