From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?UTF-8?B?IuKAnHRpZWp1bi5jaGVu4oCdIg==?= Subject: Re: [v5][PATCH] KVM: PPC: Book3E HV: call RECONCILE_IRQ_STATE to sync the software state Date: Fri, 29 Nov 2013 10:01:59 +0800 Message-ID: <5297F597.6080301@windriver.com> References: <1382491608-4535-1-git-send-email-tiejun.chen@windriver.com> <5289D10D.5020506@windriver.com> <925AA130-F432-4367-BB55-AF833861AB60@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Scott Wood , "kvm@vger.kernel.org mailing list" , To: Alexander Graf Return-path: In-Reply-To: <925AA130-F432-4367-BB55-AF833861AB60@suse.de> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org 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, =EF=BF=BDtiejun.chen=EF=BF=BD 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 >>> --- >>> 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 d= oes do lazy EE, is there any reason this code moves from booke.c into H= V 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, st= ruct kvm_vcpu *vcpu, >>> int s; >>> int idx; >>> >>> -#ifdef CONFIG_PPC64 >>> - WARN_ON(local_paca->irq_happened !=3D 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/k= vm/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 >>> +#include >>> +#include >>> #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 */ >>> >> > >