Re: [v5][PATCH] KVM: PPC: Book3E HV: call RECONCILE_IRQ_STATE to sync the software state

["\"“tiejun.chen”\"" <tiejun.chen@windriver.com>]

Alex,

I suppose Scott already elaborate anything you want to know.

Tiejun

On 11/19/2013 05:09 AM, Alexander Graf wrote:
>
> On 18.11.2013, at 03:34, �tiejun.chen� <tiejun.chen@windriver.com> wrote:
>
>> On 10/23/2013 09:26 AM, Tiejun Chen wrote:
>>> We enter with interrupts disabled in hardware, but we need to
>>> call RECONCILE_IRQ_STATE anyway to ensure that the software state
>>> is kept in sync instead of calling hard_irq_disable() directly.
>
> Why didn't this happen before? What is this patch fixing?
>
>>>
>>> Signed-off-by: Tiejun Chen <tiejun.chen@windriver.com>
>>> ---
>>> v5:
>>>
>>> Fix one typo in the comment.
>>
>> Alex,
>>
>> I already addressed Scott's comment, any further feedback?
>
> Apart from the fact that we don't build PR for 64bit and only 64bit does do lazy EE, is there any reason this code moves from booke.c into HV only paths?
>
>
> Oh, and sorry for the late reply :).
>
> Alex
>
>>
>> Tiejun
>>
>>>
>>> v4:
>>>
>>> Fix one typo in the patch description.
>>>
>>> v3:
>>>
>>> Base on the latest tree, now we can use RECONCILE_IRQ_STATE instead of SOFT_DISABLE_INTS.
>>>
>>> v2:
>>>
>>> Move SOFT_DISABLE_INTS[1] earlier to avoid clobbering the arguments we want to pass to kvmppc_handle_exit.
>>>
>>>   arch/powerpc/kvm/booke.c              |   11 -----------
>>>   arch/powerpc/kvm/bookehv_interrupts.S |   11 +++++++++++
>>>   2 files changed, 11 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>> index 15d0149..0d211ff 100644
>>> --- a/arch/powerpc/kvm/booke.c
>>> +++ b/arch/powerpc/kvm/booke.c
>>> @@ -899,17 +899,6 @@ int kvmppc_handle_exit(struct kvm_run *run, struct kvm_vcpu *vcpu,
>>>   	int s;
>>>   	int idx;
>>>
>>> -#ifdef CONFIG_PPC64
>>> -	WARN_ON(local_paca->irq_happened != 0);
>>> -#endif
>>> -
>>> -	/*
>>> -	 * We enter with interrupts disabled in hardware, but
>>> -	 * we need to call hard_irq_disable anyway to ensure that
>>> -	 * the software state is kept in sync.
>>> -	 */
>>> -	hard_irq_disable();
>>> -
>>>   	/* update before a new last_exit_type is rewritten */
>>>   	kvmppc_update_timing_stats(vcpu);
>>>
>>> diff --git a/arch/powerpc/kvm/bookehv_interrupts.S b/arch/powerpc/kvm/bookehv_interrupts.S
>>> index e8ed7d6..191c32b 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
>>> @@ -465,6 +467,15 @@ _GLOBAL(kvmppc_resume_host)
>>>   	mtspr	SPRN_EPCR, r3
>>>   	isync
>>>
>>> +#ifdef CONFIG_64BIT
>>> +	/*
>>> +	 * We enter with interrupts disabled in hardware, but
>>> +	 * we need to call RECONCILE_IRQ_STATE anyway to ensure
>>> +	 * that the software state is kept in sync.
>>> +	 */
>>> +	RECONCILE_IRQ_STATE(r3,r5)
>>> +#endif
>>> +
>>>   	/* Switch to kernel stack and jump to handler. */
>>>   	PPC_LL	r3, HOST_RUN(r1)
>>>   	mr	r5, r14 /* intno */
>>>
>>
>
>