From: Segher Boessenkool <segher@kernel•crashing.org>
To: Benjamin Herrenschmidt <benh@kernel•crashing.org>
Cc: Samuel Mendoza-Jonas <sam.mj@au1•ibm.com>, linuxppc-dev@ozlabs•org
Subject: Re: [PATCH V3 2/2] powerpc/kexec: Reset HILE before entering target kernel
Date: Fri, 17 Jul 2015 04:59:57 -0500 [thread overview]
Message-ID: <20150717095957.GB15502@gate.crashing.org> (raw)
In-Reply-To: <1437098018.28088.62.camel@kernel.crashing.org>
On Fri, Jul 17, 2015 at 11:53:38AM +1000, Benjamin Herrenschmidt wrote:
> On Fri, 2015-07-10 at 15:19 +1000, Samuel Mendoza-Jonas wrote:
> > +#if defined(CONFIG_PPC_BOOK3S_64) && defined(CONFIG_PPC_POWERNV)
> > + li r3,(FW_FEATURE_OPAL >> 16)
> > + rldicr r3,r3,16,63
> > + and. r3,r3,r26
> > + cmpwi r3,0
> > + beq 99f
>
> If FW_FEATRURE_OPAL is 0x80000000 then the li will sign extend.
>
> The rldicr has a mask of all F's so it will keep all the bits you
> don't care about.
>
> So together, you'll get compares happening on bits above the 16 you care
> about that might change the result of your comparison incorrectly.
>
> Since FW_FEATURE_* bits aren't ABI, they can change, so we don't want
> to impose a constraint on them.
>
> Thus I would recommend using an rdlicl r3,r3,16,48 (aka srdi r3,r3,48)
> instead which is going to clear all bits above 0xffff.
Or the other way around: instead of loading and masking, load zero and
OR the bits in:
+ li r3,0
+ ori r3,(FW_FEATURE_OPAL >> 16)
+ rldicr r3,r3,16,63
+ and. r3,r3,r26
+ cmpwi r3,0
+ beq 99f
which of course is better written as
+ li r3,0
+ oris r3,(FW_FEATURE_OPAL >> 16)
+ and. r3,r3,r26
+ cmpwi r3,0
+ beq 99f
which is
+ andis. r3,r26,(FW_FEATURE_OPAL >> 16)
+ cmpwi r3,0
+ beq 99f
which is
+ andis. r3,r26,(FW_FEATURE_OPAL >> 16)
+ beq 99f
> Now, that being said, FW_FEATURE_* can be 64-bit
All of the code above only works for single-bit constants inside
0xffff0000 though (or 0x7fff0000 for the original).
> and this isn't perf
> critical so why not just load the full 64-bit constant into r3 and
> be done with it ? There's a macro to do that:
>
> LOAD_REG_IMMEDIATE(r3,FW_FEATURE_OPAL)
And as you say later, use C. Yeah.
Segher
prev parent reply other threads:[~2015-07-17 10:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-10 5:19 [PATCH V3 0/2] powerpc/kexec: Reset endianess before kexec Samuel Mendoza-Jonas
2015-07-10 5:19 ` [PATCH V3 1/2] powerpc/kexec: Reset secondary cpu " Samuel Mendoza-Jonas
2015-07-10 5:19 ` [PATCH V3 2/2] powerpc/kexec: Reset HILE before entering target kernel Samuel Mendoza-Jonas
2015-07-17 1:53 ` Benjamin Herrenschmidt
2015-07-17 3:34 ` Benjamin Herrenschmidt
2015-07-17 9:59 ` Segher Boessenkool [this message]
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=20150717095957.GB15502@gate.crashing.org \
--to=segher@kernel$(echo .)crashing.org \
--cc=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=sam.mj@au1$(echo .)ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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