linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Scott Wood <scottwood@freescale.com>
Cc: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>,
	linuxppc-dev@lists.ozlabs.org, Stuart Yoder <b08248@gmail.com>
Subject: Re: [PATCH] powerpc/booke64: Configurable lazy interrupt disabling
Date: Tue, 24 Jan 2012 07:50:31 +1100	[thread overview]
Message-ID: <1327351831.19850.9.camel@pasglop> (raw)
In-Reply-To: <4F1DB344.9030709@freescale.com>

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.

Cheers,
Ben.

  reply	other threads:[~2012-01-23 20:50 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 [this message]
2012-01-25 14:32             ` Tudor Laurentiu
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=1327351831.19850.9.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=Laurentiu.Tudor@freescale.com \
    --cc=b08248@gmail.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).