Am 20.06.2016 um 17:42 schrieb Cédric Le Goater clg@kaod.org:
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/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;}- }
#endifand that's permitted ? :)
I don't see why not. But please make sure you run after the code got copied to phys 0 ;).
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.
It's not strictly necessary for tcg because that doesn't expose caching semantics and it's not really needed for pr kvm either because you patch a privileged inst with another privileged one. But it's "the correct thing to do" :).
Alex
Thanks, I will bake a patch for it.
C.