From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Subject: Re: [PATCH 3/6] KVM: PPC: booke: Check for MSR[WE] in prepare_to_enter
Date: Mon, 14 Nov 2011 11:22:35 -0600
Message-ID: <4EC14E5B.5080805@freescale.com>
References: <20111109002325.GC6132@schlenkerla.am.freescale.net> <4EC1140F.3000307@suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 7bit
Cc: <kvm@vger.kernel.org>, <kvm-ppc@vger.kernel.org>
To: Alexander Graf <agraf@suse.de>
Return-path: <kvm-ppc-owner@vger.kernel.org>
In-Reply-To: <4EC1140F.3000307@suse.de>
Sender: kvm-ppc-owner@vger.kernel.org
List-Id: kvm.vger.kernel.org

On 11/14/2011 07:13 AM, Alexander Graf wrote:
> On 11/09/2011 01:23 AM, Scott Wood wrote:
>> +/* Check pending exceptions and deliver one, if possible. */
>> +void kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
>> +{
>> +    WARN_ON_ONCE(!irqs_disabled());
>> +
>> +    kvmppc_core_check_exceptions(vcpu);
>> +
>> +    if (vcpu->arch.shared->msr&  MSR_WE) {
>> +        local_irq_enable();
>> +        kvm_vcpu_block(vcpu);
>> +        local_irq_disable();
> 
> Hrm. This specific irq enable/disable part isn't pretty but I can't
> think of a cleaner way either. Unless you move it out of the prepare
> function, since I don't see a way it could race with an interrupt.

kvmppc_core_check_exceptions can clear MSR_WE, so we need to check after
that.  We can't enable interrupts after kvmppc_core_check_exceptions (or
rather, if we do, we need to check again once interrupts are
re-disabled, as in the MSR_WE case) because otherwise we could have an
exception delivered afterward and receive the resched IPI at just the
wrong time to take any action on it (just like the signal check).

>> +
>> +        kvmppc_set_exit_type(vcpu, EMULATED_MTMSRWE_EXITS);
>> +        kvmppc_core_check_exceptions(vcpu);
> 
> Shouldn't
> 
> if (msr & MSR_WE) {
>   ...
> }
> 
> core_check_exceptions(vcpu);
> 
> 
> work just as well?

That would have us pointlessly checking the exceptions twice in the
non-WE case -- unless you mean only check after, which as described
above means you'll fail to wake up.

-Scott