* [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references"
@ 2022-08-23 11:59 Michael Ellerman
2022-08-23 11:59 ` [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell Michael Ellerman
2022-08-31 13:12 ` [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" Michael Ellerman
0 siblings, 2 replies; 7+ messages in thread
From: Michael Ellerman @ 2022-08-23 11:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: rmclure, ldufour, npiggin
This reverts commit 79b74a68486765a4fe685ac4069bc71366c538f5.
It broke booting on IBM Cell machines when the kernel is also built with
CONFIG_PPC_PS3=y.
That's because FW_FEATURE_NATIVE_ALWAYS = 0 does have an important
effect, which is to clear the PS3 ALWAYS features from
FW_FEATURE_ALWAYS.
Note that CONFIG_PPC_NATIVE has since been renamed
CONFIG_PPC_HASH_MMU_NATIVE.
Fixes: 79b74a684867 ("powerpc: Remove unused FW_FEATURE_NATIVE references")
Cc: stable@vger.kernel.org # v5.17+
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/firmware.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 398e0b5e485f..ed6db13a1d7c 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -83,6 +83,8 @@ enum {
FW_FEATURE_POWERNV_ALWAYS = 0,
FW_FEATURE_PS3_POSSIBLE = FW_FEATURE_LPAR | FW_FEATURE_PS3_LV1,
FW_FEATURE_PS3_ALWAYS = FW_FEATURE_LPAR | FW_FEATURE_PS3_LV1,
+ FW_FEATURE_NATIVE_POSSIBLE = 0,
+ FW_FEATURE_NATIVE_ALWAYS = 0,
FW_FEATURE_POSSIBLE =
#ifdef CONFIG_PPC_PSERIES
FW_FEATURE_PSERIES_POSSIBLE |
@@ -92,6 +94,9 @@ enum {
#endif
#ifdef CONFIG_PPC_PS3
FW_FEATURE_PS3_POSSIBLE |
+#endif
+#ifdef CONFIG_PPC_HASH_MMU_NATIVE
+ FW_FEATURE_NATIVE_ALWAYS |
#endif
0,
FW_FEATURE_ALWAYS =
@@ -103,6 +108,9 @@ enum {
#endif
#ifdef CONFIG_PPC_PS3
FW_FEATURE_PS3_ALWAYS &
+#endif
+#ifdef CONFIG_PPC_HASH_MMU_NATIVE
+ FW_FEATURE_NATIVE_ALWAYS &
#endif
FW_FEATURE_POSSIBLE,
--
2.37.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell 2022-08-23 11:59 [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" Michael Ellerman @ 2022-08-23 11:59 ` Michael Ellerman 2022-08-24 1:50 ` Jordan Niethe 2022-08-31 13:12 ` [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" Michael Ellerman 1 sibling, 1 reply; 7+ messages in thread From: Michael Ellerman @ 2022-08-23 11:59 UTC (permalink / raw) To: linuxppc-dev; +Cc: rmclure, ldufour, npiggin The semi-recent changes to MSR handling when entering RTAS (firmware) cause crashes on IBM Cell machines. An example trace: kernel tried to execute user page (2fff01a8) - exploit attempt? (uid: 0) BUG: Unable to handle kernel instruction fetch Faulting instruction address: 0x2fff01a8 Oops: Kernel access of bad area, sig: 11 [#1] BE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=4 NUMA Cell Modules linked in: CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.0.0-rc2-00433-gede0a8d3307a #207 NIP: 000000002fff01a8 LR: 0000000000032608 CTR: 0000000000000000 REGS: c0000000015236b0 TRAP: 0400 Tainted: G W (6.0.0-rc2-00433-gede0a8d3307a) MSR: 0000000008001002 <ME,RI> CR: 00000000 XER: 20000000 ... NIP 0x2fff01a8 LR 0x32608 Call Trace: 0xc00000000143c5f8 (unreliable) .rtas_call+0x224/0x320 .rtas_get_boot_time+0x70/0x150 .read_persistent_clock64+0x114/0x140 .read_persistent_wall_and_boot_offset+0x24/0x80 .timekeeping_init+0x40/0x29c .start_kernel+0x674/0x8f0 start_here_common+0x1c/0x50 Unlike PAPR platforms where RTAS is only used in guests, on the IBM Cell machines Linux runs with MSR[HV] set but also uses RTAS, provided by SLOF. Fix it by copying the MSR[HV] bit from the MSR value we've just read using mfmsr into the value used for RTAS. It seems like we could also fix it using an #ifdef CELL to set MSR[HV], but that doesn't work because it's possible to build a single kernel image that runs on both Cell native and pseries. Fixes: b6b1c3ce06ca ("powerpc/rtas: Keep MSR[RI] set when calling RTAS") Cc: stable@vger.kernel.org # v5.19+ Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/kernel/rtas_entry.S | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S index 9a434d42e660..6ce95ddadbcd 100644 --- a/arch/powerpc/kernel/rtas_entry.S +++ b/arch/powerpc/kernel/rtas_entry.S @@ -109,8 +109,12 @@ _GLOBAL(enter_rtas) * its critical regions (as specified in PAPR+ section 7.2.1). MSR[S] * is not impacted by RFI_TO_KERNEL (only urfid can unset it). So if * MSR[S] is set, it will remain when entering RTAS. + * If we're in HV mode, RTAS must also run in HV mode, so extract MSR_HV + * from the saved MSR value and insert into the value RTAS will use. */ + extrdi r0, r6, 1, 63 - MSR_HV_LG LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI) + insrdi r6, r0, 1, 63 - MSR_HV_LG li r0,0 mtmsrd r0,1 /* disable RI before using SRR0/1 */ -- 2.37.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell 2022-08-23 11:59 ` [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell Michael Ellerman @ 2022-08-24 1:50 ` Jordan Niethe 2022-08-24 12:04 ` Michael Ellerman 0 siblings, 1 reply; 7+ messages in thread From: Jordan Niethe @ 2022-08-24 1:50 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: rmclure, ldufour, npiggin On Tue, 2022-08-23 at 21:59 +1000, Michael Ellerman wrote: > The semi-recent changes to MSR handling when entering RTAS (firmware) > cause crashes on IBM Cell machines. An example trace: > > kernel tried to execute user page (2fff01a8) - exploit attempt? (uid: 0) > BUG: Unable to handle kernel instruction fetch > Faulting instruction address: 0x2fff01a8 > Oops: Kernel access of bad area, sig: 11 [#1] > BE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=4 NUMA Cell > Modules linked in: > CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.0.0-rc2-00433-gede0a8d3307a #207 > NIP: 000000002fff01a8 LR: 0000000000032608 CTR: 0000000000000000 > REGS: c0000000015236b0 TRAP: 0400 Tainted: G W (6.0.0-rc2-00433-gede0a8d3307a) > MSR: 0000000008001002 <ME,RI> CR: 00000000 XER: 20000000 > ... > NIP 0x2fff01a8 > LR 0x32608 > Call Trace: > 0xc00000000143c5f8 (unreliable) > .rtas_call+0x224/0x320 > .rtas_get_boot_time+0x70/0x150 > .read_persistent_clock64+0x114/0x140 > .read_persistent_wall_and_boot_offset+0x24/0x80 > .timekeeping_init+0x40/0x29c > .start_kernel+0x674/0x8f0 > start_here_common+0x1c/0x50 > > Unlike PAPR platforms where RTAS is only used in guests, on the IBM Cell > machines Linux runs with MSR[HV] set but also uses RTAS, provided by > SLOF. > > Fix it by copying the MSR[HV] bit from the MSR value we've just read > using mfmsr into the value used for RTAS. > > It seems like we could also fix it using an #ifdef CELL to set MSR[HV], > but that doesn't work because it's possible to build a single kernel > image that runs on both Cell native and pseries. > > Fixes: b6b1c3ce06ca ("powerpc/rtas: Keep MSR[RI] set when calling RTAS") > Cc: stable@vger.kernel.org # v5.19+ > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/kernel/rtas_entry.S | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S > index 9a434d42e660..6ce95ddadbcd 100644 > --- a/arch/powerpc/kernel/rtas_entry.S > +++ b/arch/powerpc/kernel/rtas_entry.S > @@ -109,8 +109,12 @@ _GLOBAL(enter_rtas) > * its critical regions (as specified in PAPR+ section 7.2.1). MSR[S] > * is not impacted by RFI_TO_KERNEL (only urfid can unset it). So if > * MSR[S] is set, it will remain when entering RTAS. > + * If we're in HV mode, RTAS must also run in HV mode, so extract MSR_HV > + * from the saved MSR value and insert into the value RTAS will use. > */ Interestingly it looks like these are the first uses of these extended mnemonics in the kernel? > + extrdi r0, r6, 1, 63 - MSR_HV_LG Or in non-mnemonic form... rldicl r0, r6, 64 - MSR_HV_LG, 63 > LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI) > + insrdi r6, r0, 1, 63 - MSR_HV_LG Or in non-mnemonic form... rldimi r6, r0, MSR_HV_LG, 63 - MSR_HV_LG It is ok to use r0 as a scratch register as it is loaded with 0 afterwards anyway. > > li r0,0 > mtmsrd r0,1 /* disable RI before using SRR0/1 */ Reviewed-by: Jordan Niethe <jniethe5@gmail.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell 2022-08-24 1:50 ` Jordan Niethe @ 2022-08-24 12:04 ` Michael Ellerman 2022-08-25 1:22 ` Jordan Niethe 0 siblings, 1 reply; 7+ messages in thread From: Michael Ellerman @ 2022-08-24 12:04 UTC (permalink / raw) To: Jordan Niethe, linuxppc-dev; +Cc: rmclure, ldufour, npiggin Jordan Niethe <jniethe5@gmail.com> writes: > On Tue, 2022-08-23 at 21:59 +1000, Michael Ellerman wrote: >> The semi-recent changes to MSR handling when entering RTAS (firmware) >> cause crashes on IBM Cell machines. An example trace: ... >> diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S >> index 9a434d42e660..6ce95ddadbcd 100644 >> --- a/arch/powerpc/kernel/rtas_entry.S >> +++ b/arch/powerpc/kernel/rtas_entry.S >> @@ -109,8 +109,12 @@ _GLOBAL(enter_rtas) >> * its critical regions (as specified in PAPR+ section 7.2.1). MSR[S] >> * is not impacted by RFI_TO_KERNEL (only urfid can unset it). So if >> * MSR[S] is set, it will remain when entering RTAS. >> + * If we're in HV mode, RTAS must also run in HV mode, so extract MSR_HV >> + * from the saved MSR value and insert into the value RTAS will use. >> */ > > Interestingly it looks like these are the first uses of these extended > mnemonics in the kernel? We used to have at least one use I know of in TM code, but it's since been converted to C. >> + extrdi r0, r6, 1, 63 - MSR_HV_LG > > Or in non-mnemonic form... > rldicl r0, r6, 64 - MSR_HV_LG, 63 It's rldicl all the way down. >> LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI) >> + insrdi r6, r0, 1, 63 - MSR_HV_LG > > Or in non-mnemonic form... > rldimi r6, r0, MSR_HV_LG, 63 - MSR_HV_LG I think the extended mnemonics are slightly more readable than the open-coded versions? > It is ok to use r0 as a scratch register as it is loaded with 0 afterwards anyway. I originally used r7, but r0 is more obviously safe. >> >> li r0,0 >> mtmsrd r0,1 /* disable RI before using SRR0/1 */ > > Reviewed-by: Jordan Niethe <jniethe5@gmail.com> Thanks. cheers ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell 2022-08-24 12:04 ` Michael Ellerman @ 2022-08-25 1:22 ` Jordan Niethe 2022-08-25 8:03 ` Michael Ellerman 0 siblings, 1 reply; 7+ messages in thread From: Jordan Niethe @ 2022-08-25 1:22 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: rmclure, ldufour, npiggin On Wed, 2022-08-24 at 22:04 +1000, Michael Ellerman wrote: > Jordan Niethe <jniethe5@gmail.com> writes: > > On Tue, 2022-08-23 at 21:59 +1000, Michael Ellerman wrote: > > > The semi-recent changes to MSR handling when entering RTAS (firmware) > > > cause crashes on IBM Cell machines. An example trace: > ... > > > diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S > > > index 9a434d42e660..6ce95ddadbcd 100644 > > > --- a/arch/powerpc/kernel/rtas_entry.S > > > +++ b/arch/powerpc/kernel/rtas_entry.S > > > @@ -109,8 +109,12 @@ _GLOBAL(enter_rtas) > > > * its critical regions (as specified in PAPR+ section 7.2.1). MSR[S] > > > * is not impacted by RFI_TO_KERNEL (only urfid can unset it). So if > > > * MSR[S] is set, it will remain when entering RTAS. > > > + * If we're in HV mode, RTAS must also run in HV mode, so extract MSR_HV > > > + * from the saved MSR value and insert into the value RTAS will use. > > > */ > > > > Interestingly it looks like these are the first uses of these extended > > mnemonics in the kernel? > > We used to have at least one use I know of in TM code, but it's since > been converted to C. > > > > + extrdi r0, r6, 1, 63 - MSR_HV_LG > > > > Or in non-mnemonic form... > > rldicl r0, r6, 64 - MSR_HV_LG, 63 > > It's rldicl all the way down. > > > > LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI) > > > + insrdi r6, r0, 1, 63 - MSR_HV_LG > > > > Or in non-mnemonic form... > > rldimi r6, r0, MSR_HV_LG, 63 - MSR_HV_LG > > I think the extended mnemonics are slightly more readable than the > open-coded versions? Yeah definitely. I was just noting the plain instruction as I think we have some existing patterns that may be potential candidates for conversion to the extended version. Like in exceptions-64s.S rldicl. r0, r12, (64-MSR_TS_LG), (64-2) to extrdi. r0, r12, 2, 63 - MSR_TS_LG - 1 Would it be worth changing these? > > > It is ok to use r0 as a scratch register as it is loaded with 0 afterwards anyway. > > I originally used r7, but r0 is more obviously safe. > > > > li r0,0 > > > mtmsrd r0,1 /* disable RI before using SRR0/1 */ > > > > Reviewed-by: Jordan Niethe <jniethe5@gmail.com> > > Thanks. > > cheers ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell 2022-08-25 1:22 ` Jordan Niethe @ 2022-08-25 8:03 ` Michael Ellerman 0 siblings, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2022-08-25 8:03 UTC (permalink / raw) To: Jordan Niethe, linuxppc-dev; +Cc: rmclure, ldufour, npiggin Jordan Niethe <jniethe5@gmail.com> writes: > On Wed, 2022-08-24 at 22:04 +1000, Michael Ellerman wrote: >> Jordan Niethe <jniethe5@gmail.com> writes: >> > On Tue, 2022-08-23 at 21:59 +1000, Michael Ellerman wrote: >> > > The semi-recent changes to MSR handling when entering RTAS (firmware) >> > > cause crashes on IBM Cell machines. An example trace: >> ... >> > > diff --git a/arch/powerpc/kernel/rtas_entry.S b/arch/powerpc/kernel/rtas_entry.S >> > > index 9a434d42e660..6ce95ddadbcd 100644 >> > > --- a/arch/powerpc/kernel/rtas_entry.S >> > > +++ b/arch/powerpc/kernel/rtas_entry.S >> > > @@ -109,8 +109,12 @@ _GLOBAL(enter_rtas) >> > > * its critical regions (as specified in PAPR+ section 7.2.1). MSR[S] >> > > * is not impacted by RFI_TO_KERNEL (only urfid can unset it). So if >> > > * MSR[S] is set, it will remain when entering RTAS. >> > > + * If we're in HV mode, RTAS must also run in HV mode, so extract MSR_HV >> > > + * from the saved MSR value and insert into the value RTAS will use. >> > > */ >> > >> > Interestingly it looks like these are the first uses of these extended >> > mnemonics in the kernel? >> >> We used to have at least one use I know of in TM code, but it's since >> been converted to C. >> >> > > + extrdi r0, r6, 1, 63 - MSR_HV_LG >> > >> > Or in non-mnemonic form... >> > rldicl r0, r6, 64 - MSR_HV_LG, 63 >> >> It's rldicl all the way down. >> >> > > LOAD_REG_IMMEDIATE(r6, MSR_ME | MSR_RI) >> > > + insrdi r6, r0, 1, 63 - MSR_HV_LG >> > >> > Or in non-mnemonic form... >> > rldimi r6, r0, MSR_HV_LG, 63 - MSR_HV_LG >> >> I think the extended mnemonics are slightly more readable than the >> open-coded versions? > > Yeah definitely. I was just noting the plain instruction as I think we > have some existing patterns that may be potential candidates for conversion to the > extended version. Like in exceptions-64s.S > > rldicl. r0, r12, (64-MSR_TS_LG), (64-2) > to > extrdi. r0, r12, 2, 63 - MSR_TS_LG - 1 > > Would it be worth changing these? Some folks are very comfortable with rldicl and probably prefer the former, but I'm not sure there's many of those people around anymore :) I think the extrdi is a bit more readable. You could use MSR_TS_T_LG to avoid the - 1? All those uses have a comment about it being 2 bits already. cheers ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" 2022-08-23 11:59 [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" Michael Ellerman 2022-08-23 11:59 ` [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell Michael Ellerman @ 2022-08-31 13:12 ` Michael Ellerman 1 sibling, 0 replies; 7+ messages in thread From: Michael Ellerman @ 2022-08-31 13:12 UTC (permalink / raw) To: Michael Ellerman, linuxppc-dev; +Cc: rmclure, ldufour, npiggin On Tue, 23 Aug 2022 21:59:51 +1000, Michael Ellerman wrote: > This reverts commit 79b74a68486765a4fe685ac4069bc71366c538f5. > > It broke booting on IBM Cell machines when the kernel is also built with > CONFIG_PPC_PS3=y. > > That's because FW_FEATURE_NATIVE_ALWAYS = 0 does have an important > effect, which is to clear the PS3 ALWAYS features from > FW_FEATURE_ALWAYS. > > [...] Applied to powerpc/fixes. [1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" https://git.kernel.org/powerpc/c/310d1344e3c58cc2d625aa4e52cfcb7d8a26fcbf [2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell https://git.kernel.org/powerpc/c/91926d8b7e71aaf5f84f0cf208fc5a8b7a761050 cheers ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-31 13:13 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-08-23 11:59 [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" Michael Ellerman 2022-08-23 11:59 ` [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell Michael Ellerman 2022-08-24 1:50 ` Jordan Niethe 2022-08-24 12:04 ` Michael Ellerman 2022-08-25 1:22 ` Jordan Niethe 2022-08-25 8:03 ` Michael Ellerman 2022-08-31 13:12 ` [PATCH 1/2] Revert "powerpc: Remove unused FW_FEATURE_NATIVE references" Michael Ellerman
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.