| Message ID | e4bfa53ea8c777105359e1770df906909dde2c57.camel@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 505a314fb28ce122091691c51426fa85c084e115 |
| Headers | show |
| Series | powerpc: Fix HMIs on big-endian with CONFIG_RELOCATABLE=y | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/checkpatch | success | Test checkpatch on branch next |
| snowpatch_ozlabs/build-ppc64le | success | Test build-ppc64le on branch next |
| snowpatch_ozlabs/build-ppc64be | success | Test build-ppc64be on branch next |
| snowpatch_ozlabs/build-ppc64e | success | Test build-ppc64e on branch next |
| snowpatch_ozlabs/build-ppc32 | success | Test build-ppc32 on branch next |
On Mon, 08 Oct 2018 15:08:31 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > HMIs will crash the kernel due to > > BRANCH_LINK_TO_FAR(hmi_exception_realmode) > > Calling into the OPD instead of the actual code. > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > --- > > This hack fixes it for me, but it's not great. Nick, any better idea ? Is it a hack because the ifdef gunk, or because there's something deeper wrong with using the .sym? I guess all those handlers that load label address by hand could have the bug silently creep in. Can we have them use the DOTSYM() macro? Thanks, Nick > > diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S > index ea04dfb..752709cc8 100644 > --- a/arch/powerpc/kernel/exceptions-64s.S > +++ b/arch/powerpc/kernel/exceptions-64s.S > @@ -1119,7 +1119,11 @@ TRAMP_REAL_BEGIN(hmi_exception_early) > EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN) > EXCEPTION_PROLOG_COMMON_3(0xe60) > addi r3,r1,STACK_FRAME_OVERHEAD > +#ifdef PPC64_ELF_ABI_v1 > + BRANCH_LINK_TO_FAR(.hmi_exception_realmode) /* Function call ABI */ > +#else > BRANCH_LINK_TO_FAR(hmi_exception_realmode) /* Function call ABI */ > +#endif > cmpdi cr0,r3,0 > > /* Windup the stack. */ > >
On Mon, 2018-10-08 at 17:04 +1000, Nicholas Piggin wrote: > On Mon, 08 Oct 2018 15:08:31 +1100 > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > > > HMIs will crash the kernel due to > > > > BRANCH_LINK_TO_FAR(hmi_exception_realmode) > > > > Calling into the OPD instead of the actual code. > > > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> > > --- > > > > This hack fixes it for me, but it's not great. Nick, any better idea ? > > Is it a hack because the ifdef gunk, or because there's something > deeper wrong with using the .sym? I'd say ifdef gunk, also the KVM use doesn't need it bcs the kvm entry isn't an OPD. > I guess all those handlers that load label address by hand could have > the bug silently creep in. Can we have them use the DOTSYM() macro? The KVM one doesnt have a dotsym does it ? Also should we load the TOC from the OPD ? > Thanks, > Nick > > > > > diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S > > index ea04dfb..752709cc8 100644 > > --- a/arch/powerpc/kernel/exceptions-64s.S > > +++ b/arch/powerpc/kernel/exceptions-64s.S > > @@ -1119,7 +1119,11 @@ TRAMP_REAL_BEGIN(hmi_exception_early) > > EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN) > > EXCEPTION_PROLOG_COMMON_3(0xe60) > > addi r3,r1,STACK_FRAME_OVERHEAD > > +#ifdef PPC64_ELF_ABI_v1 > > + BRANCH_LINK_TO_FAR(.hmi_exception_realmode) /* Function call ABI */ > > +#else > > BRANCH_LINK_TO_FAR(hmi_exception_realmode) /* Function call ABI */ > > +#endif > > cmpdi cr0,r3,0 > > > > /* Windup the stack. */ > > > >
Benjamin Herrenschmidt <benh@kernel.crashing.org> writes: > On Mon, 2018-10-08 at 17:04 +1000, Nicholas Piggin wrote: >> On Mon, 08 Oct 2018 15:08:31 +1100 >> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: >> >> > HMIs will crash the kernel due to >> > >> > BRANCH_LINK_TO_FAR(hmi_exception_realmode) >> > >> > Calling into the OPD instead of the actual code. >> > >> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> >> > --- >> > >> > This hack fixes it for me, but it's not great. Nick, any better idea ? >> >> Is it a hack because the ifdef gunk, or because there's something >> deeper wrong with using the .sym? > > I'd say ifdef gunk, also the KVM use doesn't need it bcs the kvm entry > isn't an OPD. > >> I guess all those handlers that load label address by hand could have >> the bug silently creep in. Can we have them use the DOTSYM() macro? > > The KVM one doesnt have a dotsym does it ? > > Also should we load the TOC from the OPD ? Technically yes. But I thought because we build with mcmodel=medium we'll never actually get multiple TOCs in the kernel itself, so it doesn't actually matter. So this seems to work for now: diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 301a6a86a20f..e5566e248a02 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -1149,7 +1149,7 @@ TRAMP_REAL_BEGIN(hmi_exception_early) EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN) EXCEPTION_PROLOG_COMMON_3(0xe60) addi r3,r1,STACK_FRAME_OVERHEAD - BRANCH_LINK_TO_FAR(hmi_exception_realmode) /* Function call ABI */ + BRANCH_LINK_TO_FAR(DOTSYM(hmi_exception_realmode)) /* Function call ABI */ cmpdi cr0,r3,0 /* Windup the stack. */ cheers
On Tue, 2018-10-09 at 21:37 +1100, Michael Ellerman wrote: > Technically yes. But I thought because we build with mcmodel=medium > we'll never actually get multiple TOCs in the kernel itself, so it > doesn't actually matter. > > So this seems to work for now: Ok, fine. I forgot about DOTSYM. Will do for now. Cheers, Ben. > diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S > index 301a6a86a20f..e5566e248a02 100644 > --- a/arch/powerpc/kernel/exceptions-64s.S > +++ b/arch/powerpc/kernel/exceptions-64s.S > @@ -1149,7 +1149,7 @@ TRAMP_REAL_BEGIN(hmi_exception_early) > EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN) > EXCEPTION_PROLOG_COMMON_3(0xe60) > addi r3,r1,STACK_FRAME_OVERHEAD > - BRANCH_LINK_TO_FAR(hmi_exception_realmode) /* Function call ABI */ > + BRANCH_LINK_TO_FAR(DOTSYM(hmi_exception_realmode)) /* Function call ABI */ > cmpdi cr0,r3,0 > > /* Windup the stack. */ > > > cheers
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index ea04dfb..752709cc8 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -1119,7 +1119,11 @@ TRAMP_REAL_BEGIN(hmi_exception_early) EXCEPTION_PROLOG_COMMON_2(PACA_EXGEN) EXCEPTION_PROLOG_COMMON_3(0xe60) addi r3,r1,STACK_FRAME_OVERHEAD +#ifdef PPC64_ELF_ABI_v1 + BRANCH_LINK_TO_FAR(.hmi_exception_realmode) /* Function call ABI */ +#else BRANCH_LINK_TO_FAR(hmi_exception_realmode) /* Function call ABI */ +#endif cmpdi cr0,r3,0 /* Windup the stack. */
HMIs will crash the kernel due to BRANCH_LINK_TO_FAR(hmi_exception_realmode) Calling into the OPD instead of the actual code. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- This hack fixes it for me, but it's not great. Nick, any better idea ?