[OpenBIOS] [PATCH] ppc: use rfid when running under a CPU from the 970 family.

[Cédric Le Goater]

On 06/20/2016 06:59 PM, Thomas Huth wrote:
> On 20.06.2016 17:42, Cédric Le Goater wrote:
>> On 06/20/2016 03:26 PM, Alexander Graf wrote:
>>> On 06/20/2016 02:59 PM, Cédric Le Goater wrote:
>>>> On 06/20/2016 11:49 AM, Mark Cave-Ayland wrote:
>>>>> On 20/06/16 10:33, Alexander Graf wrote:
>>>>>
>>>>>> On 06/20/2016 11:32 AM, Cédric Le Goater wrote:
>>>>>>> On 06/20/2016 09:39 AM, Alexander Graf wrote:
>>>>>>>> On 20.06.16 09:34, Cédric Le Goater wrote:
>>>>>>>>> A recent attempt to restrict the use of rfi to 64bit cpus in qemu
>>>>>>>>> broke 32bit OpenBIOS when run under a 970.
>>>>>>>>>
>>>>>>>>> This is adding a dynamic check on the pvr to choose between rfi and
>>>>>>>>> rfid. Only the 970 family is supported.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Cédric Le Goater <clg at kaod.org>
>>>>>>>>> ---
>>>>>>>>>
>>>>>>>>>    Tested on qemu.
>>>>>>>>>
>>>>>>>>>    arch/ppc/qemu/start.S |   25 +++++++++++++++++++++++--
>>>>>>>>>    arch/ppc/start.S      |   25 +++++++++++++++++++++++--
>>>>>>>>>    2 files changed, 46 insertions(+), 4 deletions(-)
>>>>>>>>>
>>>>>>>>> Index: openbios.git/arch/ppc/qemu/start.S
>>>>>>>>> ===================================================================
>>>>>>>>> --- openbios.git.orig/arch/ppc/qemu/start.S
>>>>>>>>> +++ openbios.git/arch/ppc/qemu/start.S
>>>>>>>>> @@ -27,6 +27,27 @@
>>>>>>>>>      #ifdef CONFIG_PPC_64BITSUPPORT
>>>>>>>>>    +/* When running on ppc64, we cannot use rfi anymore. Let's try to
>>>>>>>>> + * catch which cpu we are running on and act accordingly. This is
>>>>>>>>> + * for 970s only.
>>>>>>>> Do you think you could move the check into the cpu init function and use
>>>>>>>> a global variable / live patching instead? I don't remember if we need
>>>>>>>> to handle any faults before we reach that path.
>>>>>>> So we get an ISI really early, when returning from setup_mmu(), which is
>>>>>>> the first call in _entry.
>>>>>> Ok, can we add the check there?
>>>>> Bear in mind that the code for entry/context switching has been reworked
>>>>> pending commit whilst we switch the OpenBIOS repository over from SVN to
>>>>> git. The relevant commits are here:
>>>>> https://www.coreboot.org/pipermail/openbios/2016-May/009395.html.
>>>>>
>>>>> So please consider OpenBIOS git master plus the patchset above to be the
>>>>> latest code.
>>>> OK. The first version applies fine. I pushed everything here :
>>>>
>>>>     https://github.com/legoater/openbios/commits/master
>>>>
>>>> I am still looking at what Alex is asking for : a global variable doing
>>>> live patching for rfi.
>>>
>>> It's not both - it's either or :).
>>
>> ah :) I need to grow my live patching skills.
>>
>>> So either you define a global variable that we can just read from to determine 
>>> whether we do rfi or rfid 
>>
>> That is what I started doing and that was much bigger then v1.
>>
>>> or we live patch. Looking at the code, it's probably
>>> easiest to live patch even. Just call a function from setup_mmu() that checks 
>>> PVR and if it's 64bit, loops through (0x0 .. 0x1000) in 32bit steps and checks 
>>> if *p == INST_RFI. If so, replace with INST_RFID and flush the cache.
>>
>> Indeed, the hunk below seems to do the trick :
>>
>> 	@@ -524,6 +524,17 @@ setup_mmu(unsigned long ramsize)
>> 	         asm volatile("mtsrin %0,%1" :: "r" (sr_base + i), "r" (j));
>> 	     }
>> 	 
>> 	+    if (is_ppc64()) {
>> 	+#define RFI	0x4c000064
>> 	+#define RFID	0x4c000024
>> 	+        unsigned long addr;
>> 	+
>> 	+        for (addr = 0x2000; addr < 0x3000; addr += 4) {
>> 	+            unsigned long *ptr = (unsigned long *) addr;
>> 	+            if (*ptr == RFI) *ptr = RFID;
> 
> Could you please use "unsigned int" or "uint32_t" instead of unsigned
> long here? ... in case anybody every tries to compile this in 64-bit mode...

oh yes, this needs clean ups and more tests. I have your -prom-env jewel for
that :)

Thanks,

C.