| Message ID | 1436505599-32109-3-git-send-email-sam.mj@au1.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
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. Now, that being said, FW_FEATURE_* can be 64-bit 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) Cheers, Ben.
On Fri, 2015-07-17 at 11:53 +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. ../.. Even better, you should be able to just do it all in C in pnv_kexec_cpu_down(), after we wait for secondaries to be in OPAL. At that point interrupts are already off, so it should be all good.
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
diff --git a/arch/powerpc/kernel/machine_kexec_64.c b/arch/powerpc/kernel/machine_kexec_64.c index 1a74446..60bb626 100644 --- a/arch/powerpc/kernel/machine_kexec_64.c +++ b/arch/powerpc/kernel/machine_kexec_64.c @@ -29,6 +29,7 @@ #include <asm/prom.h> #include <asm/smp.h> #include <asm/hw_breakpoint.h> +#include <asm/firmware.h> int default_machine_kexec_prepare(struct kimage *image) { @@ -313,7 +314,8 @@ struct paca_struct kexec_paca; /* Our assembly helper, in misc_64.S */ extern void kexec_sequence(void *newstack, unsigned long start, void *image, void *control, - void (*clear_all)(void)) __noreturn; + void (*clear_all)(void), + unsigned long features) __noreturn; /* too late to fail here */ void default_machine_kexec(struct kimage *image) @@ -361,7 +363,7 @@ void default_machine_kexec(struct kimage *image) */ kexec_sequence(&kexec_stack, image->start, image, page_address(image->control_code_page), - ppc_md.hpte_clear_all); + ppc_md.hpte_clear_all, powerpc_firmware_features); /* NOTREACHED */ } diff --git a/arch/powerpc/kernel/misc_64.S b/arch/powerpc/kernel/misc_64.S index 6e4168c..abde07d 100644 --- a/arch/powerpc/kernel/misc_64.S +++ b/arch/powerpc/kernel/misc_64.S @@ -19,6 +19,7 @@ #include <asm/errno.h> #include <asm/processor.h> #include <asm/page.h> +#include <asm/opal.h> #include <asm/cache.h> #include <asm/ppc_asm.h> #include <asm/asm-offsets.h> @@ -539,7 +540,7 @@ real_mode: /* assume normal blr return */ /* - * kexec_sequence(newstack, start, image, control, clear_all()) + * kexec_sequence(newstack, start, image, control, clear_all(), features) * * does the grungy work with stack switching and real mode switches * also does simple calls to other code @@ -575,7 +576,7 @@ _GLOBAL(kexec_sequence) mr r29,r5 /* image (virt) */ mr r28,r6 /* control, unused */ mr r27,r7 /* clear_all() fn desc */ - mr r26,r8 /* spare */ + mr r26,r8 /* powerpc_firmware_features */ lhz r25,PACAHWCPUID(r13) /* get our phys cpu from paca */ /* disable interrupts, we are overwriting kernel data next */ @@ -590,6 +591,20 @@ _GLOBAL(kexec_sequence) /* turn off mmu */ bl real_mode +#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 + + /* Reset HILE bit now that interrupts are disabled */ + li r3,1 + li r0,OPAL_REINIT_CPUS + bl opal_call_realmode +99: +#endif + /* copy 0x100 bytes starting at start to 0 */ li r3,0 mr r4,r30 /* start, aka phys mem offset */
On powernv secondary cpus are returned to OPAL, and will then enter the target kernel in big-endian. However if it is set the HILE bit will persist, causing the first exception in the target kernel to be delivered in litte-endian regardless of the current endianess. If running on top of OPAL make sure the HILE bit is reset before any thread branches into the target kernel. Signed-off-by: Samuel Mendoza-Jonas <sam.mj@au1.ibm.com> --- arch/powerpc/kernel/machine_kexec_64.c | 6 ++++-- arch/powerpc/kernel/misc_64.S | 19 +++++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-)