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@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/masterI 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; + } + } + #endif
and that's permitted ? :)
flushing the cache does not seem necessary. I suppose because we did not go through this range of instructions yet, I am not sure how qemu handle the cache instructions. Need to check.
Thanks, I will bake a patch for it.
C.