Laurent Dufour <ldufour@xxxxxxxxxxxxx> writes: > On 03/05/2022, 18:16:29, Fabiano Rosas wrote: >> Michael Ellerman <mpe@xxxxxxxxxxxxxx> writes: >> >>>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S >>>> index 9581906b5ee9..65cb14b56f8d 100644 >>>> --- a/arch/powerpc/kernel/entry_64.S >>>> +++ b/arch/powerpc/kernel/entry_64.S >>>> @@ -330,22 +330,18 @@ _GLOBAL(enter_rtas) >>>> clrldi r4,r4,2 /* convert to realmode address */ >>>> mtlr r4 >>>> >>>> - li r0,0 >>>> - ori r0,r0,MSR_EE|MSR_SE|MSR_BE|MSR_RI >>>> - andc r0,r6,r0 >>>> - >>>> - li r9,1 >>>> - rldicr r9,r9,MSR_SF_LG,(63-MSR_SF_LG) >>>> - ori r9,r9,MSR_IR|MSR_DR|MSR_FE0|MSR_FE1|MSR_FP|MSR_RI|MSR_LE >>>> - andc r6,r0,r9 >>> >>> One advantage of the old method is it can adapt to new MSR bits being >>> set by the kernel. >>> >>> For example we used to use RTAS on powernv, and this code didn't need >>> updating to cater to MSR_HV being set. We will probably never use RTAS >>> on bare-metal again, so that's OK. >>> >>> But your change might break secure virtual machines, because it clears >>> MSR_S whereas the old code didn't. I think SVMs did use RTAS, but I >>> don't know whether it matters if it's called with MSR_S set or not? >>> >>> Not sure if anyone will remember, or has a working setup they can test. >>> Maybe for now we just copy MSR_S from the kernel MSR the way the >>> current code does. >> >> Would the kernel even be able to change the bit? I think only urfid can >> clear MSR_S. > > That's a good point, thanks Fabiano! > > The POWER ISA programming note about MSR[S] is explicit: > "MSR[S] can be set to 1 only by the System Call instruction and some > interrupts. It can be set to 0 only by urfid." > > Since RTAS is entered using rfid, MSR[S] should remain whatever is the > value in SRR1. > > And that's what POWER ISA is saying about the rfid instruction, in the > synopsis of the instruction the bit 41 of the resulting MSR (aka MSR[S]) is > not impacted. > > So there is no need to take care of this MSR bit in our code, but for sure, > this should be well commented. > > Michael, do you agree? Yep. Can you send a v3 with updated change log and comment mentioning MSR_S and MSR_LE, thanks. cheers
