Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Alexander Graf 

On 03.05.2013, at 18:11, Mihai Caraman wrote:

 A change in the generic code highlighted that we were running with IRQs
 (soft) enabled on Book3E 64-bit when trying to restart interrupts from
 handle_exit(). This is a lesson to configure lockdep often :)
 
 There is no reason to exit guest with soft_enabled == 1, a local_irq_enable()
 call will do this for us so get rid of kvmppc_layz_ee() calls. With this fix
 we eliminate irqs_disabled() warnings and some guest and host hangs revealed
 under stress tests, but guests still exhibit some unresponsiveness.
 
 The unresponsiveness has to do with the fact that arch_local_irq_restore()
 does not guarantees to hard enable interrupts. To do so replace exception
 function calls like timer_interrupt() with irq_happened flags. The
 local_irq_enable() call takes care of replaying them and lets the interrupts
 hard enabled.
 
 Signed-off-by: Mihai Caraman mihai.cara...@freescale.com

Ben, could you please review?


Alex

 ---
 arch/powerpc/kvm/booke.c |9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)
 
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 1020119..82f155e 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -673,7 +673,6 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct 
 kvm_vcpu *vcpu)
   ret = s;
   goto out;
   }
 - kvmppc_lazy_ee_enable();
 
   kvm_guest_enter();
 
 @@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu 
 *vcpu,
   switch (exit_nr) {
   case BOOKE_INTERRUPT_EXTERNAL:
   kvmppc_fill_pt_regs(regs);
 - do_IRQ(regs);
 + local_paca-irq_happened |= PACA_IRQ_EE;
   break;
   case BOOKE_INTERRUPT_DECREMENTER:
   kvmppc_fill_pt_regs(regs);
 - timer_interrupt(regs);
 + local_paca-irq_happened |= PACA_IRQ_DEC;
   break;
 #if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64)
   case BOOKE_INTERRUPT_DOORBELL:
   kvmppc_fill_pt_regs(regs);
 - doorbell_exception(regs);
 + local_paca-irq_happened |= PACA_IRQ_DBELL;
   break;
 #endif
   case BOOKE_INTERRUPT_MACHINE_CHECK:
 @@ -1148,8 +1147,6 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
 kvm_vcpu *vcpu,
   if (s = 0) {
   local_irq_enable();
   r = (s  2) | RESUME_HOST | (r  RESUME_FLAG_NV);
 - } else {
 - kvmppc_lazy_ee_enable();
   }
   }
 
 -- 
 1.7.4.1
 
 
 --
 To unsubscribe from this list: send the line unsubscribe kvm-ppc in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 11:11:10 AM, Mihai Caraman wrote:
A change in the generic code highlighted that we were running with  
IRQs

(soft) enabled on Book3E 64-bit when trying to restart interrupts from
handle_exit(). This is a lesson to configure lockdep often :)

There is no reason to exit guest with soft_enabled == 1, a  
local_irq_enable()
call will do this for us so get rid of kvmppc_layz_ee() calls. With  
this fix
we eliminate irqs_disabled() warnings and some guest and host hangs  
revealed

under stress tests, but guests still exhibit some unresponsiveness.

The unresponsiveness has to do with the fact that  
arch_local_irq_restore()

does not guarantees to hard enable interrupts.


Could you elaborate?  If the saved IRQ state was enabled, why  
wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing it  
does is __hard_irq_enable().


Where is the arch_local_irq_restore() instance you're talking about?


To do so replace exception
function calls like timer_interrupt() with irq_happened flags. The
local_irq_enable() call takes care of replaying them and lets the  
interrupts

hard enabled.


Not sure what you mean by lets the interrupts hard enabled... Do you  
mean the EE bit in regs-msr, as opposed to the EE bit in the current  
MSR?



Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
 arch/powerpc/kvm/booke.c |9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 1020119..82f155e 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -673,7 +673,6 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,  
struct kvm_vcpu *vcpu)

ret = s;
goto out;
}
-   kvmppc_lazy_ee_enable();

kvm_guest_enter();

@@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct  
kvm_vcpu *vcpu,

switch (exit_nr) {
case BOOKE_INTERRUPT_EXTERNAL:
kvmppc_fill_pt_regs(regs);
-   do_IRQ(regs);
+   local_paca-irq_happened |= PACA_IRQ_EE;
break;
case BOOKE_INTERRUPT_DECREMENTER:
kvmppc_fill_pt_regs(regs);
-   timer_interrupt(regs);
+   local_paca-irq_happened |= PACA_IRQ_DEC;
break;
 #if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64)
case BOOKE_INTERRUPT_DOORBELL:
kvmppc_fill_pt_regs(regs);
-   doorbell_exception(regs);
+   local_paca-irq_happened |= PACA_IRQ_DBELL;
break;
 #endif


Aren't you breaking 32-bit here?

-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

RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Caraman Mihai Claudiu-B02008 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 9:05 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
 
  The unresponsiveness has to do with the fact that
  arch_local_irq_restore()
  does not guarantees to hard enable interrupts.
 
 Could you elaborate?  If the saved IRQ state was enabled, why
 wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing it
 does is __hard_irq_enable().

if (!irq_happened)
return;

 
 Where is the arch_local_irq_restore() instance you're talking about?

./arch/power/kernel/irq.c

 
  To do so replace exception
  function calls like timer_interrupt() with irq_happened flags. The
  local_irq_enable() call takes care of replaying them and lets the
  interrupts
  hard enabled.
 
 Not sure what you mean by lets the interrupts hard enabled... Do you
 mean the EE bit in regs-msr, as opposed to the EE bit in the current
 MSR?

If irq_happened the last thing it does is __hard_irq_enable().

  @@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct
  kvm_vcpu *vcpu,
  switch (exit_nr) {
  case BOOKE_INTERRUPT_EXTERNAL:
  kvmppc_fill_pt_regs(regs);
  -   do_IRQ(regs);
  +   local_paca-irq_happened |= PACA_IRQ_EE;
  break;
 
 Aren't you breaking 32-bit here?

I had eyes only for 64-bit hangs :)

-Mike

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

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 03:01:26 PM, Caraman Mihai Claudiu-B02008 wrote:

 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 9:05 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: kvm-...@vger.kernel.org; kvm@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and  
hangs


  The unresponsiveness has to do with the fact that
  arch_local_irq_restore()
  does not guarantees to hard enable interrupts.

 Could you elaborate?  If the saved IRQ state was enabled, why
 wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing  
it

 does is __hard_irq_enable().

if (!irq_happened)
return;


OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when we  
hard-disable interrupts?



 Where is the arch_local_irq_restore() instance you're talking about?

./arch/power/kernel/irq.c


I meant the caller. :-P

-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

RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Caraman Mihai Claudiu-B02008 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 11:15 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
 
The unresponsiveness has to do with the fact that
arch_local_irq_restore()
does not guarantees to hard enable interrupts.
  
   Could you elaborate?  If the saved IRQ state was enabled, why
   wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing
  it
   does is __hard_irq_enable().
 
  if (!irq_happened)
  return;
 
 OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when we
 hard-disable interrupts?

We enter guest with local_irq_disable() which means soft disabled, when
do we hard-disable interrupts? If we follow host exception handlers model
they set PACA_IRQ_EE/DEC/DBELL but not PACA_IRQ_HARD_DIS. Can you give it
a try to see how KVM behaves with PACA_IRQ_HARD_DIS? I can't do it right now.

 
   Where is the arch_local_irq_restore() instance you're talking about?
 
  ./arch/power/kernel/irq.c
 
 I meant the caller. :-P

./arch/powerpc/include/asm/hw_irq.h

  55static inline unsigned long arch_local_irq_disable(void)
  56{
  57unsigned long flags, zero;
  58
  59asm volatile(
  60li %1,0; lbz %0,%2(13); stb %1,%2(13)
  61: =r (flags), =r (zero)
  62: i (offsetof(struct paca_struct, soft_enabled))
  63: memory);
  64
  65return flags;
  66}
  67
  68extern void arch_local_irq_restore(unsigned long);
  69
  70static inline void arch_local_irq_enable(void)
  71{
  72arch_local_irq_restore(1);
  73}

-Mike

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

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Benjamin Herrenschmidt 
On Fri, 2013-05-03 at 18:24 +0200, Alexander Graf wrote:
  There is no reason to exit guest with soft_enabled == 1, a 
  local_irq_enable()
  call will do this for us so get rid of kvmppc_layz_ee() calls. With this fix
  we eliminate irqs_disabled() warnings and some guest and host hangs revealed
  under stress tests, but guests still exhibit some unresponsiveness.
  
  The unresponsiveness has to do with the fact that arch_local_irq_restore()
  does not guarantees to hard enable interrupts. To do so replace exception
  function calls like timer_interrupt() with irq_happened flags. The
  local_irq_enable() call takes care of replaying them and lets the interrupts
  hard enabled.
  
  Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
 
 Ben, could you please review?

That does look like the right thing to do indeed.

Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org

Cheers,
Ben.


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

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 03:56:47 PM, Caraman Mihai Claudiu-B02008 wrote:

 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 11:15 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and  
hangs


The unresponsiveness has to do with the fact that
arch_local_irq_restore()
does not guarantees to hard enable interrupts.
  
   Could you elaborate?  If the saved IRQ state was enabled, why
   wouldn't arch_local_irq_restore() hard-enable IRQs?  The last  
thing

  it
   does is __hard_irq_enable().
 
if (!irq_happened)
return;

 OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when  
we

 hard-disable interrupts?

We enter guest with local_irq_disable() which means soft disabled,


Hmm... I don't see any obvious breakage from that, but it makes me  
nervous.  I'd be more comfortable if we just hard-disabled interrupts  
there.



when do we hard-disable interrupts?


Interrupts will be hard-disabled when we take an exception to exit  
guest state.



If we follow host exception handlers model
they set PACA_IRQ_EE/DEC/DBELL but not PACA_IRQ_HARD_DIS. Can you  
give it
a try to see how KVM behaves with PACA_IRQ_HARD_DIS? I can't do it  
right now.


I replaced the two calls to kvmppc_lazy_ee_enable() with calls to  
hard_irq_disable(), and it seems to be working fine.


   Where is the arch_local_irq_restore() instance you're talking  
about?

 
  ./arch/power/kernel/irq.c

 I meant the caller. :-P

./arch/powerpc/include/asm/hw_irq.h

  55static inline unsigned long arch_local_irq_disable(void)
  56{
  57unsigned long flags, zero;
  58
  59asm volatile(
  60li %1,0; lbz %0,%2(13); stb %1,%2(13)
  61: =r (flags), =r (zero)
  62: i (offsetof(struct paca_struct, soft_enabled))
  63: memory);
  64
  65return flags;
  66}
  67
  68extern void arch_local_irq_restore(unsigned long);
  69
  70static inline void arch_local_irq_enable(void)
  71{
  72arch_local_irq_restore(1);
  73}


Sigh.  I meant the real caller, who's calling local_irq_restore().

-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

RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Caraman Mihai Claudiu-B02008 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Saturday, May 04, 2013 1:07 AM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
 
 I replaced the two calls to kvmppc_lazy_ee_enable() with calls to
 hard_irq_disable(), and it seems to be working fine.

Please take a look on 'KVM: PPC64: booke: Hard disable interrupts when
entering guest' RFC thread and see if your solution addresses Ben's
comments.

 
 Where is the arch_local_irq_restore() instance you're talking
  about?
   
./arch/power/kernel/irq.c
  
   I meant the caller. :-P
 
  ./arch/powerpc/include/asm/hw_irq.h
 
55static inline unsigned long arch_local_irq_disable(void)
56{
57unsigned long flags, zero;
58
59asm volatile(
60li %1,0; lbz %0,%2(13); stb %1,%2(13)
61: =r (flags), =r (zero)
62: i (offsetof(struct paca_struct, soft_enabled))
63: memory);
64
65return flags;
66}
67
68extern void arch_local_irq_restore(unsigned long);
69
70static inline void arch_local_irq_enable(void)
71{
72arch_local_irq_restore(1);
73}
 
 Sigh.  I meant the real caller, who's calling local_irq_restore().

I'm not sure what you mean, arch_local_irq_restore() is called indirectly
by local_irq_enable() in our case from handle_exit().

-Mike

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

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 05:59:32 PM, Caraman Mihai Claudiu-B02008 wrote:

 -Original Message-
 From: Wood Scott-B07421
 Sent: Saturday, May 04, 2013 1:07 AM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-...@vger.kernel.org; kvm@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and  
hangs


 I replaced the two calls to kvmppc_lazy_ee_enable() with calls to
 hard_irq_disable(), and it seems to be working fine.

Please take a look on 'KVM: PPC64: booke: Hard disable interrupts when
entering guest' RFC thread and see if your solution addresses Ben's
comments.


My original one didn't (there was a race if an interrupt comes in  
between soft-disabling and hard-disabling, it wouldn't be received  
until the guest exits for some other reason).


Instead, I turned the local_irq_disable() into hard_irq_disable() plus  
trace_hardirqs_off().  This worked without warnings.


-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

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Alexander Graf 

On 03.05.2013, at 18:11, Mihai Caraman wrote:

 A change in the generic code highlighted that we were running with IRQs
 (soft) enabled on Book3E 64-bit when trying to restart interrupts from
 handle_exit(). This is a lesson to configure lockdep often :)
 
 There is no reason to exit guest with soft_enabled == 1, a local_irq_enable()
 call will do this for us so get rid of kvmppc_layz_ee() calls. With this fix
 we eliminate irqs_disabled() warnings and some guest and host hangs revealed
 under stress tests, but guests still exhibit some unresponsiveness.
 
 The unresponsiveness has to do with the fact that arch_local_irq_restore()
 does not guarantees to hard enable interrupts. To do so replace exception
 function calls like timer_interrupt() with irq_happened flags. The
 local_irq_enable() call takes care of replaying them and lets the interrupts
 hard enabled.
 
 Signed-off-by: Mihai Caraman mihai.cara...@freescale.com

Ben, could you please review?


Alex

 ---
 arch/powerpc/kvm/booke.c |9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)
 
 diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
 index 1020119..82f155e 100644
 --- a/arch/powerpc/kvm/booke.c
 +++ b/arch/powerpc/kvm/booke.c
 @@ -673,7 +673,6 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct 
 kvm_vcpu *vcpu)
   ret = s;
   goto out;
   }
 - kvmppc_lazy_ee_enable();
 
   kvm_guest_enter();
 
 @@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct kvm_vcpu 
 *vcpu,
   switch (exit_nr) {
   case BOOKE_INTERRUPT_EXTERNAL:
   kvmppc_fill_pt_regs(regs);
 - do_IRQ(regs);
 + local_paca-irq_happened |= PACA_IRQ_EE;
   break;
   case BOOKE_INTERRUPT_DECREMENTER:
   kvmppc_fill_pt_regs(regs);
 - timer_interrupt(regs);
 + local_paca-irq_happened |= PACA_IRQ_DEC;
   break;
 #if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64)
   case BOOKE_INTERRUPT_DOORBELL:
   kvmppc_fill_pt_regs(regs);
 - doorbell_exception(regs);
 + local_paca-irq_happened |= PACA_IRQ_DBELL;
   break;
 #endif
   case BOOKE_INTERRUPT_MACHINE_CHECK:
 @@ -1148,8 +1147,6 @@ int kvmppc_handle_exit(struct kvm_run *run, struct 
 kvm_vcpu *vcpu,
   if (s = 0) {
   local_irq_enable();
   r = (s  2) | RESUME_HOST | (r  RESUME_FLAG_NV);
 - } else {
 - kvmppc_lazy_ee_enable();
   }
   }
 
 -- 
 1.7.4.1
 
 
 --
 To unsubscribe from this list: send the line unsubscribe kvm-ppc in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 11:11:10 AM, Mihai Caraman wrote:
A change in the generic code highlighted that we were running with  
IRQs

(soft) enabled on Book3E 64-bit when trying to restart interrupts from
handle_exit(). This is a lesson to configure lockdep often :)

There is no reason to exit guest with soft_enabled == 1, a  
local_irq_enable()
call will do this for us so get rid of kvmppc_layz_ee() calls. With  
this fix
we eliminate irqs_disabled() warnings and some guest and host hangs  
revealed

under stress tests, but guests still exhibit some unresponsiveness.

The unresponsiveness has to do with the fact that  
arch_local_irq_restore()

does not guarantees to hard enable interrupts.


Could you elaborate?  If the saved IRQ state was enabled, why  
wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing it  
does is __hard_irq_enable().


Where is the arch_local_irq_restore() instance you're talking about?


To do so replace exception
function calls like timer_interrupt() with irq_happened flags. The
local_irq_enable() call takes care of replaying them and lets the  
interrupts

hard enabled.


Not sure what you mean by lets the interrupts hard enabled... Do you  
mean the EE bit in regs-msr, as opposed to the EE bit in the current  
MSR?



Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
---
 arch/powerpc/kvm/booke.c |9 +++--
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 1020119..82f155e 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -673,7 +673,6 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run,  
struct kvm_vcpu *vcpu)

ret = s;
goto out;
}
-   kvmppc_lazy_ee_enable();

kvm_guest_enter();

@@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct  
kvm_vcpu *vcpu,

switch (exit_nr) {
case BOOKE_INTERRUPT_EXTERNAL:
kvmppc_fill_pt_regs(regs);
-   do_IRQ(regs);
+   local_paca-irq_happened |= PACA_IRQ_EE;
break;
case BOOKE_INTERRUPT_DECREMENTER:
kvmppc_fill_pt_regs(regs);
-   timer_interrupt(regs);
+   local_paca-irq_happened |= PACA_IRQ_DEC;
break;
 #if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_BOOK3E_64)
case BOOKE_INTERRUPT_DOORBELL:
kvmppc_fill_pt_regs(regs);
-   doorbell_exception(regs);
+   local_paca-irq_happened |= PACA_IRQ_DBELL;
break;
 #endif


Aren't you breaking 32-bit here?

-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Caraman Mihai Claudiu-B02008 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 9:05 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
 
  The unresponsiveness has to do with the fact that
  arch_local_irq_restore()
  does not guarantees to hard enable interrupts.
 
 Could you elaborate?  If the saved IRQ state was enabled, why
 wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing it
 does is __hard_irq_enable().

if (!irq_happened)
return;

 
 Where is the arch_local_irq_restore() instance you're talking about?

./arch/power/kernel/irq.c

 
  To do so replace exception
  function calls like timer_interrupt() with irq_happened flags. The
  local_irq_enable() call takes care of replaying them and lets the
  interrupts
  hard enabled.
 
 Not sure what you mean by lets the interrupts hard enabled... Do you
 mean the EE bit in regs-msr, as opposed to the EE bit in the current
 MSR?

If irq_happened the last thing it does is __hard_irq_enable().

  @@ -789,16 +788,16 @@ static void kvmppc_restart_interrupt(struct
  kvm_vcpu *vcpu,
  switch (exit_nr) {
  case BOOKE_INTERRUPT_EXTERNAL:
  kvmppc_fill_pt_regs(regs);
  -   do_IRQ(regs);
  +   local_paca-irq_happened |= PACA_IRQ_EE;
  break;
 
 Aren't you breaking 32-bit here?

I had eyes only for 64-bit hangs :)

-Mike

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 03:01:26 PM, Caraman Mihai Claudiu-B02008 wrote:

 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 9:05 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
 d...@lists.ozlabs.org; Caraman Mihai Claudiu-B02008
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and  
hangs


  The unresponsiveness has to do with the fact that
  arch_local_irq_restore()
  does not guarantees to hard enable interrupts.

 Could you elaborate?  If the saved IRQ state was enabled, why
 wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing  
it

 does is __hard_irq_enable().

if (!irq_happened)
return;


OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when we  
hard-disable interrupts?



 Where is the arch_local_irq_restore() instance you're talking about?

./arch/power/kernel/irq.c


I meant the caller. :-P

-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Caraman Mihai Claudiu-B02008 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 11:15 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
 
The unresponsiveness has to do with the fact that
arch_local_irq_restore()
does not guarantees to hard enable interrupts.
  
   Could you elaborate?  If the saved IRQ state was enabled, why
   wouldn't arch_local_irq_restore() hard-enable IRQs?  The last thing
  it
   does is __hard_irq_enable().
 
  if (!irq_happened)
  return;
 
 OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when we
 hard-disable interrupts?

We enter guest with local_irq_disable() which means soft disabled, when
do we hard-disable interrupts? If we follow host exception handlers model
they set PACA_IRQ_EE/DEC/DBELL but not PACA_IRQ_HARD_DIS. Can you give it
a try to see how KVM behaves with PACA_IRQ_HARD_DIS? I can't do it right now.

 
   Where is the arch_local_irq_restore() instance you're talking about?
 
  ./arch/power/kernel/irq.c
 
 I meant the caller. :-P

./arch/powerpc/include/asm/hw_irq.h

  55static inline unsigned long arch_local_irq_disable(void)
  56{
  57unsigned long flags, zero;
  58
  59asm volatile(
  60li %1,0; lbz %0,%2(13); stb %1,%2(13)
  61: =r (flags), =r (zero)
  62: i (offsetof(struct paca_struct, soft_enabled))
  63: memory);
  64
  65return flags;
  66}
  67
  68extern void arch_local_irq_restore(unsigned long);
  69
  70static inline void arch_local_irq_enable(void)
  71{
  72arch_local_irq_restore(1);
  73}

-Mike

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Benjamin Herrenschmidt 
On Fri, 2013-05-03 at 18:24 +0200, Alexander Graf wrote:
  There is no reason to exit guest with soft_enabled == 1, a 
  local_irq_enable()
  call will do this for us so get rid of kvmppc_layz_ee() calls. With this fix
  we eliminate irqs_disabled() warnings and some guest and host hangs revealed
  under stress tests, but guests still exhibit some unresponsiveness.
  
  The unresponsiveness has to do with the fact that arch_local_irq_restore()
  does not guarantees to hard enable interrupts. To do so replace exception
  function calls like timer_interrupt() with irq_happened flags. The
  local_irq_enable() call takes care of replaying them and lets the interrupts
  hard enabled.
  
  Signed-off-by: Mihai Caraman mihai.cara...@freescale.com
 
 Ben, could you please review?

That does look like the right thing to do indeed.

Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org

Cheers,
Ben.


--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 03:56:47 PM, Caraman Mihai Claudiu-B02008 wrote:

 -Original Message-
 From: Wood Scott-B07421
 Sent: Friday, May 03, 2013 11:15 PM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and  
hangs


The unresponsiveness has to do with the fact that
arch_local_irq_restore()
does not guarantees to hard enable interrupts.
  
   Could you elaborate?  If the saved IRQ state was enabled, why
   wouldn't arch_local_irq_restore() hard-enable IRQs?  The last  
thing

  it
   does is __hard_irq_enable().
 
if (!irq_happened)
return;

 OK, so the problem is that we're not setting PACA_IRQ_HARD_DIS when  
we

 hard-disable interrupts?

We enter guest with local_irq_disable() which means soft disabled,


Hmm... I don't see any obvious breakage from that, but it makes me  
nervous.  I'd be more comfortable if we just hard-disabled interrupts  
there.



when do we hard-disable interrupts?


Interrupts will be hard-disabled when we take an exception to exit  
guest state.



If we follow host exception handlers model
they set PACA_IRQ_EE/DEC/DBELL but not PACA_IRQ_HARD_DIS. Can you  
give it
a try to see how KVM behaves with PACA_IRQ_HARD_DIS? I can't do it  
right now.


I replaced the two calls to kvmppc_lazy_ee_enable() with calls to  
hard_irq_disable(), and it seems to be working fine.


   Where is the arch_local_irq_restore() instance you're talking  
about?

 
  ./arch/power/kernel/irq.c

 I meant the caller. :-P

./arch/powerpc/include/asm/hw_irq.h

  55static inline unsigned long arch_local_irq_disable(void)
  56{
  57unsigned long flags, zero;
  58
  59asm volatile(
  60li %1,0; lbz %0,%2(13); stb %1,%2(13)
  61: =r (flags), =r (zero)
  62: i (offsetof(struct paca_struct, soft_enabled))
  63: memory);
  64
  65return flags;
  66}
  67
  68extern void arch_local_irq_restore(unsigned long);
  69
  70static inline void arch_local_irq_enable(void)
  71{
  72arch_local_irq_restore(1);
  73}


Sigh.  I meant the real caller, who's calling local_irq_restore().

-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Caraman Mihai Claudiu-B02008 
 -Original Message-
 From: Wood Scott-B07421
 Sent: Saturday, May 04, 2013 1:07 AM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs
 
 I replaced the two calls to kvmppc_lazy_ee_enable() with calls to
 hard_irq_disable(), and it seems to be working fine.

Please take a look on 'KVM: PPC64: booke: Hard disable interrupts when
entering guest' RFC thread and see if your solution addresses Ben's
comments.

 
 Where is the arch_local_irq_restore() instance you're talking
  about?
   
./arch/power/kernel/irq.c
  
   I meant the caller. :-P
 
  ./arch/powerpc/include/asm/hw_irq.h
 
55static inline unsigned long arch_local_irq_disable(void)
56{
57unsigned long flags, zero;
58
59asm volatile(
60li %1,0; lbz %0,%2(13); stb %1,%2(13)
61: =r (flags), =r (zero)
62: i (offsetof(struct paca_struct, soft_enabled))
63: memory);
64
65return flags;
66}
67
68extern void arch_local_irq_restore(unsigned long);
69
70static inline void arch_local_irq_enable(void)
71{
72arch_local_irq_restore(1);
73}
 
 Sigh.  I meant the real caller, who's calling local_irq_restore().

I'm not sure what you mean, arch_local_irq_restore() is called indirectly
by local_irq_enable() in our case from handle_exit().

-Mike

--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and hangs

2013-05-03 Thread Scott Wood 

On 05/03/2013 05:59:32 PM, Caraman Mihai Claudiu-B02008 wrote:

 -Original Message-
 From: Wood Scott-B07421
 Sent: Saturday, May 04, 2013 1:07 AM
 To: Caraman Mihai Claudiu-B02008
 Cc: Wood Scott-B07421; kvm-ppc@vger.kernel.org; k...@vger.kernel.org;
 linuxppc-...@lists.ozlabs.org
 Subject: Re: [PATCH] KVM: PPC: Book3E 64: Fix IRQs warnings and  
hangs


 I replaced the two calls to kvmppc_lazy_ee_enable() with calls to
 hard_irq_disable(), and it seems to be working fine.

Please take a look on 'KVM: PPC64: booke: Hard disable interrupts when
entering guest' RFC thread and see if your solution addresses Ben's
comments.


My original one didn't (there was a race if an interrupt comes in  
between soft-disabling and hard-disabling, it wouldn't be received  
until the guest exits for some other reason).


Instead, I turned the local_irq_disable() into hard_irq_disable() plus  
trace_hardirqs_off().  This worked without warnings.


-Scott
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html