Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 03/10] raven: move BIOS loading from board code to PCI host

["Andreas Färber" <andreas.faerber@web.de>]

[-- Attachment #1: Type: text/plain, Size: 6115 bytes --]

Am 24.12.2013 01:32, schrieb Alexander Graf:
> 
> On 23.12.2013, at 22:54, Hervé Poussineau <hpoussin@reactos.org> wrote:
> 
>> Andreas Färber a écrit :
>>> Am 23.12.2013 19:13, schrieb Hervé Poussineau:
>>>> Alexander Graf a écrit :
>>>>> On 23.12.2013, at 07:48, Hervé Poussineau <hpoussin@reactos.org> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Andreas Färber a écrit :
>>>>>>> Hi,
>>>>>>> Am 05.11.2013 00:09, schrieb Hervé Poussineau:
>>>>>>>> Raven datasheet explains where firmware lives in system memory, so do
>>>>>>>> it there instead of in board code. Other boards using the same PCI
>>>>>>>> host will not have to copy the firmware loading code.
>>>>>>> This part we had discussed and no one objected to the approach, so OK.
>>>>>>>> However, add a specific hack for Open Hack'Ware, which provides only
>>>>>>>> a 512KB blob to be loaded at 0xfff00000, but expects valid code at
>>>>>>>> 0xfffffffc (specific Open Hack'Ware reset instruction pointer).
>>>>>>> Was this part explained before? I don't spot the equivalent in the
>>>>>>> deleted code. If this is a new workaround, I would rather like to
>>>>>>> put it
>>>>>>> in a separate patch for bisecting (can offer to do that myself then).
>>>>>>> What are the symptoms? I am testing all these patches with OHW.
>>>>>> Old code does (error checking removed):
>>>>>>>> -            bios_size = get_image_size(filename);
>>>>>>>> -                bios_addr = (uint32_t)(-bios_size);
>>>>>>>> -                bios_size = load_image_targphys(filename, bios_addr,
>>>>>> Ie, bios_addr = -512KB (size of OHW blob) = 0xfff80000
>>>>>> and firmware is loaded in the range 0xfff80000-0xffffffff
>>>>>> OHW expects reset instruction pointer to be 0xfffffffc (not valid for
>>>>>> 604, but that's not the point now), which contains a valid instruction.
>>>>>> Note that range 0xfff00000-0xfff7ffff is empty.
>>>>>>
>>>>>> Datasheet for raven says that firmware is at 0xfff00000, so I changed
>>>>>> code to:
>>>>>> +#define BIOS_SIZE (1024 * 1024)
>>>>>> +                  bios_addr = (uint32_t)(-BIOS_SIZE);
>>>>>> +                  bios_size = load_image_targphys(filename, bios_addr,
>>>>>> +                                                  bios_size);
>>>>>> Ie, bios_addr = -1MB = 0xfff00000
>>>>>> and firmware is loaded in the range 0xfff00000-0xfff7ffff.
>>>>>> This doesn't work due to reset instruction pointer which now is
>>>>>> pointing to empty memory, and symptoms are an empty screen on OHW.
>>>>>>
>>>>>> So, I'm adding this hack for OHW, to mirror the 0xfff00000-0xfff7ffff
>>>>>> range to 0xfff80000-0xffffffff.
>>>>>>
>>>>>> So, this patch is a small functional change, as it adds a copy of the
>>>>>> firmware in a new range 0xfff00000-0xfff7ffff, but I think we can
>>>>>> live with it.
>>>>>>
>>>>>> We'll be able to remove it once we switch to another firmware which
>>>>>> uses the right reset instruction pointer or whose size is 1MB.
>>>>> Couldn't we just make the ROM fill the upper part of the 1MB region
>>>>> when we see it's smaller than 1MB? So that we pad at the bottom, not
>>>>> the top?
>>>>>
>>>>>  bios_size = get_image_size(filename);
>>>>>  if (bios_size < 0) {
>>>>>    // error handling
>>>>>  }
>>>>>  assert(bios_size <= (1*MB));
>>>>>  bios_addr = (uint32_t)(-bios_size);
>>>>>
>>>> I don't think that's a good idea, because the PReP cpus (601/604) have a
>>>> reset vector at 0xfff00100. So you have to put some firmware at this
>>>> address, even if firmware is smaller than 1MB.
>>>>
>>>> OHW is the problem here, because it is less than 1MB and expects a reset
>>>> vector at 0xfffffffc. That's why I want to put the hack outside raven
>>>> chipset, in prep machine code.
>>> Let me clarify then that it was me who disabled some checks that used to
>>> assure that the loaded binary is in fact 1MB:
>>> http://git.qemu.org/?p=qemu.git;a=commit;h=74145374bfc0b7b02415184606236f0390479deb
>>> So the issue is actually that the OHW binary is really messed up.
>>> And me, Hervé and Mark have been working on getting OpenBIOS working for
>>> PReP in its place.
>>> So I'm currently considering the following options:
>>> 1)
>>> Revert OHW alias and size/position change
>>> Strip ELF loading and elf-machine
>>> Add back Raven ELF support separately
>>> 2)
>>> Apply my prep.c ELF support patch first
>>> Apply this patch as pure loading-logic movement
>>> 3)
>>> Leave broken OHW loading in prep.c
>>> Only implement 1MB / ELF loading support in Raven
>>> 4)
>>> Accept a 1MB OHW image and drop support for 512KB OHW
> 
> I like this option the best.

Alex, are you volunteering to build one? You wanted to look into that
with me since a looong time ago... :)

http://repo.or.cz/w/openhackware.git

As you will remember, Jocelyn fixed an IDE issue binary-only, without
updating pc-bios/ohw.diff:
http://git.qemu.org/?p=qemu.git;a=commit;h=55aa45ddde3283cdd781326d001f7456bf02f684

I dug out the patch René Rebe proposed later for fixing that IDE issue:
http://lists.gnu.org/archive/html/qemu-devel/2008-05/msg00223.html

I've just managed to fix up that patch to finally apply (apart from line
wraps in a patch to a patch - gosh! - there was also an "address@hidden"
from the archive hidden in the patch context), attached, but haven't yet
re-tried to build with it. It includes a linker script fix that might
explain my previous build issues.

Andreas

> 
> 
> Alex
> 
>>> 5)
>>> Move only MemoryRegion into Raven PHB
>>> Leave loading code in prep.c but move into function for reuse
>>> -> avoids ELF machine property
>>> Opinions?
>>
>> Or maybe:
>> 6) Add the bios loading in Raven like done on this patch (only 1M / ELF loading support), which won't currently be used as long as raven.bios-size is not set,
>> and keep the broken code in prep.c, which will be removed when switching to OpenBIOS ?

P.S. That's my 3). :)

>>
>> Hervé
>>
>>> Another issue that came to my attention is that surely the MemoryRegion
>>> and firmware-loading code should live in the SysBusDevice and not in the
>>> PCIDevice? Also some instance_init vs. realize separation would seem
>>> possible.
>>> Regards,
>>> Andreas
>>
> 


[-- Attachment #2: 0001-fix-PPC-OpenHackWare-for-Linux-2.6.patch --]
[-- Type: text/x-patch, Size: 3912 bytes --]

>From 79265ffab56281416edc48b3daf825a1e00ee5fa Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Rebe?= <rene@exactcode.de>
Date: Tue, 24 Dec 2013 02:39:54 +0100
Subject: [PATCH] fix PPC OpenHackWare for Linux 2.6

---
 pc-bios/ohw.diff | 67 ++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/pc-bios/ohw.diff b/pc-bios/ohw.diff
index c6b6623..4f1680b 100644
--- a/pc-bios/ohw.diff
+++ b/pc-bios/ohw.diff
@@ -1,3 +1,19 @@
+Make it link at al.
+
+  - Rene Rebe <rene@exactcode.de>
+
+--- OpenHackWare-release-0.4/src/main.ld       2005-03-31 09:23:33.000000000 +0200
++++ OpenHackWare-release-0.4-hacked/src/main.ld        2008-05-06 11:23:29.000000000 +0200
+@@ -49,7 +49,7 @@
+         _sdata_end = . ;
+         . = ALIGN(4) ;
+         _ro_start = . ;
+-        .rodata    : { *(.rodata) } > bios
++        .rodata    : { *(.rodata*) } > bios
+         _ro_end = . ;
+         . = ALIGN(4) ;
+         _RTAS_start = .;
+
 diff -wruN --exclude '*~' --exclude '*.o' --exclude '*.bin' --exclude '*.out' --exclude mkdiff OpenHackWare-release-0.4.org/src/bios.h OpenHackWare-release-0.4/src/bios.h
 --- OpenHackWare-release-0.4.org/src/bios.h	2005-04-06 23:20:22.000000000 +0200
 +++ OpenHackWare-release-0.4/src/bios.h	2005-07-07 01:10:20.000000000 +0200
@@ -748,24 +764,14 @@ diff -wruN --exclude '*~' --exclude '*.o' --exclude '*.bin' --exclude '*.out' --
          {
              /* Hack taken 'as-is' from PearPC */
              unsigned char info[] = {
-@@ -1596,7 +1627,9 @@
-         OF_node_put(OF_env, brom);
+@@ -1596,6 +1627,7 @@
          OF_node_put(OF_env, rom);
      }
-+#if 0
      /* From here, hardcoded hacks to get a Mac-like machine */
-+    /* XXX: Core99 does not seem to like this NVRAM tree */
++    /* XXX: Not yet perfect, but Linux 2.6 does oops on boot on Core99 without NVRAM node */
      /* "/nvram@fff04000" node */
      {
          OF_regprop_t regs;
-@@ -1617,6 +1650,7 @@
-         OF_prop_int_new(OF_env, chs, "nvram", OF_pack_handle(OF_env, nvr));
-         OF_node_put(OF_env, nvr);
-     }
-+#endif
-     /* "/pseudo-hid" : hid emulation as Apple does */
-     {
-         OF_node_t *hid;
 @@ -1663,7 +1697,27 @@
          }
          OF_node_put(OF_env, hid);
@@ -1841,3 +1847,40 @@ diff -wruN --exclude '*~' --exclude '*.o' --exclude '*.bin' --exclude '*.out' --
      case ARCH_MAC99:
          /* We are supposed to have 3 host bridges:
           * - the uninorth AGP bridge at 0xF0000000
+
+
+The 2.6 Linux kernel checks for 1, as I do not have the IEEE spec on
+my desk I can only guess it should return 1 on success, not the
+string length.
+
+  - Rene Rebe <rene@exactcode.de>
+
+--- OpenHackWare-release-0.4/src/of.c  2005-04-06 23:17:26.000000000 +0200
++++ OpenHackWare-release-0.4-hacked/src/of.c   2008-05-06 14:50:48.000000000 +0200
+@@ -3841,7 +4061,7 @@
+             OF_DPRINTF("Return property name [%s]\n", next->name);
+             OF_sts(next_name, (void *)(next->name));
+             OF_DUMP_STRING(OF_env, next_name);
+-            pushd(OF_env, strlen(next->name) + 1);
++            pushd(OF_env, 1); /* ReneR: Linux 2.6 flatten_device_tree */
+         }
+     }
+ }
+
+
+In qemu r3309 j_mayer did some "Quickly hack PowerPC BIOS able to boot
+on CDROM again.", as I did not feel like disassemble to find out this
+worked for me for now.
+
+  - Rene Rebe <rene@exactcode.de>
+
+--- OpenHackWare-release-0.4/src/bloc.c        2005-04-06 23:21:00.000000000 +0200
++++ OpenHackWare-release-0.4-hacked/src/bloc.c 2008-05-06 14:20:10.000000000 +0200
+@@ -1021,7 +1038,7 @@
+         status = ide_port_read(bd, 0x07);
+         if (status != 0x08) {
+             ERROR("ATAPI TEST_UNIT_READY : status %0x != 0x08\n", status);
+-            return -1;
++            /*return -1;*/ /* fails to boot from cdrom? */
+         }
+         for (i = 0; i < 3; i++) {
-- 
1.8.4