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?
>
> > 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 <jniet...@gmail.com>
>
> Thanks.
>
> cheers
