All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	benh@kernel.crashing.org, linuxppc-dev@lists.ozlabs.org,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on
Date: Wed, 07 May 2014 11:37:53 +0000	[thread overview]
Message-ID: <536A1B11.2030101@suse.de> (raw)
In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com>

On 05/07/2014 07:56 AM, Paul Mackerras wrote:
> 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?
>
> In other words, how come we're not already preempt-disabled at this
> point?

This is probably because we're trying to confuse Linux :). The entry 
path happens with interrupts hard disabled, but preempt enabled so that 
Linux doesn't consider the guest time as non-preemptible. That's the 
only call I could find where preempt is logically enabled (though it 
really isn't).


Alex


WARNING: multiple messages have this Message-ID (diff)
From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org,
	"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on
Date: Wed, 07 May 2014 13:37:53 +0200	[thread overview]
Message-ID: <536A1B11.2030101@suse.de> (raw)
In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com>

On 05/07/2014 07:56 AM, Paul Mackerras wrote:
> 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?
>
> In other words, how come we're not already preempt-disabled at this
> point?

This is probably because we're trying to confuse Linux :). The entry 
path happens with interrupts hard disabled, but preempt enabled so that 
Linux doesn't consider the guest time as non-preemptible. That's the 
only call I could find where preempt is logically enabled (though it 
really isn't).


Alex

WARNING: multiple messages have this Message-ID (diff)
From: Alexander Graf <agraf@suse.de>
To: Paul Mackerras <paulus@samba.org>
Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>,
	benh@kernel.crashing.org, linuxppc-dev@lists.ozlabs.org,
	kvm-ppc@vger.kernel.org, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on
Date: Wed, 07 May 2014 13:37:53 +0200	[thread overview]
Message-ID: <536A1B11.2030101@suse.de> (raw)
In-Reply-To: <20140507055626.GA26650@iris.ozlabs.ibm.com>

On 05/07/2014 07:56 AM, Paul Mackerras wrote:
> 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?
>
> In other words, how come we're not already preempt-disabled at this
> point?

This is probably because we're trying to confuse Linux :). The entry 
path happens with interrupts hard disabled, but preempt enabled so that 
Linux doesn't consider the guest time as non-preemptible. That's the 
only call I could find where preempt is logically enabled (though it 
really isn't).


Alex


  parent reply	other threads:[~2014-05-07 11:37 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-04 17:26 [PATCH] KVM: PPC: BOOK3S: PR: Fix WARN_ON with debug options on Aneesh Kumar K.V
2014-05-04 17:38 ` Aneesh Kumar K.V
2014-05-04 17:26 ` Aneesh Kumar K.V
2014-05-05 11:29 ` Alexander Graf
2014-05-05 11:29   ` Alexander Graf
2014-05-05 11:29   ` Alexander Graf
2014-05-07  5:56 ` Paul Mackerras
2014-05-07  5:56   ` Paul Mackerras
2014-05-07  5:56   ` Paul Mackerras
2014-05-07  7:58   ` Aneesh Kumar K.V
2014-05-07  7:58     ` Aneesh Kumar K.V
2014-05-07  7:58     ` Aneesh Kumar K.V
2014-05-07 11:37   ` Alexander Graf [this message]
2014-05-07 11:37     ` Alexander Graf
2014-05-07 11:37     ` Alexander Graf

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=536A1B11.2030101@suse.de \
    --to=agraf@suse.de \
    --cc=aneesh.kumar@linux.vnet.ibm.com \
    --cc=benh@kernel.crashing.org \
    --cc=kvm-ppc@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.