All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tudor Laurentiu <Laurentiu.Tudor@freescale.com>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Scott Wood <scottwood@freescale.com>,
	linuxppc-dev@lists.ozlabs.org, Stuart Yoder <b08248@gmail.com>
Subject: Re: [PATCH] powerpc/booke64: Configurable lazy interrupt disabling
Date: Wed, 25 Jan 2012 16:32:16 +0200	[thread overview]
Message-ID: <4F201270.4020000@freescale.com> (raw)
In-Reply-To: <1327351831.19850.9.camel@pasglop>

On 01/23/2012 10:50 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2012-01-23 at 13:21 -0600, Scott Wood wrote:
>
>>> BTW, for non-booke, when is DEC checked when interrupts are hard-enabled
>>> as part of exception return?  Likewise with the PS3 HV thing.  I only
>>> see the iseries check in the exception path.
>
> Exception return restores them to their previous state, so shouldn't be
> a problem unless your exception takes long enough to overflow the DEC in
> which case you have other problems. The PS/3 case might be broken.
>
>> Perhaps the issues with a higher priority interrupt intervening can be
>> addressed by messing around with current task priority at the MPIC (with
>> an hcall introduced for the hv case, since currently task priority is
>> not exposed to the guest).  I haven't had time to revisit this, and
>> don't expect to soon.  If someone else wants to try, fine.  In the
>> meantime, lazy EE is causing problems.
>
> Or by storing pending interrupts in an array.
>
>> I'd still like to know the answer to that last question.  It's difficult
>> to determine how to correctly implement this stuff for our hardware if
>> it looks like there are holes in the scheme for hardware that this is
>> supposed to already work with.
>>
>>> Traditionally EE's have always been "level sensitive" on PowerPC,
>>
>> You mean besides the places you already have to work around, such as
>> doorbells
>
> Which you introduced as well... this is especially ironic since BookE
> had the decrementer done right in the first place :-)
>
>> and book3s decrementers
>
> Book3s decrementer is level sensitive based on the sign bit of the
> decrementer (a bit odd but heh....) at least on 64-bit processors.
>
>> and other hypervisors...
>
> I wouldn't take the PS3 HV and legacy iseries HV as good design
> examples :-) The later was working around limited HW functionality at
> the time as well.
>
>> and you force
>> all functions that enable interrupts to create a stack frame to deal
>> with this.
>
> Right, but overall this is still more efficient performance wise on most
> processors than whacking the MSR.
>
> However the main thing is that this significantly improves the quality
> of the samples obtained from performance interrupts which can now act as
> pseudo-NMI up to a certain point.
>
> It also helps with Alex PR KVM but that's just a detail really.
>
>>> the way you changed that is arguably an utterly broken HW design
>>
>> Just because you don't like it, and it doesn't play well with your hack,
>> doesn't make it "utterly broken".
>
> Deal with it. The "hack" has been there for long enough and works well.
> I don't want compile-time conditional to change that behaviour.
>
>>> and I am not too keen on changing our core interrupt handling to deal with it via
>>> ifdef's if we can find a less invasive solution.
>>
>> Wheras having to significantly change the way interrupts work because
>> the register size doubled is so totally reasonable...
>
> We were very tempted at some point to do lazy EE for 32-bit as well,
> eventually decided we didn't care enough :-)
>
>> What is the compelling reason for forcing lazy EE on us?  Why is it tied
>> to 64-bit?
>
> Because that's where we historically implemented it and because iSeries
> more/less required to begin with. And I don't want to have a split
> scheme, especially not a compile time one.

I'm thinking that we could get rid of the compile time option by using 
the "feature fixup" mechanism.

---
Best Regards, Laurentiu

  reply	other threads:[~2012-01-25 14:32 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-18 14:35 [PATCH] powerpc/booke64: Configurable lazy interrupt disabling Laurentiu Tudor
2012-01-18 21:10 ` Benjamin Herrenschmidt
2012-01-19 13:18   ` Tudor Laurentiu
2012-01-19 19:21   ` Stuart Yoder
2012-01-19 19:29     ` Stuart Yoder
2012-01-20 23:05       ` Benjamin Herrenschmidt
2012-01-23 19:21         ` Scott Wood
2012-01-23 20:50           ` Benjamin Herrenschmidt
2012-01-25 14:32             ` Tudor Laurentiu [this message]
2012-01-30 21:47             ` Scott Wood
2012-01-30 22:15               ` Benjamin Herrenschmidt
2012-01-30 23:13                 ` Scott Wood
2012-01-20 23:02     ` Benjamin Herrenschmidt
2012-01-23 19:31       ` Scott Wood

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=4F201270.4020000@freescale.com \
    --to=laurentiu.tudor@freescale.com \
    --cc=b08248@gmail.com \
    --cc=benh@kernel.crashing.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=scottwood@freescale.com \
    /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.