Re: [PATCH 2/2] powerpc/rtas: Fix RTAS MSR[HV] handling for Cell

[Michael Ellerman <mpe@ellerman.id.au>]

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