From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Aneesh Kumar K.V" Date: Wed, 07 May 2014 07:58:38 +0000 Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on Message-Id: <87iopiuijg.fsf@linux.vnet.ibm.com> List-Id: References: <1399224368-22122-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <20140507055626.GA26650@iris.ozlabs.ibm.com> In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Paul Mackerras Cc: agraf@suse.de, benh@kernel.crashing.org, linuxppc-dev@lists.ozlabs.org, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org Paul Mackerras writes: > On Sun, May 04, 2014 at 10:56:08PM +0530, Aneesh Kumar K.V wrote: >> With debug option "sleep inside atomic section checking" enabled we get >> the below WARN_ON during a PR KVM boot. This is because upstream now >> have PREEMPT_COUNT enabled even if we have preempt disabled. Fix the >> warning by adding preempt_disable/enable around floating point and altivec >> enable. > > This worries me a bit. In this code: > >> if (msr & MSR_FP) { >> + preempt_disable(); >> enable_kernel_fp(); >> load_fp_state(&vcpu->arch.fp); >> t->fp_save_area = &vcpu->arch.fp; >> + preempt_enable(); > > What would happen if we actually did get preempted at this point? > Wouldn't we lose the FP state we just loaded? I was not sure we have got CONFIG_PREEMPT working with kvm. So i was not looking at preempted case. But yes, if we have PREEMPT enabled it would break as per the current code. > > In other words, how come we're not already preempt-disabled at this > point? I don't see us disabling preempt in the code path. Are you suggesting that we should be preempt disabled for the whole duration of kvmppc_handle_ext ? -aneesh