[RFC,09/17] KVM: PPC64: booke: Hard disable interrupts when entering guest

64-bit host runs with lazy interrupt disabling, so local_irq_disable() does
not disable interrupts right away and does not protect against preemption
required by __kvmppc_vcpu_run(). Define a macro for 64-bit to use
hard_irq_disable().

Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
---
 arch/powerpc/kvm/booke.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

On 25.06.2012, at 14:26, Mihai Caraman wrote:

> 64-bit host runs with lazy interrupt disabling, so local_irq_disable() does
> not disable interrupts right away and does not protect against preemption
> required by __kvmppc_vcpu_run(). Define a macro for 64-bit to use
> hard_irq_disable().
> 
> Signed-off-by: Mihai Caraman <mihai.caraman@freescale.com>
> ---
> arch/powerpc/kvm/booke.c |   14 ++++++++++----
> 1 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
> index 93b48e0..db05692 100644
> --- a/arch/powerpc/kvm/booke.c
> +++ b/arch/powerpc/kvm/booke.c
> @@ -45,6 +45,12 @@ unsigned long kvmppc_booke_handlers;
> #define VM_STAT(x) offsetof(struct kvm, stat.x), KVM_STAT_VM
> #define VCPU_STAT(x) offsetof(struct kvm_vcpu, stat.x), KVM_STAT_VCPU
> 
> +#ifdef CONFIG_64BIT
> +#define _hard_irq_disable() hard_irq_disable()
> +#else
> +#define _hard_irq_disable() local_irq_disable()
> +#endif

So you only swap out the disable bit, but not the enable one? Ben, would this work out?


Alex

> +
> struct kvm_stats_debugfs_item debugfs_entries[] = {
> 	{ "mmio",       VCPU_STAT(mmio_exits) },
> 	{ "dcr",        VCPU_STAT(dcr_exits) },
> @@ -456,7 +462,7 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
> 		local_irq_enable();
> 		kvm_vcpu_block(vcpu);
> 		clear_bit(KVM_REQ_UNHALT, &vcpu->requests);
> -		local_irq_disable();
> +		_hard_irq_disable();
> 
> 		kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
> 		r = 1;
> @@ -480,7 +486,7 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
> 		if (need_resched()) {
> 			local_irq_enable();
> 			cond_resched();
> -			local_irq_disable();
> +			_hard_irq_disable();
> 			continue;
> 		}
> 
> @@ -515,7 +521,7 @@ int kvmppc_vcpu_run(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
> 		return -EINVAL;
> 	}
> 
> -	local_irq_disable();
> +	_hard_irq_disable();
> 	if (kvmppc_prepare_to_enter(vcpu)) {
> 		kvm_run->exit_reason = KVM_EXIT_INTR;
> 		ret = -EINTR;
> @@ -955,7 +961,7 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
> 	 * aren't already exiting to userspace for some other reason.
> 	 */
> 	if (!(r & RESUME_HOST)) {
> -		local_irq_disable();
> +		_hard_irq_disable();
> 		if (kvmppc_prepare_to_enter(vcpu)) {
> 			run->exit_reason = KVM_EXIT_INTR;
> 			r = (-EINTR << 2) | RESUME_HOST | (r & RESUME_FLAG_NV);
> -- 
> 1.7.4.1
> 
> 
> 

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

On Wed, 2012-07-04 at 16:14 +0200, Alexander Graf wrote:
> > +#ifdef CONFIG_64BIT
> > +#define _hard_irq_disable() hard_irq_disable()
> > +#else
> > +#define _hard_irq_disable() local_irq_disable()
> > +#endif
> 
> So you only swap out the disable bit, but not the enable one? Ben,
> would this work out?

hard_irq_disable() both soft and hard disable. local_irq_enable() will
see that irqs are hard disabled and will hard enable.

However, there's a nastier discrepancy above: local_irq_disable will
properly inform lockdep that we are disabling, while hard_irq_disable
won't.

Arguably we might want to fix that inside hard_irq_disable() itself...

Also you need to be careful. If you are coming with interrupts already
enabled, it's fine, but if you have interrupts soft disabled, then
you hard disable, before you enter the guest you probably want to
check if anything was left "pending" and cancel the entering of the
guest if that is the case.

Cheers,
Ben.


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

> -----Original Message-----
> From: Linuxppc-dev [mailto:linuxppc-dev-
> bounces+mihai.caraman=freescale.com@lists.ozlabs.org] On Behalf Of
> Benjamin Herrenschmidt
> Sent: Thursday, July 05, 2012 1:21 AM
> To: Alexander Graf
> Cc: qemu-ppc@nongnu.org List; Caraman Mihai Claudiu-B02008; linuxppc-dev;
> KVM list; <kvm-ppc@vger.kernel.org>
> Subject: Re: [Qemu-ppc] [RFC PATCH 09/17] KVM: PPC64: booke: Hard disable
> interrupts when entering guest
> 
> On Wed, 2012-07-04 at 16:14 +0200, Alexander Graf wrote:
> > > +#ifdef CONFIG_64BIT
> > > +#define _hard_irq_disable() hard_irq_disable()
> > > +#else
> > > +#define _hard_irq_disable() local_irq_disable()
> > > +#endif
> >
> > So you only swap out the disable bit, but not the enable one? Ben,
> > would this work out?
> 
> hard_irq_disable() both soft and hard disable. local_irq_enable() will
> see that irqs are hard disabled and will hard enable.
> 
> However, there's a nastier discrepancy above: local_irq_disable will
> properly inform lockdep that we are disabling, while hard_irq_disable
> won't.
> 
> Arguably we might want to fix that inside hard_irq_disable() itself...
> 
> Also you need to be careful. If you are coming with interrupts already
> enabled, it's fine, but if you have interrupts soft disabled, then
> you hard disable, before you enter the guest you probably want to
> check if anything was left "pending" and cancel the entering of the
> guest if that is the case.

On which cases I can find interrupts soft disabled if I call local_irq_enable()
ahead? Can this happen when my kernel task is scheduled? 

I presume that if I call hard_irq_disable() before entering the guest, a guest exit
will find interrupts soft disabled.

-Mike

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