Jordan Niethe <jniet...@gmail.com> writes: > On Wed, 2022-08-24 at 22:04 +1000, Michael Ellerman wrote: >> Jordan Niethe <jniet...@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
